Patchwork ARM 64-bit shift ICE

login
register
mail settings
Submitter Paul Brook
Date Sept. 19, 2011, 9:12 a.m.
Message ID <201109191012.09414.paul@codesourcery.com>
Download mbox | patch
Permalink /patch/115312/
State New
Headers show

Comments

Paul Brook - Sept. 19, 2011, 9:12 a.m.
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.
Stubbs, Andrew - Sept. 19, 2011, 1:35 p.m.
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

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")
 {