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

Jump to bottom

math/bits: different results when using a pure Go copy of Add64 #54255

Closed
mertcandav opened this issue Aug 4, 2022 · 4 comments
Closed

math/bits: different results when using a pure Go copy of Add64 #54255

mertcandav opened this issue Aug 4, 2022 · 4 comments
Labels
FrozenDueToAge

Comments

@mertcandav
Copy link

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

$ go version
go version go1.18.1 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=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/mertcandav/.cache/go-build"
GOENV="/home/mertcandav/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/mertcandav/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/mertcandav/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go-1.18"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.18/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18.1"
GCCGO="/usr/bin/gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/mertcandav/Desktop/xxc/go.mod"
GOWORK=""
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-build4177709774=/tmp/go-build -gno-record-gcc-switches"

What did you do?

The result of the Add64 function in the math.bits package is not the same as the result of the copy of the function.
https://go.dev/play/p/K16VoyuwfvK

What did you expect to see?

Terminal output:

313 0

What did you see instead?

Terminal output:

304 0
@mvdan mvdan changed the title affected/package: math.bits math/bits: different results when using a pure Go copy of Add64 Aug 4, 2022
@mvdan
Copy link
Member

mvdan commented Aug 4, 2022

I think your program runs into documented undefined behavior:

Add64 returns the sum with carry of x, y and carry: sum = x + y + carry. The carry input must be 0 or 1; otherwise the behavior is undefined. The carryOut output is guaranteed to be 0 or 1.

As to why your results are different, note that Add64 is an intrinsic on amd64, meaning that it compiles down to efficient code: https://cs.opensource.google/go/go/+/refs/tags/go1.19:src/cmd/compile/internal/ssagen/ssa.go;l=4685

I would guess that the efficient intrinsic takes advantage of the undefined inputs to be faster.

@mvdan
Copy link
Member

mvdan commented Aug 4, 2022

cc @randall77 in case I got the above wrong.

@mertcandav
Copy link
Author

I think your program runs into documented undefined behavior:

Add64 returns the sum with carry of x, y and carry: sum = x + y + carry. The carry input must be 0 or 1; otherwise the behavior is undefined. The carryOut output is guaranteed to be 0 or 1.

As to why your results are different, note that Add64 is an intrinsic on amd64, meaning that it compiles down to efficient code: https://cs.opensource.google/go/go/+/refs/tags/go1.19:src/cmd/compile/internal/ssagen/ssa.go;l=4685

I would guess that the efficient intrinsic takes advantage of the undefined inputs to be faster.

If Add64 has a different assembly output, I think that's the likely reason why the results are different.

@rsc
Copy link
Contributor

rsc commented Aug 4, 2022

@mvdan's explanation is correct. Carry is a single bit. Calling Add with carry>1 is not defined and will not give consistent results.

@rsc rsc closed this as completed Aug 4, 2022
@golang golang locked and limited conversation to collaborators Aug 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@rsc @mvdan @gopherbot @mertcandav