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: float64 to int64 conversion gives different result between amd64 and arm64 on overflows #45588

Closed
aderouineau opened this issue Apr 15, 2021 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@aderouineau
Copy link

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

$ go version
go version go1.16.3 darwin/amd64

$ go version
go version go1.16.3 linux/arm64

Does this issue reproduce with the latest release?

Yes

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

go env Output (amd64)
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/derouin/Library/Caches/go-build"
GOENV="/Users/derouin/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/derouin/go/pkg/mod"
GONOPROXY="*"
GONOSUMDB="*"
GOOS="darwin"
GOPATH="/Users/derouin/go"
GOPRIVATE="*"
GOPROXY="direct"
GOROOT="/usr/local/Cellar/go/1.16.3/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.16.3/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.16.3"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
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 -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/2m/168135fn6l73g8k7h7rmzdsc_h4s04/T/go-build1167711572=/tmp/go-build -gno-record-gcc-switches -fno-common"
go env Output (arm64)
$ go env
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/home/derouin/.cache/go-build"
GOENV="/home/derouin/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/derouin/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/derouin/go"
GOPRIVATE=""
GOPROXY="direct"
GOROOT="/local/home/derouin/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/local/home/derouin/go/pkg/tool/linux_arm64"
GOVCS=""
GOVERSION="go1.16.3"
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 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build70387541=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Run the following small program on amd64 and arm64: https://play.golang.org/p/8nY1Dwgq-BU

What did you expect to see?

Same result on both platforms.

What did you see instead?

-9223372036854775808 on amd64 and 9223372036854775807 on arm64.

@mknyszek mknyszek changed the title int64 conversion gives different result between amd64 and arm64 on overflows cmd/compile: float64 to int64 conversion gives different result between amd64 and arm64 on overflows Apr 15, 2021
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 15, 2021
@mknyszek mknyszek added this to the Backlog milestone Apr 15, 2021
@mknyszek
Copy link
Contributor

For some reason I think this is actually implementation-define behavior, but I'll have a look at the spec real quick. Will CC people in if I can't figure it out.

@mknyszek
Copy link
Contributor

Yes, this appears to be working-as-intended.

From https://golang.org/ref/spec#Conversions:

In all non-constant conversions involving floating-point or complex values, if the result type cannot represent the value the conversion succeeds but the result value is implementation-dependent.

That's what specified now. If you think it should work differently, please file a proposal issue for it. Closing this one for now.

@mknyszek
Copy link
Contributor

FWIW #6923 seems somewhat related (I think for constants this sort of conversion may be well-defined), but I don't actually see any proposals. I believe for constants these non-representable conversions are caught at compile-time.

@aderouineau
Copy link
Author

aderouineau commented Apr 15, 2021

@mknyszek Can you clarify "implementation-dependent" and how to contact the corresponding contributors?

@mknyszek
Copy link
Contributor

I think it means specifically that Go implementations are free to choose the behavior, which includes allowing these differences across architectures. I suspect this particular rule is chosen as a performance optimization.

In general, the result of such a conversion is fairly non-sensical when you can't represent some value in the new type. By allowing a broad range of behaviors, the compiler is able to generate faster code by not having to deal with the case where the conversion fails. Note that there are lots of places where Go doesn't choose implementation-defined behavior for both platform compatibility and just general programmer sanity, but to me this particular case seems sensible because one frequently can't do anything much with such a conversion.

If you'd like to start a discussion about this, the golang-nuts group is a good place to do so. If you have a specific proposal in mind for how this should work, feel free to file a new issue about it with "proposal:" in the title.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants