aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCherry Mui <cherryyz@google.com>2022-05-09 12:57:31 -0400
committerThan McIntosh <thanm@google.com>2022-08-08 16:24:37 +0000
commite1099eb289a3da3bfb9f69527e9931d75ff9799b (patch)
treebe890301004a0bdf9ea1d18ce284b0e62cece196
parente05bd7572247b680c3382c9f9c4e6e06179d5354 (diff)
downloadgo-e1099eb289a3da3bfb9f69527e9931d75ff9799b.tar.gz
go-e1099eb289a3da3bfb9f69527e9931d75ff9799b.zip
[release-branch.go1.18] cmd/compile: more fix on boolean ops on ARM64
Following CL 421457, the extension rule is also wrong. It is safe to drop the extension if the value is from a boolean-generating instruction, but not a boolean-typed Value in general (e.g. a Phi or a in-register parameter). Fix it. Updates #52788. Updates #53397. Change-Id: Icf3028fe8e90806f9f57fbe2b38d47da27a97e2a Reviewed-on: https://go-review.googlesource.com/c/go/+/405115 Reviewed-by: David Chase <drchase@google.com> Run-TryBot: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-on: https://go-review.googlesource.com/c/go/+/421458 Reviewed-by: Cherry Mui <cherryyz@google.com> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
-rw-r--r--src/cmd/compile/internal/ssa/gen/ARM64.rules4
-rw-r--r--src/cmd/compile/internal/ssa/rewriteARM64.go148
-rw-r--r--test/fixedbugs/issue52788a.go29
-rw-r--r--test/fixedbugs/issue52788a.out1
4 files changed, 177 insertions, 5 deletions
diff --git a/src/cmd/compile/internal/ssa/gen/ARM64.rules b/src/cmd/compile/internal/ssa/gen/ARM64.rules
index cc237ad8f3..09e70ad13b 100644
--- a/src/cmd/compile/internal/ssa/gen/ARM64.rules
+++ b/src/cmd/compile/internal/ssa/gen/ARM64.rules
@@ -1578,9 +1578,9 @@
(GreaterThanF (InvertFlags x)) => (LessThanF x)
(GreaterEqualF (InvertFlags x)) => (LessEqualF x)
-// Boolean-generating instructions always
+// Boolean-generating instructions (NOTE: NOT all boolean Values) always
// zero upper bit of the register; no need to zero-extend
-(MOVBUreg x) && x.Type.IsBoolean() => (MOVDreg x)
+(MOVBUreg x:((Equal|NotEqual|LessThan|LessThanU|LessThanF|LessEqual|LessEqualU|LessEqualF|GreaterThan|GreaterThanU|GreaterThanF|GreaterEqual|GreaterEqualU|GreaterEqualF) _)) => (MOVDreg x)
// absorb flag constants into conditional instructions
(CSEL [cc] x _ flag) && ccARM64Eval(cc, flag) > 0 => x
diff --git a/src/cmd/compile/internal/ssa/rewriteARM64.go b/src/cmd/compile/internal/ssa/rewriteARM64.go
index c356d6b97a..1ee25c2eee 100644
--- a/src/cmd/compile/internal/ssa/rewriteARM64.go
+++ b/src/cmd/compile/internal/ssa/rewriteARM64.go
@@ -7321,12 +7321,154 @@ func rewriteValueARM64_OpARM64MOVBUreg(v *Value) bool {
v.AuxInt = int64ToAuxInt(int64(uint8(c)))
return true
}
- // match: (MOVBUreg x)
- // cond: x.Type.IsBoolean()
+ // match: (MOVBUreg x:(Equal _))
// result: (MOVDreg x)
for {
x := v_0
- if !(x.Type.IsBoolean()) {
+ if x.Op != OpARM64Equal {
+ break
+ }
+ v.reset(OpARM64MOVDreg)
+ v.AddArg(x)
+ return true
+ }
+ // match: (MOVBUreg x:(NotEqual _))
+ // result: (MOVDreg x)
+ for {
+ x := v_0
+ if x.Op != OpARM64NotEqual {
+ break
+ }
+ v.reset(OpARM64MOVDreg)
+ v.AddArg(x)
+ return true
+ }
+ // match: (MOVBUreg x:(LessThan _))
+ // result: (MOVDreg x)
+ for {
+ x := v_0
+ if x.Op != OpARM64LessThan {
+ break
+ }
+ v.reset(OpARM64MOVDreg)
+ v.AddArg(x)
+ return true
+ }
+ // match: (MOVBUreg x:(LessThanU _))
+ // result: (MOVDreg x)
+ for {
+ x := v_0
+ if x.Op != OpARM64LessThanU {
+ break
+ }
+ v.reset(OpARM64MOVDreg)
+ v.AddArg(x)
+ return true
+ }
+ // match: (MOVBUreg x:(LessThanF _))
+ // result: (MOVDreg x)
+ for {
+ x := v_0
+ if x.Op != OpARM64LessThanF {
+ break
+ }
+ v.reset(OpARM64MOVDreg)
+ v.AddArg(x)
+ return true
+ }
+ // match: (MOVBUreg x:(LessEqual _))
+ // result: (MOVDreg x)
+ for {
+ x := v_0
+ if x.Op != OpARM64LessEqual {
+ break
+ }
+ v.reset(OpARM64MOVDreg)
+ v.AddArg(x)
+ return true
+ }
+ // match: (MOVBUreg x:(LessEqualU _))
+ // result: (MOVDreg x)
+ for {
+ x := v_0
+ if x.Op != OpARM64LessEqualU {
+ break
+ }
+ v.reset(OpARM64MOVDreg)
+ v.AddArg(x)
+ return true
+ }
+ // match: (MOVBUreg x:(LessEqualF _))
+ // result: (MOVDreg x)
+ for {
+ x := v_0
+ if x.Op != OpARM64LessEqualF {
+ break
+ }
+ v.reset(OpARM64MOVDreg)
+ v.AddArg(x)
+ return true
+ }
+ // match: (MOVBUreg x:(GreaterThan _))
+ // result: (MOVDreg x)
+ for {
+ x := v_0
+ if x.Op != OpARM64GreaterThan {
+ break
+ }
+ v.reset(OpARM64MOVDreg)
+ v.AddArg(x)
+ return true
+ }
+ // match: (MOVBUreg x:(GreaterThanU _))
+ // result: (MOVDreg x)
+ for {
+ x := v_0
+ if x.Op != OpARM64GreaterThanU {
+ break
+ }
+ v.reset(OpARM64MOVDreg)
+ v.AddArg(x)
+ return true
+ }
+ // match: (MOVBUreg x:(GreaterThanF _))
+ // result: (MOVDreg x)
+ for {
+ x := v_0
+ if x.Op != OpARM64GreaterThanF {
+ break
+ }
+ v.reset(OpARM64MOVDreg)
+ v.AddArg(x)
+ return true
+ }
+ // match: (MOVBUreg x:(GreaterEqual _))
+ // result: (MOVDreg x)
+ for {
+ x := v_0
+ if x.Op != OpARM64GreaterEqual {
+ break
+ }
+ v.reset(OpARM64MOVDreg)
+ v.AddArg(x)
+ return true
+ }
+ // match: (MOVBUreg x:(GreaterEqualU _))
+ // result: (MOVDreg x)
+ for {
+ x := v_0
+ if x.Op != OpARM64GreaterEqualU {
+ break
+ }
+ v.reset(OpARM64MOVDreg)
+ v.AddArg(x)
+ return true
+ }
+ // match: (MOVBUreg x:(GreaterEqualF _))
+ // result: (MOVDreg x)
+ for {
+ x := v_0
+ if x.Op != OpARM64GreaterEqualF {
break
}
v.reset(OpARM64MOVDreg)
diff --git a/test/fixedbugs/issue52788a.go b/test/fixedbugs/issue52788a.go
new file mode 100644
index 0000000000..38b8416150
--- /dev/null
+++ b/test/fixedbugs/issue52788a.go
@@ -0,0 +1,29 @@
+// 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 ops on ARM64.
+
+package main
+
+import (
+ "fmt"
+ "reflect"
+ "os"
+)
+
+func f(next func() bool) {
+ for b := next(); b; b = next() {
+ fmt.Printf("%v\n", b)
+ os.Exit(0)
+ }
+}
+
+func main() {
+ next := reflect.MakeFunc(reflect.TypeOf((func() bool)(nil)), func(_ []reflect.Value) []reflect.Value {
+ return []reflect.Value{reflect.ValueOf(true)}
+ })
+ reflect.ValueOf(f).Call([]reflect.Value{next})
+}
diff --git a/test/fixedbugs/issue52788a.out b/test/fixedbugs/issue52788a.out
new file mode 100644
index 0000000000..27ba77ddaf
--- /dev/null
+++ b/test/fixedbugs/issue52788a.out
@@ -0,0 +1 @@
+true