diff mbox

[ARM] - Fix for PR56470

Message ID 513DC54F.90301@arm.com
State New
Headers show

Commit Message

Richard Earnshaw March 11, 2013, 11:51 a.m. UTC 
In PR56470 the compiler is failing an internal check on the operands to 
shift_operator after reload has completed.  This comes from reload 
spilling a constant shift value from a register-specified shift which is 
out-of-range, but permitted by the constraints due to the overloaded 
nature of this operation (shift_operator is intended to permit any shift 
by 0..31 or a multiply by a constant power of 2 -- it's really a bit of 
a hack, but it keeps the number of patterns in the machine description 
down significantly).[1]

Anyway, if reload spills out a constant that can match the constriant M, 
then it has no way of knowing if this is for a multiply or for a shift, 
so we have to just handle it.

Unfortunately, the attempt to tighten up the validation of user ASM 
blocks that mis-use %S output specifier, means we can now ICE internally.

The fix is to do the validation inside shift_op, rather than in the 
caller and to permit the out-of-range constants that reload can generate.

Tested on a native bootstrap, with no regressions.

R.

[1] Now that we have iterators, there is perhaps a better way of doing 
this, but that's a big change at this point and not suitable for 4.8.


	PR target/56470
	* arm.md (shift_op): Validate RTL pattern on the fly.
	(arm_print_operand, case 'S'): Don't use shift_operator
	to validate the RTL.
diff mbox

Patch

Index: arm.c
===================================================================
--- arm.c	(revision 196523)
+++ arm.c	(working copy)
@@ -15430,71 +15430,87 @@  shift_op (rtx op, HOST_WIDE_INT *amountp
   const char * mnem;
   enum rtx_code code = GET_CODE (op);
 
-  switch (GET_CODE (XEXP (op, 1)))
-    {
-    case REG:
-    case SUBREG:
-      *amountp = -1;
-      break;
-
-    case CONST_INT:
-      *amountp = INTVAL (XEXP (op, 1));
-      break;
-
-    default:
-      gcc_unreachable ();
-    }
-
   switch (code)
     {
     case ROTATE:
-      gcc_assert (*amountp != -1);
-      *amountp = 32 - *amountp;
-      code = ROTATERT;
+      if (!CONST_INT_P (XEXP (op, 1)))
+	{
+	  output_operand_lossage ("invalid shift operand");
+	  return NULL;
+	}
 
-      /* Fall through.  */
+      code = ROTATERT;
+      *amountp = 32 - INTVAL (XEXP (op, 1));
+      mnem = "ror";
+      break;
 
     case ASHIFT:
     case ASHIFTRT:
     case LSHIFTRT:
     case ROTATERT:
       mnem = arm_shift_nmem(code);
+      if (CONST_INT_P (XEXP (op, 1)))
+	{
+	  *amountp = INTVAL (XEXP (op, 1));
+	}
+      else if (REG_P (XEXP (op, 1)))
+	{
+	  *amountp = -1;
+	  return mnem;
+	}
+      else
+	{
+	  output_operand_lossage ("invalid shift operand");
+	  return NULL;
+	}
       break;
 
     case MULT:
       /* We never have to worry about the amount being other than a
 	 power of 2, since this case can never be reloaded from a reg.  */
-      gcc_assert (*amountp != -1);
+      if (!CONST_INT_P (XEXP (op, 1)))
+	{
+	  output_operand_lossage ("invalid shift operand");
+	  return NULL;
+	}
+
+      *amountp = INTVAL (XEXP (op, 1)) & 0xFFFFFFFF;
+
+      /* Amount must be a power of two.  */
+      if (*amountp & (*amountp - 1))
+	{
+	  output_operand_lossage ("invalid shift operand");
+	  return NULL;
+	}
+
       *amountp = int_log2 (*amountp);
       return ARM_LSL_NAME;
 
     default:
-      gcc_unreachable ();
+      output_operand_lossage ("invalid shift operand");
+      return NULL;
     }
 
-  if (*amountp != -1)
-    {
-      /* This is not 100% correct, but follows from the desire to merge
-	 multiplication by a power of 2 with the recognizer for a
-	 shift.  >=32 is not a valid shift for "lsl", so we must try and
-	 output a shift that produces the correct arithmetical result.
-	 Using lsr #32 is identical except for the fact that the carry bit
-	 is not set correctly if we set the flags; but we never use the
-	 carry bit from such an operation, so we can ignore that.  */
-      if (code == ROTATERT)
-	/* Rotate is just modulo 32.  */
-	*amountp &= 31;
-      else if (*amountp != (*amountp & 31))
-	{
-	  if (code == ASHIFT)
-	    mnem = "lsr";
-	  *amountp = 32;
-	}
-
-      /* Shifts of 0 are no-ops.  */
-      if (*amountp == 0)
-	return NULL;
-    }
+  /* This is not 100% correct, but follows from the desire to merge
+     multiplication by a power of 2 with the recognizer for a
+     shift.  >=32 is not a valid shift for "lsl", so we must try and
+     output a shift that produces the correct arithmetical result.
+     Using lsr #32 is identical except for the fact that the carry bit
+     is not set correctly if we set the flags; but we never use the
+     carry bit from such an operation, so we can ignore that.  */
+  if (code == ROTATERT)
+    /* Rotate is just modulo 32.  */
+    *amountp &= 31;
+  else if (*amountp != (*amountp & 31))
+    {
+      if (code == ASHIFT)
+	mnem = "lsr";
+      *amountp = 32;
+    }
+
+  /* Shifts of 0 are no-ops.  */
+  if (*amountp == 0)
+    return NULL;
 
   return mnem;
 }
@@ -17743,12 +17759,6 @@  arm_print_operand (FILE *stream, rtx x,
 	HOST_WIDE_INT val;
 	const char *shift;
 
-	if (!shift_operator (x, SImode))
-	  {
-	    output_operand_lossage ("invalid shift operand");
-	    break;
-	  }
-
 	shift = shift_op (x, &val);
 
 	if (shift)