Package: golang-1.11 / 1.11.5-1

0005-fix-MIPS-SGTconst-with-shift-rules.patch Patch series | download
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")
+	}
+}