aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKeith Randall <khr@golang.org>2024-03-12 12:56:03 -0700
committerThan McIntosh <thanm@google.com>2024-03-27 18:49:49 +0000
commit2c6d106541c1976c52f0c835ce1edbf5ac06b2f2 (patch)
treec25cfd213feaa83710975ae609166916ccca47f2
parent46587483e30188ce485097cdeb48a4d1e0c8a445 (diff)
downloadgo-2c6d106541c1976c52f0c835ce1edbf5ac06b2f2.tar.gz
go-2c6d106541c1976c52f0c835ce1edbf5ac06b2f2.zip
[release-branch.go1.22] cmd/compile: don't assume args are always zero-extended
On amd64, we always zero-extend when loading arguments from the stack. On arm64, we extend based on the type. This causes problems with zeroUpper*Bits, which reports the top bits are zero when they aren't. Fix it to use the type to decide if the top bits are really zero. For tests, only f32 currently fails on arm64. Added other tests just for future-proofing. Fixes #66076 Change-Id: I2f13fb47198e139ef13c9a34eb1edc932eea3ee3 Reviewed-on: https://go-review.googlesource.com/c/go/+/571135 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Carlos Amedee <carlos@golang.org> Reviewed-by: David Chase <drchase@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> (cherry picked from commit 6bf8b76b95b22e3a7fbcd7cb95bfdb3f42bba60a) Reviewed-on: https://go-review.googlesource.com/c/go/+/573395 Reviewed-by: Cherry Mui <cherryyz@google.com>
-rw-r--r--src/cmd/compile/internal/ssa/rewrite.go8
-rw-r--r--test/fixedbugs/issue66066b.go58
2 files changed, 63 insertions, 3 deletions
diff --git a/src/cmd/compile/internal/ssa/rewrite.go b/src/cmd/compile/internal/ssa/rewrite.go
index 531c5e4cb2..5ed7331703 100644
--- a/src/cmd/compile/internal/ssa/rewrite.go
+++ b/src/cmd/compile/internal/ssa/rewrite.go
@@ -1295,7 +1295,9 @@ func zeroUpper32Bits(x *Value, depth int) bool {
OpARM64MADDW, OpARM64MSUBW, OpARM64RORW, OpARM64RORWconst:
return true
case OpArg: // note: but not ArgIntReg
- return x.Type.Size() == 4
+ // amd64 always loads args from the stack unsigned.
+ // most other architectures load them sign/zero extended based on the type.
+ return x.Type.Size() == 4 && (x.Type.IsUnsigned() || x.Block.Func.Config.arch == "amd64")
case OpPhi, OpSelect0, OpSelect1:
// Phis can use each-other as an arguments, instead of tracking visited values,
// just limit recursion depth.
@@ -1319,7 +1321,7 @@ func zeroUpper48Bits(x *Value, depth int) bool {
case OpAMD64MOVWQZX, OpAMD64MOVWload, OpAMD64MOVWloadidx1, OpAMD64MOVWloadidx2:
return true
case OpArg: // note: but not ArgIntReg
- return x.Type.Size() == 2
+ return x.Type.Size() == 2 && (x.Type.IsUnsigned() || x.Block.Func.Config.arch == "amd64")
case OpPhi, OpSelect0, OpSelect1:
// Phis can use each-other as an arguments, instead of tracking visited values,
// just limit recursion depth.
@@ -1343,7 +1345,7 @@ func zeroUpper56Bits(x *Value, depth int) bool {
case OpAMD64MOVBQZX, OpAMD64MOVBload, OpAMD64MOVBloadidx1:
return true
case OpArg: // note: but not ArgIntReg
- return x.Type.Size() == 1
+ return x.Type.Size() == 1 && (x.Type.IsUnsigned() || x.Block.Func.Config.arch == "amd64")
case OpPhi, OpSelect0, OpSelect1:
// Phis can use each-other as an arguments, instead of tracking visited values,
// just limit recursion depth.
diff --git a/test/fixedbugs/issue66066b.go b/test/fixedbugs/issue66066b.go
new file mode 100644
index 0000000000..7540a85293
--- /dev/null
+++ b/test/fixedbugs/issue66066b.go
@@ -0,0 +1,58 @@
+// run
+
+// Copyright 2024 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+//go:noinline
+func f32(_, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, x int32) uint64 {
+ return uint64(uint32(x))
+}
+
+//go:noinline
+func f16(_, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, x int16) uint64 {
+ return uint64(uint16(x))
+}
+
+//go:noinline
+func f8(_, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, x int8) uint64 {
+ return uint64(uint8(x))
+}
+
+//go:noinline
+func g32(_, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, x uint32) int64 {
+ return int64(int32(x))
+}
+
+//go:noinline
+func g16(_, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, x uint16) int64 {
+ return int64(int16(x))
+}
+
+//go:noinline
+func g8(_, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, x uint8) int64 {
+ return int64(int8(x))
+}
+
+func main() {
+ if got := f32(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -1); got != 0xffffffff {
+ println("bad f32", got)
+ }
+ if got := f16(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -1); got != 0xffff {
+ println("bad f16", got)
+ }
+ if got := f8(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -1); got != 0xff {
+ println("bad f8", got)
+ }
+ if got := g32(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xffffffff); got != -1 {
+ println("bad g32", got)
+ }
+ if got := g16(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xffff); got != -1 {
+ println("bad g16", got)
+ }
+ if got := g8(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff); got != -1 {
+ println("bad g8", got)
+ }
+}