Skip to content

Commit 6bf8b76

Browse files
committed
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. Update #66066 Change-Id: I2f13fb47198e139ef13c9a34eb1edc932eea3ee3 Reviewed-on: https://go-review.googlesource.com/c/go/+/571135 Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Than McIntosh <[email protected]> Reviewed-by: Carlos Amedee <[email protected]> Reviewed-by: David Chase <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent 6dca707 commit 6bf8b76

File tree

2 files changed

+63
-3
lines changed

2 files changed

+63
-3
lines changed

src/cmd/compile/internal/ssa/rewrite.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,7 +1295,9 @@ func zeroUpper32Bits(x *Value, depth int) bool {
12951295
OpARM64MADDW, OpARM64MSUBW, OpARM64RORW, OpARM64RORWconst:
12961296
return true
12971297
case OpArg: // note: but not ArgIntReg
1298-
return x.Type.Size() == 4
1298+
// amd64 always loads args from the stack unsigned.
1299+
// most other architectures load them sign/zero extended based on the type.
1300+
return x.Type.Size() == 4 && (x.Type.IsUnsigned() || x.Block.Func.Config.arch == "amd64")
12991301
case OpPhi, OpSelect0, OpSelect1:
13001302
// Phis can use each-other as an arguments, instead of tracking visited values,
13011303
// just limit recursion depth.
@@ -1319,7 +1321,7 @@ func zeroUpper48Bits(x *Value, depth int) bool {
13191321
case OpAMD64MOVWQZX, OpAMD64MOVWload, OpAMD64MOVWloadidx1, OpAMD64MOVWloadidx2:
13201322
return true
13211323
case OpArg: // note: but not ArgIntReg
1322-
return x.Type.Size() == 2
1324+
return x.Type.Size() == 2 && (x.Type.IsUnsigned() || x.Block.Func.Config.arch == "amd64")
13231325
case OpPhi, OpSelect0, OpSelect1:
13241326
// Phis can use each-other as an arguments, instead of tracking visited values,
13251327
// just limit recursion depth.
@@ -1343,7 +1345,7 @@ func zeroUpper56Bits(x *Value, depth int) bool {
13431345
case OpAMD64MOVBQZX, OpAMD64MOVBload, OpAMD64MOVBloadidx1:
13441346
return true
13451347
case OpArg: // note: but not ArgIntReg
1346-
return x.Type.Size() == 1
1348+
return x.Type.Size() == 1 && (x.Type.IsUnsigned() || x.Block.Func.Config.arch == "amd64")
13471349
case OpPhi, OpSelect0, OpSelect1:
13481350
// Phis can use each-other as an arguments, instead of tracking visited values,
13491351
// just limit recursion depth.

test/fixedbugs/issue66066b.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// run
2+
3+
// Copyright 2024 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package main
8+
9+
//go:noinline
10+
func f32(_, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, x int32) uint64 {
11+
return uint64(uint32(x))
12+
}
13+
14+
//go:noinline
15+
func f16(_, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, x int16) uint64 {
16+
return uint64(uint16(x))
17+
}
18+
19+
//go:noinline
20+
func f8(_, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, x int8) uint64 {
21+
return uint64(uint8(x))
22+
}
23+
24+
//go:noinline
25+
func g32(_, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, x uint32) int64 {
26+
return int64(int32(x))
27+
}
28+
29+
//go:noinline
30+
func g16(_, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, x uint16) int64 {
31+
return int64(int16(x))
32+
}
33+
34+
//go:noinline
35+
func g8(_, _, _, _, _, _, _, _, _, _, _, _, _, _, _, _, x uint8) int64 {
36+
return int64(int8(x))
37+
}
38+
39+
func main() {
40+
if got := f32(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -1); got != 0xffffffff {
41+
println("bad f32", got)
42+
}
43+
if got := f16(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -1); got != 0xffff {
44+
println("bad f16", got)
45+
}
46+
if got := f8(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -1); got != 0xff {
47+
println("bad f8", got)
48+
}
49+
if got := g32(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xffffffff); got != -1 {
50+
println("bad g32", got)
51+
}
52+
if got := g16(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xffff); got != -1 {
53+
println("bad g16", got)
54+
}
55+
if got := g8(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff); got != -1 {
56+
println("bad g8", got)
57+
}
58+
}

0 commit comments

Comments
 (0)