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: miscompilation of simple arithmetic program targeting darwin/arm64 #62723

Closed
yshcz opened this issue Sep 19, 2023 · 9 comments
Closed
Labels
arch-arm64 compiler/runtime Issues related to the Go compiler and/or runtime.

Comments

@yshcz
Copy link

yshcz commented Sep 19, 2023

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

$ go version
go version go1.21.1 darwin/arm64

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='arm64'
GOBIN=''
GOCACHE='/Users/ysh/Library/Caches/go-build'
GOENV='/Users/ysh/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/ysh/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/ysh/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.21.1'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/ysh/code/go/temp/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/g4/cb9q589d7z58fwmb3694y0900000gq/T/go-build4179046707=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

Here's the reproducer.

package main

import (
	"fmt"
)

func f1(v float64) float64 {
	a := v
	b := v * v

	for i := 0; i < 0; i++ {
		a += v
		b += v * v
	}

	return b - (a * a)
}

func f2(v float64) float64 {
	a := v
	b := v * v

	return b - (a * a)
}

func main() {
	v1 := f1(160985993.83345187)
	v2 := f2(160985993.83345187)
	fmt.Println(v1)
	fmt.Println(v2)
}

What did you expect to see?

0
0

What did you see instead?

1.418611340247807
0

This also reproduces on linux/arm64.

@quki3
Copy link

quki3 commented Sep 19, 2023

https://go.dev/play/p/uU_6sbW3r5E
it isn't be able to reproduce.
run perfect 0 and 0 response

@mauri870
Copy link
Member

mauri870 commented Sep 19, 2023

Seems like this only happens for arm64, I reproduced on darwin/arm64 with go 1.19/1.20/1.21. With tip it does not happen anymore.

Not sure if it happens for arm as well.

Godbolt output for 1.21 vs tip: https://go.godbolt.org/z/Eja7b8jEP

cc @golang/arm

@bcmills bcmills added compiler/runtime Issues related to the Go compiler and/or runtime. arch-arm64 labels Sep 19, 2023
@mauri870
Copy link
Member

mauri870 commented Sep 19, 2023

I did bisect the changes and this issue appears to have been fixed by CL498795, issue #59399.

@bcmills I wonder if we should consider this for backporting to 1.21

@seankhliao seankhliao changed the title possible miscompilation of simple arithmetic program targeting darwin/arm64 cmd/compile: miscompilation of simple arithmetic program targeting darwin/arm64 Sep 19, 2023
@randall77
Copy link
Contributor

This is just extra accuracy provided by FMA (fused multiply-add). Run with -gcflags=-d=fmahash=0 and you'll get the answer you expected.

The difference between f1 and f2 is that f2 inlines and f1 doesn't. That lets the computation in f2 be done entirely at compile time, which doesn't do FMA. If you add a //go:noinline directive to f2, it computes the same answer.

The problem you're seeing is that b := a*a isn't exact, but the computation b-a*a is exact, exactly using the not-quite-the-square-of-a value of b.

@randall77
Copy link
Contributor

@dr2chase Thanks for the flag!

@bcmills
Copy link
Contributor

bcmills commented Sep 19, 2023

Note that this optimization is explicitly mentioned in the spec.

Per https://go.dev/ref/spec#Floating_point_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.

@yshcz
Copy link
Author

yshcz commented Sep 19, 2023

thanks for the spec ref. but this is a quite surprising behavior. I've recently read rsc's post about optimizations affecting program behavior(maybe not a correctness in this case though, but still...). exploiting "unexact" floating point arithmetic inconsistently so that syntactically equivalent programs produces different results feels similar to exploiting undefined behavior in other programming languages as rsc mentioned in the post.

adding severely undocumented compiler flags or inline directives to the program to make its behavior consistent and saying "in some languages, performance trumps everything, but golang is different" sounds pretty dubious to me.

@randall77
Copy link
Contributor

adding severely undocumented compiler flags or inline directives to the program to make its behavior consistent

We are suggesting neither of those fixes. They aren't intended for end users, they are just to help with debugging issues like this.
The right fix is mentioned in the spec, adding a float64 cast in places where you want to force rounding.

@ianlancetaylor
Copy link
Contributor

@y-sh0 Any programming language must make a choice for how to handle floating-point operations. They can aim to get identical results on all platforms, or they can aim to make programs run faster while permitting the possibility of different results. Go has made the latter choice, along with a clear procedure for how to get identical results when that is desirable. For the reason for this choice see, for example, https://people.eecs.berkeley.edu/~wkahan/JAVAhurt.pdf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-arm64 compiler/runtime Issues related to the Go compiler and/or runtime.
Projects
None yet
Development

No branches or pull requests

6 participants