1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158
|
From 18a3c114a9e0ed523b6083d5ffcd794115ead878 Mon Sep 17 00:00:00 2001
From: Cherry Zhang <cherryyz@google.com>
Date: Tue, 25 Dec 2018 19:36:25 -0500
Subject: [PATCH] cmd/compile: fix MIPS SGTconst-with-shift rules
(SGTconst [c] (SRLconst _ [d])) && 0 <= int32(c) && uint32(d) <= 31 && 1<<(32-uint32(d)) <= int32(c) -> (MOVWconst [1])
This rule is problematic. 1<<(32-uint32(d)) <= int32(c) meant to
say that it is true if c is greater than the largest possible
value of the right shift. But when d==1, 1<<(32-1) is negative
and results in the wrong comparison.
Rewrite the rules in a more direct way.
Fixes #29402.
Change-Id: I5940fc9538d9bc3a4bcae8aa34672867540dc60e
---
diff --git a/src/cmd/compile/internal/ssa/gen/MIPS.rules b/src/cmd/compile/internal/ssa/gen/MIPS.rules
index 098e19c..db9c5bc 100644
--- a/src/cmd/compile/internal/ssa/gen/MIPS.rules
+++ b/src/cmd/compile/internal/ssa/gen/MIPS.rules
@@ -670,8 +670,8 @@
(SGTUconst [c] (MOVHUreg _)) && 0xffff < uint32(c) -> (MOVWconst [1])
(SGTconst [c] (ANDconst [m] _)) && 0 <= int32(m) && int32(m) < int32(c) -> (MOVWconst [1])
(SGTUconst [c] (ANDconst [m] _)) && uint32(m) < uint32(c) -> (MOVWconst [1])
-(SGTconst [c] (SRLconst _ [d])) && 0 <= int32(c) && uint32(d) <= 31 && 1<<(32-uint32(d)) <= int32(c) -> (MOVWconst [1])
-(SGTUconst [c] (SRLconst _ [d])) && uint32(d) <= 31 && 1<<(32-uint32(d)) <= uint32(c) -> (MOVWconst [1])
+(SGTconst [c] (SRLconst _ [d])) && 0 <= int32(c) && uint32(d) <= 31 && 0xffffffff>>uint32(d) < uint32(c) -> (MOVWconst [1])
+(SGTUconst [c] (SRLconst _ [d])) && uint32(d) <= 31 && 0xffffffff>>uint32(d) < uint32(c) -> (MOVWconst [1])
// absorb constants into branches
(EQ (MOVWconst [0]) yes no) -> (First nil yes no)
diff --git a/src/cmd/compile/internal/ssa/gen/MIPS64.rules b/src/cmd/compile/internal/ssa/gen/MIPS64.rules
index 70f4f0d..9c16c35 100644
--- a/src/cmd/compile/internal/ssa/gen/MIPS64.rules
+++ b/src/cmd/compile/internal/ssa/gen/MIPS64.rules
@@ -667,8 +667,8 @@
(SGTconst [c] (MOVWUreg _)) && c < 0 -> (MOVVconst [0])
(SGTconst [c] (ANDconst [m] _)) && 0 <= m && m < c -> (MOVVconst [1])
(SGTUconst [c] (ANDconst [m] _)) && uint64(m) < uint64(c) -> (MOVVconst [1])
-(SGTconst [c] (SRLVconst _ [d])) && 0 <= c && 0 < d && d <= 63 && 1<<uint64(64-d) <= c -> (MOVVconst [1])
-(SGTUconst [c] (SRLVconst _ [d])) && 0 < d && d <= 63 && 1<<uint64(64-d) <= uint64(c) -> (MOVVconst [1])
+(SGTconst [c] (SRLVconst _ [d])) && 0 <= c && 0 < d && d <= 63 && 0xffffffffffffffff>>uint64(d) < uint64(c) -> (MOVVconst [1])
+(SGTUconst [c] (SRLVconst _ [d])) && 0 < d && d <= 63 && 0xffffffffffffffff>>uint64(d) < uint64(c) -> (MOVVconst [1])
// absorb constants into branches
(EQ (MOVVconst [0]) yes no) -> (First nil yes no)
diff --git a/src/cmd/compile/internal/ssa/rewriteMIPS.go b/src/cmd/compile/internal/ssa/rewriteMIPS.go
index e513981..951c5a5 100644
--- a/src/cmd/compile/internal/ssa/rewriteMIPS.go
+++ b/src/cmd/compile/internal/ssa/rewriteMIPS.go
@@ -5625,7 +5625,7 @@
return true
}
// match: (SGTUconst [c] (SRLconst _ [d]))
- // cond: uint32(d) <= 31 && 1<<(32-uint32(d)) <= uint32(c)
+ // cond: uint32(d) <= 31 && 0xffffffff>>uint32(d) < uint32(c)
// result: (MOVWconst [1])
for {
c := v.AuxInt
@@ -5634,7 +5634,7 @@
break
}
d := v_0.AuxInt
- if !(uint32(d) <= 31 && 1<<(32-uint32(d)) <= uint32(c)) {
+ if !(uint32(d) <= 31 && 0xffffffff>>uint32(d) < uint32(c)) {
break
}
v.reset(OpMIPSMOVWconst)
@@ -5862,7 +5862,7 @@
return true
}
// match: (SGTconst [c] (SRLconst _ [d]))
- // cond: 0 <= int32(c) && uint32(d) <= 31 && 1<<(32-uint32(d)) <= int32(c)
+ // cond: 0 <= int32(c) && uint32(d) <= 31 && 0xffffffff>>uint32(d) < uint32(c)
// result: (MOVWconst [1])
for {
c := v.AuxInt
@@ -5871,7 +5871,7 @@
break
}
d := v_0.AuxInt
- if !(0 <= int32(c) && uint32(d) <= 31 && 1<<(32-uint32(d)) <= int32(c)) {
+ if !(0 <= int32(c) && uint32(d) <= 31 && 0xffffffff>>uint32(d) < uint32(c)) {
break
}
v.reset(OpMIPSMOVWconst)
diff --git a/src/cmd/compile/internal/ssa/rewriteMIPS64.go b/src/cmd/compile/internal/ssa/rewriteMIPS64.go
index 04df5b8..9e12780 100644
--- a/src/cmd/compile/internal/ssa/rewriteMIPS64.go
+++ b/src/cmd/compile/internal/ssa/rewriteMIPS64.go
@@ -6005,7 +6005,7 @@
return true
}
// match: (SGTUconst [c] (SRLVconst _ [d]))
- // cond: 0 < d && d <= 63 && 1<<uint64(64-d) <= uint64(c)
+ // cond: 0 < d && d <= 63 && 0xffffffffffffffff>>uint64(d) < uint64(c)
// result: (MOVVconst [1])
for {
c := v.AuxInt
@@ -6014,7 +6014,7 @@
break
}
d := v_0.AuxInt
- if !(0 < d && d <= 63 && 1<<uint64(64-d) <= uint64(c)) {
+ if !(0 < d && d <= 63 && 0xffffffffffffffff>>uint64(d) < uint64(c)) {
break
}
v.reset(OpMIPS64MOVVconst)
@@ -6223,7 +6223,7 @@
return true
}
// match: (SGTconst [c] (SRLVconst _ [d]))
- // cond: 0 <= c && 0 < d && d <= 63 && 1<<uint64(64-d) <= c
+ // cond: 0 <= c && 0 < d && d <= 63 && 0xffffffffffffffff>>uint64(d) < uint64(c)
// result: (MOVVconst [1])
for {
c := v.AuxInt
@@ -6232,7 +6232,7 @@
break
}
d := v_0.AuxInt
- if !(0 <= c && 0 < d && d <= 63 && 1<<uint64(64-d) <= c) {
+ if !(0 <= c && 0 < d && d <= 63 && 0xffffffffffffffff>>uint64(d) < uint64(c)) {
break
}
v.reset(OpMIPS64MOVVconst)
diff --git a/test/fixedbugs/issue29402.go b/test/fixedbugs/issue29402.go
new file mode 100644
index 0000000..8a1f959
--- /dev/null
+++ b/test/fixedbugs/issue29402.go
@@ -0,0 +1,23 @@
+// run
+
+// Copyright 2018 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 29402: wrong optimization of comparison of
+// constant and shift on MIPS.
+
+package main
+
+//go:noinline
+func F(s []int) bool {
+ half := len(s) / 2
+ return half >= 0
+}
+
+func main() {
+ b := F([]int{1, 2, 3, 4})
+ if !b {
+ panic("FAIL")
+ }
+}
|