Skip to content

Commit 9ae7dc3

Browse files
committed
cmd/compile: fix If lowering on ARM64
On ARM64, an If block is lowered to (NZ cond yes no). This is incorrect because cond is a boolean value and therefore only the last byte is meaningful (same as AMD64, see ARM64Ops.go). But here we are comparing a full register width with 0. Correct it by comparing only the last bit. Fixes #52788. Change-Id: I2cacf9f3d2f45e149c361a290f511b2d4ed845c4 Reviewed-on: https://go-review.googlesource.com/c/go/+/405114 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: David Chase <[email protected]> Run-TryBot: Cherry Mui <[email protected]>
1 parent 53f1312 commit 9ae7dc3

File tree

3 files changed

+201
-3
lines changed

3 files changed

+201
-3
lines changed

src/cmd/compile/internal/ssa/gen/ARM64.rules

+16-1
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,7 @@
532532
(If (GreaterThanF cc) yes no) => (FGT cc yes no)
533533
(If (GreaterEqualF cc) yes no) => (FGE cc yes no)
534534

535-
(If cond yes no) => (NZ cond yes no)
535+
(If cond yes no) => (TBNZ [0] cond yes no)
536536

537537
// atomic intrinsics
538538
// Note: these ops do not accept offset.
@@ -593,6 +593,21 @@
593593
(NZ (GreaterThanF cc) yes no) => (FGT cc yes no)
594594
(NZ (GreaterEqualF cc) yes no) => (FGE cc yes no)
595595

596+
(TBNZ [0] (Equal cc) yes no) => (EQ cc yes no)
597+
(TBNZ [0] (NotEqual cc) yes no) => (NE cc yes no)
598+
(TBNZ [0] (LessThan cc) yes no) => (LT cc yes no)
599+
(TBNZ [0] (LessThanU cc) yes no) => (ULT cc yes no)
600+
(TBNZ [0] (LessEqual cc) yes no) => (LE cc yes no)
601+
(TBNZ [0] (LessEqualU cc) yes no) => (ULE cc yes no)
602+
(TBNZ [0] (GreaterThan cc) yes no) => (GT cc yes no)
603+
(TBNZ [0] (GreaterThanU cc) yes no) => (UGT cc yes no)
604+
(TBNZ [0] (GreaterEqual cc) yes no) => (GE cc yes no)
605+
(TBNZ [0] (GreaterEqualU cc) yes no) => (UGE cc yes no)
606+
(TBNZ [0] (LessThanF cc) yes no) => (FLT cc yes no)
607+
(TBNZ [0] (LessEqualF cc) yes no) => (FLE cc yes no)
608+
(TBNZ [0] (GreaterThanF cc) yes no) => (FGT cc yes no)
609+
(TBNZ [0] (GreaterEqualF cc) yes no) => (FGE cc yes no)
610+
596611
(EQ (CMPWconst [0] x:(ANDconst [c] y)) yes no) && x.Uses == 1 => (EQ (TSTWconst [int32(c)] y) yes no)
597612
(NE (CMPWconst [0] x:(ANDconst [c] y)) yes no) && x.Uses == 1 => (NE (TSTWconst [int32(c)] y) yes no)
598613
(LT (CMPWconst [0] x:(ANDconst [c] y)) yes no) && x.Uses == 1 => (LT (TSTWconst [int32(c)] y) yes no)

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

+158-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/fixedbugs/issue52788.go

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// run
2+
3+
// Copyright 2022 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+
// Issue 52788: miscompilation for boolean comparison on ARM64.
8+
9+
package main
10+
11+
import (
12+
"fmt"
13+
"reflect"
14+
)
15+
16+
func f(next func() bool) {
17+
for b := next(); b; b = next() {
18+
fmt.Printf("next() returned %v\n", b)
19+
}
20+
}
21+
22+
func main() {
23+
next := reflect.MakeFunc(reflect.TypeOf((func() bool)(nil)), func(_ []reflect.Value) []reflect.Value {
24+
return []reflect.Value{reflect.ValueOf(false)}
25+
})
26+
reflect.ValueOf(f).Call([]reflect.Value{next})
27+
}

0 commit comments

Comments
 (0)