Remove implicit floating-point FMAs

[Jake Bailey (jakebailey)]

Inspired by recent Go compression bugs with Go 1.27 doing more FMA on ARM, I had copilot write a lint rule to go through our repo and find anything that might be a bug. The lint rule (and GOCOMPILEDEBUG=fmahash) found two such places where implicit FMA could cause the behavior to change! Fun!

[Copilot]

🟡 Changes recommended

The analyzer incorrectly reports some expressions that already contain valid explicit rounding barriers.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds deterministic floating-point behavior by banning implicit FMAs and explicitly rounding affected calculations.

Changes:

• Adds and registers an SSA-based implicitfma analyzer.
• Adds analyzer fixtures and golden diagnostics.
• Introduces explicit rounding in checker assignment scoring.

File summaries

File	Description
tsc/internal/compiler/checkerpool.go	Explicitly rounds penalty calculations.
tools/customlint/implicitfma.go	Implements FMA detection.
tools/customlint/plugin.go	Registers the analyzer.
tools/customlint/testdata/implicitfma/implicitfma.go	Adds lint test cases.
tools/customlint/testdata/implicitfma/implicitfma.go.golden	Records expected diagnostics.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Balanced

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The analyzer misses package initializers, computed call targets, and products propagated through non-identifier locations.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

tools/customlint/implicitfma.go:204

• This branch visits call arguments but not the function operand, even though the latter may contain runtime arithmetic. Consequently, an expression such as makeT(x*y + z).M() is missed because the inner call is inside the outer call's selector receiver. Evaluate expression.Fun too so implicit FMAs in method receivers and other computed callees are checked.

	case *ast.CallExpr:
		for _, argument := range expression.Args {
			f.evaluate(argument, state)
		}
		return false

tools/customlint/implicitfma.go:237

• Restricting tracked assignments to identifiers misses unrounded products stored in other local variables. For example, s.p = x*y; return s.p + z can still be fused when s is a local unaliased struct, but neither the selector write nor read is represented in state, so the lint reports nothing. The state model needs to cover eligible selector/index/dereference locations or conservatively diagnose these flows.

func (f *implicitFMAPass) setExpressionState(expression ast.Expr, value bool, state implicitFMAState) {
	ident, ok := expression.(*ast.Ident)
	if !ok {
		return

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Balanced

[Ryan Cavanaugh (RyanCavanaugh)]

TIL

[Copilot]

🟡 Changes recommended

The analyzer has several false negatives, a false positive, and non-monotone summaries that can fail to converge.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

tools/customlint/implicitfma.go:311

• Bare returns never inspect named result variables, so a helper that assigns result = x*y and then uses return is summarized as not returning a multiplication. When that helper is inlined into helper(x, y) + z, the ARM compiler emits FMADD, but this analyzer produces no diagnostic. Read the named result objects from the state for result-less returns.

	case *ast.ReturnStmt:
		for _, result := range node.Results {
			f.returnedMultiply = f.evaluate(result, state) || f.returnedMultiply
		}

tools/customlint/implicitfma.go:399

• Immediate function literals discard the taint computed for their arguments. Thus (func(p float64) float64 { return p + z })(x*y) analyzes the body with p untainted and misses an FMA that inlining can introduce. Seed the literal's parameters from arguments before analyzing its CFG.

		if functionLiteral, ok := expression.Fun.(*ast.FuncLit); ok {
			returnedMultiply, foundFMA := f.analyze(f.cfgs.FuncLit(functionLiteral), state)
			f.foundFMA = f.foundFMA || foundFMA
			return returnedMultiply

tools/customlint/implicitfma.go:439

• Collapsing a composite literal to one boolean taints the entire aggregate. For example, value := storage{product: x*y}; return value.other + z is reported even though other never contains the product and cannot participate in that FMA. Preserve taint by keyed field/element location rather than propagating one aggregate-wide value.

	case *ast.CompositeLit:
		unrounded := false
		for _, element := range expression.Elts {

tools/customlint/implicitfma.go:394

• Method expressions place the receiver in Args[0], but this code always derives the receiver from expression.Fun and then treats Args[0] as parameter 0. Consequently T.method(x*y, z) neither applies receiverCausesFMA to x*y nor aligns the remaining arguments with the method parameters. Detect types.MethodExpr, consume its first argument as the receiver, and offset parameter indexing.

	case *ast.CallExpr:
		receiver := f.evaluate(expression.Fun, state)
		arguments := make([]bool, len(expression.Args))
		for i, argument := range expression.Args {
			arguments[i] = f.evaluate(argument, state)

tools/customlint/implicitfma.go:428

• Arguments beyond the declared parameter count are ignored for variadic calls. For func consume(values ...float64), a call such as consume(0, x*y) never consults the summary for values, so an addition using that element after inlining is missed. Map every variadic argument at or beyond the final parameter to that final summary entry.

				for i, argument := range arguments {
					if i < len(summary.parameterCausesFMA) && argument && summary.parameterCausesFMA[i] {
						f.report(expression)
					}
					if i < len(summary.parameterReturnsMultiply) && argument && summary.parameterReturnsMultiply[i] {

• Files reviewed: 7/7 changed files
• Comments generated: 1
• Review effort level: Balanced

[Wesley Wigham (weswigham)]

This feels weird to lint and work around like this. There's no, like, internal compiler flag to just disable the FMA compiler optimization?

[Jake Bailey (jakebailey)]

There is not. You can use GOCOMPILERDEBUG to diagnose it, but then we'd need some sort of matrix to run through the entire OS + arch + GOARM/GOAMD64 etc combo, which is a waste.

I'm considering just killing the lint rule and then just have fix and move on.