[release-branch.go1.18] 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.

For #52788.
Fixes #53397.

Change-Id: I2cacf9f3d2f45e149c361a290f511b2d4ed845c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/405114
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/421457
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>

diff --git a/src/cmd/compile/internal/ssa/gen/ARM64.rules b/src/cmd/compile/internal/ssa/gen/ARM64.rules
index be8be4ebe3215dddbf266b4536c0e235e4a18c09..cc237ad8f3615eb6c1d204c989e7783757e4fda0 100644 (file)
--- a/src/cmd/compile/internal/ssa/gen/ARM64.rules
+++ b/src/cmd/compile/internal/ssa/gen/ARM64.rules
@@ -532,7 +532,7 @@
 (If (GreaterThanF cc) yes no) => (FGT cc yes no)
 (If (GreaterEqualF cc) yes no) => (FGE cc yes no)
 
-(If cond yes no) => (NZ cond yes no)
+(If cond yes no) => (TBNZ [0] cond yes no)
 
 // atomic intrinsics
 // Note: these ops do not accept offset.
@@ -593,6 +593,21 @@
 (NZ (GreaterThanF cc) yes no) => (FGT cc yes no)
 (NZ (GreaterEqualF cc) yes no) => (FGE cc yes no)
 
+(TBNZ [0] (Equal cc) yes no) => (EQ cc yes no)
+(TBNZ [0] (NotEqual cc) yes no) => (NE cc yes no)
+(TBNZ [0] (LessThan cc) yes no) => (LT cc yes no)
+(TBNZ [0] (LessThanU cc) yes no) => (ULT cc yes no)
+(TBNZ [0] (LessEqual cc) yes no) => (LE cc yes no)
+(TBNZ [0] (LessEqualU cc) yes no) => (ULE cc yes no)
+(TBNZ [0] (GreaterThan cc) yes no) => (GT cc yes no)
+(TBNZ [0] (GreaterThanU cc) yes no) => (UGT cc yes no)
+(TBNZ [0] (GreaterEqual cc) yes no) => (GE cc yes no)
+(TBNZ [0] (GreaterEqualU cc) yes no) => (UGE cc yes no)
+(TBNZ [0] (LessThanF cc) yes no) => (FLT cc yes no)
+(TBNZ [0] (LessEqualF cc) yes no) => (FLE cc yes no)
+(TBNZ [0] (GreaterThanF cc) yes no) => (FGT cc yes no)
+(TBNZ [0] (GreaterEqualF cc) yes no) => (FGE cc yes no)
+
 (EQ (CMPWconst [0] x:(ANDconst [c] y)) yes no) && x.Uses == 1 => (EQ (TSTWconst [int32(c)] y) yes no)
 (NE (CMPWconst [0] x:(ANDconst [c] y)) yes no) && x.Uses == 1 => (NE (TSTWconst [int32(c)] y) yes no)
 (LT (CMPWconst [0] x:(ANDconst [c] y)) yes no) && x.Uses == 1 => (LT (TSTWconst [int32(c)] y) yes no)

diff --git a/src/cmd/compile/internal/ssa/rewriteARM64.go b/src/cmd/compile/internal/ssa/rewriteARM64.go
index c5f53e55072efd3df5d33a865c2c39275d798390..c356d6b97a8603a27c07b136334ee38c4a58868d 100644 (file)
--- a/src/cmd/compile/internal/ssa/rewriteARM64.go
+++ b/src/cmd/compile/internal/ssa/rewriteARM64.go
@@ -28660,10 +28660,11 @@ func rewriteBlockARM64(b *Block) bool {
                        return true
                }
                // match: (If cond yes no)
-               // result: (NZ cond yes no)
+               // result: (TBNZ [0] cond yes no)
                for {
                        cond := b.Controls[0]
-                       b.resetWithControl(BlockARM64NZ, cond)
+                       b.resetWithControl(BlockARM64TBNZ, cond)
+                       b.AuxInt = int64ToAuxInt(0)
                        return true
                }
        case BlockARM64LE:
@@ -30052,6 +30053,161 @@ func rewriteBlockARM64(b *Block) bool {
                        b.Reset(BlockFirst)
                        return true
                }
