Linaro GCC

Merge lp:~paul-codesourcery/gcc-linaro/fix-lp-605255 into lp:gcc-linaro/4.4

  1. fix-lp-605255
  2. Merge into 4.4
Proposed by Paul Brook
Status: Merged
Merged at revision: 93542
Proposed branch: lp:~paul-codesourcery/gcc-linaro/fix-lp-605255
Merge into: lp:gcc-linaro/4.4
Diff against target: 185 lines (+50/-16)
4 files modified
ChangeLog.linaro (+17/-0)
gcc/config/arm/predicates.md (+2/-3)
gcc/config/arm/thumb2.md (+13/-13)
gcc/testsuite/gcc.dg/long-long-shift-1.c (+18/-0)
To merge this branch: bzr merge lp:~paul-codesourcery/gcc-linaro/fix-lp-605255
Reviewer Review Type Date Requested Status
Andrew Stubbs (community) Approve
Review via email: mp+30470@code.launchpad.net

Description of the change

Fix ICE when building libtheora.

The failure involves a shift of a 64-bit variable inside a loop. The RTL loop
optimizers hoist this out of the loop, turning it into a shift-by-constant.
ARM has no 64-bit shift instruction, so the code for a variable shift looks
something like
low = (count > 32)
        ? (high >> (count - 32))
         : (high << (count -32) | low >> count)

When a constant is substituted for count, some of these shifts become
negative. These will eventually be culled by DCE, but last long enough to
cause problems in the Thumb-2 patterns.

The fix is to tighten up the predicates on the Thumb-2 combined arith+shift
patterns so that only valid shift counts are accepted. The ICE does not effect
pure shifts or ARM mode as those patterns allow the shift count to be
reloaded into a register.

In theory we could allow and substitute alternate values for out-of-range
shifts but, given this is dead code, it's not worth the effort.

To post a comment you must log in.
Revision history for this message
Andrew Stubbs (ams-codesourcery) wrote :

Paul, please add something suitable to ChangeLog.linaro. Then update this merge request.

Thanks

Andrew

review: Needs Fixing
Revision history for this message
Andrew Stubbs (ams-codesourcery) wrote :

Thanks Paul, this seems fine now.

