mdempsky / unconvert

Remove unnecessary type conversions from Go source
BSD 3-Clause "New" or "Revised" License
377 stars 26 forks source link

Potential false positives in Go 1.9 due to new syntax to force intermediate rounding of floating point operations. #24

Closed dmitshur closed 6 years ago

dmitshur commented 7 years ago

@mdempsky, a new language change in Go 1.9 is described as:

A smaller language change is that the language specification now states when implementations are allowed to fuse floating point operations together, such as by using an architecture's "fused multiply and add" (FMA) instruction to compute x*y + z without rounding the intermediate result x*y. To force the intermediate rounding, write float64(x*y) + z.

If I understand correctly, that means in Go 1.9, the following two snippets have different behavior:

var a, b, c float64 = 1.2, 3.4, 5.6
x := (a * b) + c
var a, b, c float64 = 1.2, 3.4, 5.6
x := float64(a * b) + c // Force intermediate rounding in Go 1.9.

However, unconvert would simplify the second block to the first, which would change behavior.

Is my understanding correct?

dmitshur commented 7 years ago

It seems this was mentioned/discussed in https://github.com/golang/go/issues/17895#issuecomment-293417087.

mdempsky commented 6 years ago

Correct, in visitor.unconvert there should be a check that if ft.Type's underlying type is float32/float64/complex64/complex128, then the conversion needs to be kept.

At least by default. I suppose we could add a -fma=false flag or something to restore historical behavior.