Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: inconsistent float64 behaviour between arm64 and amd64 #36536

Closed
kortschak opened this issue Jan 13, 2020 · 7 comments
Closed

cmd/compile: inconsistent float64 behaviour between arm64 and amd64 #36536

kortschak opened this issue Jan 13, 2020 · 7 comments
Milestone

Comments

@kortschak
Copy link
Contributor

@kortschak kortschak commented Jan 13, 2020

What version of Go are you using (go version)?

$ go version
go version go1.13.6 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/user/bin"
GOCACHE="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/user"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/user/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/user/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build254211515=/tmp/go-build -gno-record-gcc-switches"

What did you do?

~ $ cat inv.go 
package main

import "fmt"

func main() {
	nNeg := 10.0
	fpr := []float64{0, 10}
	invNeg := 1 / nNeg
	for i := range fpr {
		fpr[i] = 1 - fpr[i]*invNeg
	}
	fmt.Println(fpr)
}
~ $ cat div.go 
package main

import "fmt"

func main() {
	nNeg := 10.0
	fpr := []float64{0, 10}
	for i := range fpr {
		fpr[i] = 1 - fpr[i]/nNeg
	}
	fmt.Println(fpr)
}
~ $ GOARCH=amd64 go run inv.go 
[1 0]
~ $ GOARCH=amd64 go run div.go 
[1 0]
~ $ GOARCH=arm64 go run inv.go 
[1 -5.551115123125783e-17]
~ $ GOARCH=arm64 go run div.go 
[1 0]

inv.go: https://play.golang.org/p/yqox_9PRxI1
div.go: https://play.golang.org/p/8HhGeB-rUpn

Note that while this terminal session is all run on one machine using qemu-user for arm64, the same behaviour is seen on real arm64 hardware.

What did you expect to see?

Matching behaviour between arm64 and amd64.

What did you see instead?

Floating point difference for one approach.

This difference is only seen when the floats are in a slice, the equivalent in a single value behaves as expected.

~ $ cat inv_single.go 
package main

import "fmt"

func main() {
	nNeg := 10.0
	fpr := 10.0
	invNeg := 1 / nNeg
	fpr = 1 - fpr*invNeg
	fmt.Println(fpr)
}
~ $ GOARCH=amd64 go run inv_single.go 
0
~ $ GOARCH=arm64 go run inv_single.go 
0
@kortschak kortschak mentioned this issue Jan 13, 2020
25 of 25 tasks complete
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 13, 2020

CC @randall77 @griesemer

The spec says (https://golang.org/ref/spec#Arithmetic_operators):

An implementation may combine multiple floating-point operations into a single fused operation, possibly across statements, and produce a result that differs from the value obtained by executing and rounding the instructions individually. An explicit floating-point type conversion rounds to the precision of the target type, preventing fusion that would discard that rounding.

That is, Go has never promised to produce bit-identical results on different architectures. I suspect that is what is happening here, but leaving the issue open for investigation or discussion.

@kortschak

This comment has been minimized.

Copy link
Contributor Author

@kortschak kortschak commented Jan 13, 2020

Yes, that's why I asked at go-nuts. The thing that tipped the balance is that I see a difference between in slice and single value versions of the code. Also note that the original code where the issue was identified expressed the mul and add on different lines, and so should not have been fused. Despite this, amd64 behaved the same way, and I am unable to get the same behaviour as arm64 by interposing explicit float64 conversions within the expression.

@kortschak

This comment has been minimized.

Copy link
Contributor Author

@kortschak kortschak commented Jan 13, 2020

So it does look like it's due to that, just not in the direction I was thinking. On arm64 a fused multiply sub instruction is generated, but on amd64, a mul and sub set are emitted. Adding an explicit float64 conversion makes the arm64 behaviour match amd64.

@randall77

This comment has been minimized.

Copy link
Contributor

@randall77 randall77 commented Jan 13, 2020

Yes, this is the arm64 backend using a fused multiply-subtract to compute the result. invNeg is not exactly 1/10, so 1 - (~1/10)*10 is not exactly zero. But (~1/10)*10 is within an epsilon of 1 (epsilon is around 10^-16 here), so the intermediate rounding gives a result of 0 on other platforms.

@kortschak

This comment has been minimized.

Copy link
Contributor Author

@kortschak kortschak commented Jan 13, 2020

Thanks. So the surprising thing is that in the old code, where the expressions were over two lines

		fpr[i] *= invNeg
		fpr[i] = 1 - fpr[i]

the fused operation is still generated. I would have thought that the *= did a float64 conversion.

@randall77

This comment has been minimized.

Copy link
Contributor

@randall77 randall77 commented Jan 13, 2020

You can only prohibit the fusing with an explicit float64 cast.

@kortschak

This comment has been minimized.

Copy link
Contributor Author

@kortschak kortschak commented Jan 13, 2020

Yes, just confirmed that, the second line would need to be fpr[i] = 1 - float64(fpr[i]). It seems the SSA analysis does an amazing job at finding diffuse objects (though interestingly not if there are separating lines)

@kortschak kortschak closed this Jan 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.