diff mbox

ARM 64-bit shift ICE

Message ID 201109191012.09414.paul@codesourcery.com
State New
Headers show

Commit Message

Paul Brook Sept. 19, 2011, 9:12 a.m. UTC
The patch below fixes gcc.c-torture/execute/builtin-bitops-1.c on ARM/Thumb-2 
targets.

The story here is that ARM doesn't have a 64-bit shift operator, so these get 
decomposed into something like:

low1 = (low >> n)
low1 |= (high << (32 - n)) /**/
low2 = high >> (32 - n)
low_result = (n > 32) ? low2 : low1;

Under some circumstances (typically involving loop bounds) the RTL optimizers 
propagate a constant shift count into this sequence.  Combine then squishes 
the line marked /**/ into a single shift-and-or insn. If n > 32 then the shift 
count will be negative.  Of course this code is dead, but we don't figure that 
out until much later.

In ARM mode we have an "r" alternative so the negative shift count can be 
reloaded into a register. A bit lame, but DCE culls the whole thing before it 
can do any real damage.

In Thumb-2 mode we try and match an "M" constraint (const_int between 0 and 
31) with no other alternatives. This fails, and with nowhere else to go we 
ICE.

The fix is to restrict the predicate for these insns to only accept valid 
const_int shift counts.  This prevents combine forming the shift-or insn, and 
we proceed as before.

Tested on arm-none-eabi
Applied to svn.

Paul

2011-09-19  Paul Brook  <paul@codesourcery.com>

	gcc/
	* config/arm/predicates.md (shift_amount_operand): Check constant
	shift count is in range.
	(const_shift_operand): Remove.

Comments

Stubbs, Andrew Sept. 19, 2011, 1:35 p.m. UTC | #1
On 19/09/11 10:12, Paul Brook wrote:
> In Thumb-2 mode we try and match an "M" constraint (const_int between 0 and
> 31) with no other alternatives. This fails, and with nowhere else to go we
> ICE.
>
> The fix is to restrict the predicate for these insns to only accept valid
> const_int shift counts.  This prevents combine forming the shift-or insn, and
> we proceed as before.

This is the exact same problem I've been trying to fix here:

http://gcc.gnu.org/ml/gcc-patches/2011-09/msg00049.html

The fix was initially rejected for not being restrictive enough, and 
then my predicate patch was rejected because Richard Sandiford pointed 
out that the predicate must not be more restrictive than the constraints 
or else reload may introduce recog ICE.

I believe your patch has the exact same problem.

I didn't get around to fixing both problems satisfactorily yet, but I 
have something cooking.

Andrew
diff mbox

Patch

Index: gcc/config/arm/predicates.md
===================================================================
--- gcc/config/arm/predicates.md	(revision 178906)
+++ gcc/config/arm/predicates.md	(working copy)
@@ -132,7 +132,8 @@  (define_predicate "arm_rhsm_operand"
 (define_predicate "shift_amount_operand"
   (ior (and (match_test "TARGET_ARM")
 	    (match_operand 0 "s_register_operand"))
-       (match_operand 0 "const_int_operand")))
+       (and (match_code "const_int")
+	    (match_test "((unsigned HOST_WIDE_INT) INTVAL (op)) < 32"))))
 
 (define_predicate "arm_add_operand"
   (ior (match_operand 0 "arm_rhs_operand")
@@ -344,12 +345,6 @@  (define_predicate "soft_df_operand"
        (and (match_code "reg,subreg,mem")
 	    (match_operand 0 "nonimmediate_soft_df_operand"))))
 
-(define_predicate "const_shift_operand"
-  (and (match_code "const_int")
-       (ior (match_operand 0 "power_of_two_operand")
-	    (match_test "((unsigned HOST_WIDE_INT) INTVAL (op)) < 32"))))
-
-
 (define_special_predicate "load_multiple_operation"
   (match_code "parallel")
 {