-
Notifications
You must be signed in to change notification settings - Fork 18k
cmd/internal/obj/arm64: golang assembler handles arm64 instruction madd/msub incorrectly. #20723
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
Comments
CL https://golang.org/cl/45851 mentions this issue. |
Was this in go1.8 too? Or it was handled correctly in go1.8 and the issue is new on tip (so, a recent regression)? |
I just load the latest golang source code and build using go1.6 for bootstrapping. Then run the test and failed. |
Looking at the git history it looks like it was encoded like that for at least 2 years, so I'll guess that no, it's not a recent regression. Leaving for others to decide if we want this in go1.9. |
Ok, I looked into this. The tests that @zhangfannie added is expecting I agree that the first form looks nicer. But this is potentially a breaking change. There may be user code that works that way, especially given that this has been like that for several releases. We need to be sure that no user will be affected unexpectedly, if we want to make this change. On the other hand, on PPC64, similar FMADD instruction means |
@cherrymui I found the comment ("madd/msub Rm,Rn,Ra,Rd ") in the current code, the current code gets "Rn" value from prog.From3.Reg and gets "Ra" value from prog.Reg. Refer to the instructions assemble process(asm.go), if the comment is right, get arguments value in that way is wrong. And the comment means R4 = R1 * R3 + R2. |
One possibility is to fix the comment to match the implementation. I'm not sure. |
@cherrymui @ALTree I will fix the comment and keep the source code not changed. |
CL https://golang.org/cl/45850 mentions this issue. |
Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (
go version
)?go version devel +59051da Fri Jun 16 06:05:57 2017 +0000 linux/arm64
What operating system and processor architecture are you using (
go env
)?GOARCH="arm64"
GOBIN=""
GOEXE=""
GOHOSTARCH="arm64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/fanzha02/go"
GORACE=""
GOROOT="/home/fanzha02/work/asm_test/golang_us/golang_1/golang"
GOTOOLDIR="/home/fanzha02/work/asm_test/golang_us/golang_1/golang/pkg/tool/linux_arm64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build468264647=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
What did you do?
cd src/cmd/asm/internal/asm
add arm64enc.s test file
enable the ARM64Encoder test command
go test -run "ARM64Encoder"
arm64enc.s test file and endtoend_test.go you can load from this patch:
https://go-review.googlesource.com/c/45851/
If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.
What did you expect to see?
go test successfully
What did you see instead?
the madd/maddw/msub/msubw test cases are failed
The text was updated successfully, but these errors were encountered: