-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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: ssa ARM64 LoweredAtomic[And|Or]8 generates architecturally unpredictable instruction #25823
Comments
This appears to be due to the sequence emitted by the code in src/cmd/compile/internal/ssa/gen/ARM64Ops.go (from line 604): case ssa.OpARM64LoweredAtomicAnd8,
ssa.OpARM64LoweredAtomicOr8:
// LDAXRB (Rarg0), Rtmp
// AND/OR Rarg1, Rtmp
// STLXRB Rtmp, (Rarg0), Rtmp
// CBNZ Rtmp, -3(PC)
r0 := v.Args[0].Reg()
r1 := v.Args[1].Reg()
p := s.Prog(arm64.ALDAXRB)
p.From.Type = obj.TYPE_MEM
p.From.Reg = r0
p.To.Type = obj.TYPE_REG
p.To.Reg = arm64.REGTMP
p1 := s.Prog(v.Op.Asm())
p1.From.Type = obj.TYPE_REG
p1.From.Reg = r1
p1.To.Type = obj.TYPE_REG
p1.To.Reg = arm64.REGTMP
p2 := s.Prog(arm64.ASTLXRB)
p2.From.Type = obj.TYPE_REG
p2.From.Reg = arm64.REGTMP
p2.To.Type = obj.TYPE_MEM
p2.To.Reg = r0
p2.RegTo2 = arm64.REGTMP
p3 := s.Prog(arm64.ACBNZ)
p3.From.Type = obj.TYPE_REG
p3.From.Reg = arm64.REGTMP
p3.To.Type = obj.TYPE_BRANCH
gc.Patch(p3, p) The use of STLXRB with identical src and transfer registers (in the above pseudo sequence |
Seems to be related to 38d35e7 |
If I compile something trivial like:
package main
func main() {
println("Hello")
} with
These code snippets for And8 and Or8 are not guaranteed to work as expected (see above comments), when the s (source) and t (transfer) registers are the same register. |
Thank you for the report @matt2909! /cc @cherrymui @randall77 |
@williamweixiao Does any hardware disallow same src and dst registers for STLXRB? |
Change https://golang.org/cl/117976 mentions this issue: |
It’s not easy to answer this question since there are so many implementations for Arm designed CPU. So I’ve just consulted our hardware design department and their answer is:
|
…/Or8 on ARM64 ARM64 manual says it is "constrained unpredictable" if the src and dst registers of STLXRB are same, although it doesn't seem to cause any problem on real hardwares so far. Fix by allocating a different register to hold the updated value for AtomicAnd8/Or8. We do this by making the ops returns <val,mem> like AtomicAdd, although val will not be used elsewhere. Fixes golang#25823. Change-Id: I735b9822f99877b3c7aee67a65e62b7278dc40df Reviewed-on: https://go-review.googlesource.com/117976 Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Wei Xiao <Wei.Xiao@arm.com>
Change https://golang.org/cl/120660 mentions this issue: |
…for some load/store According to ARM64 manual, it is "constrained unpredictable behavior" if the src and dst registers of some load/store instructions are same. In order to completely prevent such unpredictable behavior, adding the check for load/store instructions that are supported by the assembler in the assembler. Add test cases. Update #25823 Change-Id: I64c14ad99ee543d778e7ec8ae6516a532293dbb3 Reviewed-on: https://go-review.googlesource.com/120660 Run-TryBot: Cherry Zhang <cherryyz@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com>
Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (
go version
)?go version devel +30a63ec Mon Jun 11 12:13:11 2018 +0000 linux/amd64
Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?GOARCH="amd64"
GOBIN="/work/go/go/bin/"
GOCACHE="/home/user/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/work/user/go"
GORACE=""
GOROOT="/work/go/go"
GOTMPDIR=""
GOTOOLDIR="/work/go/go/pkg/tool/linux_amd64"
GCCGO="/usr/bin/gccgo"
CC="gcc"
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"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build897149026=/tmp/go-build -gno-record-gcc-switches"
What did you do?
Compiled some code for arm64. Inspected or executed the code sequence generated by LoweredAtomic[And|Or]8.
What did you expect to see?
The code to run without generating an illegal instruction.
What did you see instead?
This generated an illegal instruction panic.
The text was updated successfully, but these errors were encountered: