Skip to content

Commit

Permalink
[release-branch.go1.22] cmd/compile: don't assume args are always zer…
Browse files Browse the repository at this point in the history
…o-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 <[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]>
(cherry picked from commit 6bf8b76)
Reviewed-on: https://go-review.googlesource.com/c/go/+/573395
Reviewed-by: Cherry Mui <[email protected]>
  • Loading branch information
randall77 authored and thanm committed Mar 27, 2024
1 parent 4658748 commit 2c6d106
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 3 deletions.
8 changes: 5 additions & 3 deletions src/cmd/compile/internal/ssa/rewrite.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand Down
58 changes: 58 additions & 0 deletions test/fixedbugs/issue66066b.go
Original file line number Diff line number Diff line change
@@ -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)
}
}

0 comments on commit 2c6d106

Please sign in to comment.