+       case BlockARM64TBNZ:
+               // match: (TBNZ [0] (Equal cc) yes no)
+               // result: (EQ cc yes no)
+               for b.Controls[0].Op == OpARM64Equal {
+                       v_0 := b.Controls[0]
+                       cc := v_0.Args[0]
+                       if auxIntToInt64(b.AuxInt) != 0 {
+                               break
+                       }
+                       b.resetWithControl(BlockARM64EQ, cc)
+                       return true
+               }
+               // match: (TBNZ [0] (NotEqual cc) yes no)
+               // result: (NE cc yes no)
+               for b.Controls[0].Op == OpARM64NotEqual {
+                       v_0 := b.Controls[0]
+                       cc := v_0.Args[0]
+                       if auxIntToInt64(b.AuxInt) != 0 {
+                               break
+                       }
+                       b.resetWithControl(BlockARM64NE, cc)
+                       return true
+               }
+               // match: (TBNZ [0] (LessThan cc) yes no)
+               // result: (LT cc yes no)
+               for b.Controls[0].Op == OpARM64LessThan {
+                       v_0 := b.Controls[0]
+                       cc := v_0.Args[0]
+                       if auxIntToInt64(b.AuxInt) != 0 {
+                               break
+                       }
+                       b.resetWithControl(BlockARM64LT, cc)
+                       return true
+               }
+               // match: (TBNZ [0] (LessThanU cc) yes no)
+               // result: (ULT cc yes no)
+               for b.Controls[0].Op == OpARM64LessThanU {
+                       v_0 := b.Controls[0]
+                       cc := v_0.Args[0]
+                       if auxIntToInt64(b.AuxInt) != 0 {
+                               break
+                       }
+                       b.resetWithControl(BlockARM64ULT, cc)
+                       return true
+               }
+               // match: (TBNZ [0] (LessEqual cc) yes no)
+               // result: (LE cc yes no)
+               for b.Controls[0].Op == OpARM64LessEqual {
+                       v_0 := b.Controls[0]
+                       cc := v_0.Args[0]
+                       if auxIntToInt64(b.AuxInt) != 0 {
+                               break
+                       }
+                       b.resetWithControl(BlockARM64LE, cc)
+                       return true
+               }
+               // match: (TBNZ [0] (LessEqualU cc) yes no)
+               // result: (ULE cc yes no)
+               for b.Controls[0].Op == OpARM64LessEqualU {
+                       v_0 := b.Controls[0]
+                       cc := v_0.Args[0]
+                       if auxIntToInt64(b.AuxInt) != 0 {
+                               break
+                       }
+                       b.resetWithControl(BlockARM64ULE, cc)
+                       return true
+               }
+               // match: (TBNZ [0] (GreaterThan cc) yes no)
+               // result: (GT cc yes no)
+               for b.Controls[0].Op == OpARM64GreaterThan {
+                       v_0 := b.Controls[0]
+                       cc := v_0.Args[0]
+                       if auxIntToInt64(b.AuxInt) != 0 {
+                               break
+                       }
+                       b.resetWithControl(BlockARM64GT, cc)
+                       return true
+               }
+               // match: (TBNZ [0] (GreaterThanU cc) yes no)
+               // result: (UGT cc yes no)
+               for b.Controls[0].Op == OpARM64GreaterThanU {
+                       v_0 := b.Controls[0]
+                       cc := v_0.Args[0]
+                       if auxIntToInt64(b.AuxInt) != 0 {
+                               break
+                       }
+                       b.resetWithControl(BlockARM64UGT, cc)
+                       return true
+               }
+               // match: (TBNZ [0] (GreaterEqual cc) yes no)
+               // result: (GE cc yes no)
+               for b.Controls[0].Op == OpARM64GreaterEqual {
+                       v_0 := b.Controls[0]
+                       cc := v_0.Args[0]
+                       if auxIntToInt64(b.AuxInt) != 0 {
+                               break
+                       }
+                       b.resetWithControl(BlockARM64GE, cc)
+                       return true
+               }
+               // match: (TBNZ [0] (GreaterEqualU cc) yes no)
+               // result: (UGE cc yes no)
+               for b.Controls[0].Op == OpARM64GreaterEqualU {
+                       v_0 := b.Controls[0]
+                       cc := v_0.Args[0]
+                       if auxIntToInt64(b.AuxInt) != 0 {
+                               break
+                       }
+                       b.resetWithControl(BlockARM64UGE, cc)
+                       return true
+               }
+               // match: (TBNZ [0] (LessThanF cc) yes no)
+               // result: (FLT cc yes no)
+               for b.Controls[0].Op == OpARM64LessThanF {
+                       v_0 := b.Controls[0]
+                       cc := v_0.Args[0]
+                       if auxIntToInt64(b.AuxInt) != 0 {
+                               break
+                       }
+                       b.resetWithControl(BlockARM64FLT, cc)
+                       return true
+               }
+               // match: (TBNZ [0] (LessEqualF cc) yes no)
+               // result: (FLE cc yes no)
+               for b.Controls[0].Op == OpARM64LessEqualF {
+                       v_0 := b.Controls[0]
+                       cc := v_0.Args[0]
+                       if auxIntToInt64(b.AuxInt) != 0 {
+                               break
+                       }
+                       b.resetWithControl(BlockARM64FLE, cc)
+                       return true
+               }
+               // match: (TBNZ [0] (GreaterThanF cc) yes no)
+               // result: (FGT cc yes no)
+               for b.Controls[0].Op == OpARM64GreaterThanF {
+                       v_0 := b.Controls[0]
+                       cc := v_0.Args[0]
+                       if auxIntToInt64(b.AuxInt) != 0 {
+                               break
+                       }
+                       b.resetWithControl(BlockARM64FGT, cc)
+                       return true
+               }
+               // match: (TBNZ [0] (GreaterEqualF cc) yes no)
+               // result: (FGE cc yes no)
+               for b.Controls[0].Op == OpARM64GreaterEqualF {
+                       v_0 := b.Controls[0]
+                       cc := v_0.Args[0]
+                       if auxIntToInt64(b.AuxInt) != 0 {
+                               break
+                       }
+                       b.resetWithControl(BlockARM64FGE, cc)
+                       return true
+               }
        case BlockARM64UGE:
                // match: (UGE (FlagConstant [fc]) yes no)
                // cond: fc.uge()

diff --git a/test/fixedbugs/issue52788.go b/test/fixedbugs/issue52788.go
new file mode 100644 (file)
index 0000000..b0d7d14
--- /dev/null
+++ b/test/fixedbugs/issue52788.go
@@ -0,0 +1,27 @@
+// run
+
+// Copyright 2022 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.
+
+// Issue 52788: miscompilation for boolean comparison on ARM64.
+
+package main
+
+import (
+       "fmt"
+       "reflect"
+)
+
+func f(next func() bool) {
+       for b := next(); b; b = next() {
+               fmt.Printf("next() returned %v\n", b)
+       }
+}
+
+func main() {
+       next := reflect.MakeFunc(reflect.TypeOf((func() bool)(nil)), func(_ []reflect.Value) []reflect.Value {
+               return []reflect.Value{reflect.ValueOf(false)}
+       })
+       reflect.ValueOf(f).Call([]reflect.Value{next})
+}