I'll do the merge.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ChangeLog.linaro'
2--- ChangeLog.linaro 2010-07-13 15:12:55 +0000
3+++ ChangeLog.linaro 2010-07-26 10:28:28 +0000
4@@ -1,3 +1,20 @@
5+2010-07-20 Paul Brook <paul@codesourcery.com>
6+
7+ gcc/
8+ * config/arm/thumb2.md (thumb_andsi_not_shiftsi_si,
9+ thumb2_notsi_shiftsi, thumb2_notsi_shiftsi_compare0,
10+ thumb2_not_shiftsi_compare0_scratch, thumb2_cmpsi_shiftsi,
11+ thumb2_cmpsi_shiftsi_swp, thumb2_cmpsi_neg_shiftsi,
12+ thumb2_arith_shiftsi, thumb2_arith_shiftsi_compare0,
13+ thumb2_arith_shiftsi_compare0_scratch, thumb2_sub_shiftsi,
14+ thumb2_sub_shiftsi_compare0, thumb2_sub_shiftsi_compare0_scratch):
15+ Use const_shift_count predicate for "M" constraints.
16+ * config/arm/predicates.md (const_shift_operand): Remove.
17+ (const_shift_count): New.
18+
19+ gcc/testsuite/
20+ * gcc.dg/long-long-shift-1.c: New test.
21+
22 2010-07-13 Andrew Stubbs <ams@codesourcery.com>
23
24 gcc/
25
26=== modified file 'gcc/config/arm/predicates.md'
27--- gcc/config/arm/predicates.md 2010-06-17 08:44:15 +0000
28+++ gcc/config/arm/predicates.md 2010-07-26 10:28:28 +0000
29@@ -299,10 +299,9 @@
30 (and (match_code "reg,subreg,mem")
31 (match_operand 0 "nonimmediate_soft_df_operand"))))
32
33-(define_predicate "const_shift_operand"
34+(define_predicate "const_shift_count"
35 (and (match_code "const_int")
36- (ior (match_operand 0 "power_of_two_operand")
37- (match_test "((unsigned HOST_WIDE_INT) INTVAL (op)) < 32"))))
38+ (match_test "((unsigned HOST_WIDE_INT) INTVAL (op)) < 32")))
39
40
41 (define_special_predicate "load_multiple_operation"
42
43=== modified file 'gcc/config/arm/thumb2.md'
44--- gcc/config/arm/thumb2.md 2010-06-17 08:44:15 +0000
45+++ gcc/config/arm/thumb2.md 2010-07-26 10:28:28 +0000
46@@ -57,7 +57,7 @@
47 [(set (match_operand:SI 0 "s_register_operand" "=r")
48 (and:SI (not:SI (match_operator:SI 4 "shift_operator"
49 [(match_operand:SI 2 "s_register_operand" "r")
50- (match_operand:SI 3 "const_int_operand" "M")]))
51+ (match_operand:SI 3 "const_shift_count" "M")]))
52 (match_operand:SI 1 "s_register_operand" "r")))]
53 "TARGET_THUMB2"
54 "bic%?\\t%0, %1, %2%S4"
55@@ -126,7 +126,7 @@
56 [(set (match_operand:SI 0 "s_register_operand" "=r")
57 (not:SI (match_operator:SI 3 "shift_operator"
58 [(match_operand:SI 1 "s_register_operand" "r")
59- (match_operand:SI 2 "const_int_operand" "M")])))]
60+ (match_operand:SI 2 "const_shift_count" "M")])))]
61 "TARGET_THUMB2"
62 "mvn%?\\t%0, %1%S3"
63 [(set_attr "predicable" "yes")
64@@ -138,7 +138,7 @@
65 [(set (reg:CC_NOOV CC_REGNUM)
66 (compare:CC_NOOV (not:SI (match_operator:SI 3 "shift_operator"
67 [(match_operand:SI 1 "s_register_operand" "r")
68- (match_operand:SI 2 "const_int_operand" "M")]))
69+ (match_operand:SI 2 "const_shift_count" "M")]))
70 (const_int 0)))
71 (set (match_operand:SI 0 "s_register_operand" "=r")
72 (not:SI (match_op_dup 3 [(match_dup 1) (match_dup 2)])))]
73@@ -153,7 +153,7 @@
74 [(set (reg:CC_NOOV CC_REGNUM)
75 (compare:CC_NOOV (not:SI (match_operator:SI 3 "shift_operator"
76 [(match_operand:SI 1 "s_register_operand" "r")
77- (match_operand:SI 2 "const_int_operand" "M")]))
78+ (match_operand:SI 2 "const_shift_count" "M")]))
79 (const_int 0)))
80 (clobber (match_scratch:SI 0 "=r"))]
81 "TARGET_THUMB2"
82@@ -319,7 +319,7 @@
83 (compare:CC (match_operand:SI 0 "s_register_operand" "r")
84 (match_operator:SI 3 "shift_operator"
85 [(match_operand:SI 1 "s_register_operand" "r")
86- (match_operand:SI 2 "const_int_operand" "M")])))]
87+ (match_operand:SI 2 "const_shift_count" "M")])))]
88 "TARGET_THUMB2"
89 "cmp%?\\t%0, %1%S3"
90 [(set_attr "conds" "set")
91@@ -331,7 +331,7 @@
92 [(set (reg:CC_SWP CC_REGNUM)
93 (compare:CC_SWP (match_operator:SI 3 "shift_operator"
94 [(match_operand:SI 1 "s_register_operand" "r")
95- (match_operand:SI 2 "const_int_operand" "M")])
96+ (match_operand:SI 2 "const_shift_count" "M")])
97 (match_operand:SI 0 "s_register_operand" "r")))]
98 "TARGET_THUMB2"
99 "cmp%?\\t%0, %1%S3"
100@@ -345,7 +345,7 @@
101 (compare:CC (match_operand:SI 0 "s_register_operand" "r")
102 (neg:SI (match_operator:SI 3 "shift_operator"
103 [(match_operand:SI 1 "s_register_operand" "r")
104- (match_operand:SI 2 "const_int_operand" "M")]))))]
105+ (match_operand:SI 2 "const_shift_count" "M")]))))]
106 "TARGET_THUMB2"
107 "cmn%?\\t%0, %1%S3"
108 [(set_attr "conds" "set")
109@@ -457,7 +457,7 @@
110 (match_operator:SI 1 "shiftable_operator"
111 [(match_operator:SI 3 "shift_operator"
112 [(match_operand:SI 4 "s_register_operand" "r")
113- (match_operand:SI 5 "const_int_operand" "M")])
114+ (match_operand:SI 5 "const_shift_count" "M")])
115 (match_operand:SI 2 "s_register_operand" "r")]))]
116 "TARGET_THUMB2"
117 "%i1%?\\t%0, %2, %4%S3"
118@@ -490,7 +490,7 @@
119 (compare:CC_NOOV (match_operator:SI 1 "shiftable_operator"
120 [(match_operator:SI 3 "shift_operator"
121 [(match_operand:SI 4 "s_register_operand" "r")
122- (match_operand:SI 5 "const_int_operand" "M")])
123+ (match_operand:SI 5 "const_shift_count" "M")])
124 (match_operand:SI 2 "s_register_operand" "r")])
125 (const_int 0)))
126 (set (match_operand:SI 0 "s_register_operand" "=r")
127@@ -508,7 +508,7 @@
128 (compare:CC_NOOV (match_operator:SI 1 "shiftable_operator"
129 [(match_operator:SI 3 "shift_operator"
130 [(match_operand:SI 4 "s_register_operand" "r")
131- (match_operand:SI 5 "const_int_operand" "M")])
132+ (match_operand:SI 5 "const_shift_count" "M")])
133 (match_operand:SI 2 "s_register_operand" "r")])
134 (const_int 0)))
135 (clobber (match_scratch:SI 0 "=r"))]
136@@ -524,7 +524,7 @@
137 (minus:SI (match_operand:SI 1 "s_register_operand" "r")
138 (match_operator:SI 2 "shift_operator"
139 [(match_operand:SI 3 "s_register_operand" "r")
140- (match_operand:SI 4 "const_int_operand" "M")])))]
141+ (match_operand:SI 4 "const_shift_count" "M")])))]
142 "TARGET_THUMB2"
143 "sub%?\\t%0, %1, %3%S2"
144 [(set_attr "predicable" "yes")
145@@ -538,7 +538,7 @@
146 (minus:SI (match_operand:SI 1 "s_register_operand" "r")
147 (match_operator:SI 2 "shift_operator"
148 [(match_operand:SI 3 "s_register_operand" "r")
149- (match_operand:SI 4 "const_int_operand" "M")]))
150+ (match_operand:SI 4 "const_shift_count" "M")]))
151 (const_int 0)))
152 (set (match_operand:SI 0 "s_register_operand" "=r")
153 (minus:SI (match_dup 1) (match_op_dup 2 [(match_dup 3)
154@@ -556,7 +556,7 @@
155 (minus:SI (match_operand:SI 1 "s_register_operand" "r")
156 (match_operator:SI 2 "shift_operator"
157 [(match_operand:SI 3 "s_register_operand" "r")
158- (match_operand:SI 4 "const_int_operand" "M")]))
159+ (match_operand:SI 4 "const_shift_count" "M")]))
160 (const_int 0)))
161 (clobber (match_scratch:SI 0 "=r"))]
162 "TARGET_THUMB2"
163
164=== added file 'gcc/testsuite/gcc.dg/long-long-shift-1.c'
165--- gcc/testsuite/gcc.dg/long-long-shift-1.c 1970-01-01 00:00:00 +0000
166+++ gcc/testsuite/gcc.dg/long-long-shift-1.c 2010-07-26 10:28:28 +0000
167@@ -0,0 +1,18 @@
168+/* The initial >> 39 is hoisted out of the loop by the RTL loop passes.
169+ When the 39 is propagated into the variable shift code, this results
170+ in an immediate left shift by -7 (assuming no native 64-bit shift operator).
171+ This will eventially be eliminated by DCE, but lasts long enough to
172+ cause problems in the Thumb-2 shift-arith patterns. */
173+/* { dg-do compile } */
174+/* { dg-options "-O2" } */
175+long long
176+test (long long pat, long long mask, int n)
177+{
178+ long long z = 0;
179+ int i;
180+
181+ for (i = 39; i < 62; i++)
182+ z += (pat >> i) + mask ^ mask;
183+
184+ return z;
185+}

Subscribers

People subscribed via source and target branches