[ARM] More strictly validate LDRD/STRD peepholes

Submitted by Richard Earnshaw on Dec. 9, 2013, 2:58 p.m.

Details

Message ID 52A5DA87.4060800@arm.com
State New
Headers show

Commit Message

Richard Earnshaw Dec. 9, 2013, 2:58 p.m.
This patch fixes a bug on ARM where we can end up generating invalid
addresses for the LDRD/STRD peepholes.  We then end up with an ICE later
on when we check the results.  The patch does more strict validation of
the addresses, including having better tests for side effects that we
can't handle.

Tested on arm-linux-gnueabi and applied to trunk.

R.

gcc/
	* arm.c (mem_ok_for_ldrd_strd): Rename first argument as MEM.
	Do more address validation checks.
gcc/testsuite/
	* gcc.target/arm/ldrd-strd-offset.c: New.

Patch hide | download patch | download mbox

Index: gcc/testsuite/gcc.target/arm/ldrd-strd-offset.c
===================================================================
--- gcc/testsuite/gcc.target/arm/ldrd-strd-offset.c	(revision 0)
+++ gcc/testsuite/gcc.target/arm/ldrd-strd-offset.c	(revision 0)
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+typedef struct
+{
+  int x;
+  int i, j;
+} off_struct;
+
+int foo (char *str, int *a, int b, int c)
+{
+  off_struct *p = (off_struct *)(str + 3);
+  b = p->i;
+  c = p->j;
+  *a = b + c;
+  return 0;
+}
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	(revision 205803)
+++ gcc/config/arm/arm.c	(working copy)
@@ -15169,28 +15169,37 @@  operands_ok_ldrd_strd (rtx rt, rtx rt2, 
 }
 
 /* Helper for gen_operands_ldrd_strd.  Returns true iff the memory
-   operand ADDR is an immediate offset from the base register and is
-   not volatile, in which case it sets BASE and OFFSET
-   accordingly.  */
-bool
-mem_ok_for_ldrd_strd (rtx addr, rtx *base, rtx *offset)
+   operand MEM's address contains an immediate offset from the base
+   register and has no side effects, in which case it sets BASE and
+   OFFSET accordingly.  */
+static bool
+mem_ok_for_ldrd_strd (rtx mem, rtx *base, rtx *offset)
 {
+  rtx addr;
+
+  gcc_assert (base != NULL && offset != NULL);
+
   /* TODO: Handle more general memory operand patterns, such as
      PRE_DEC and PRE_INC.  */
 
-  /* Convert a subreg of mem into mem itself.  */
-  if (GET_CODE (addr) == SUBREG)
-    addr = alter_subreg (&addr, true);
-
-  gcc_assert (MEM_P (addr));
+  if (side_effects_p (mem))
+    return false;
 
-  /* Don't modify volatile memory accesses.  */
-  if (MEM_VOLATILE_P (addr))
+  /* Can't deal with subregs.  */
+  if (GET_CODE (mem) == SUBREG)
     return false;
 
+  gcc_assert (MEM_P (mem));
+
   *offset = const0_rtx;
 
-  addr = XEXP (addr, 0);
+  addr = XEXP (mem, 0);
+
+  /* If addr isn't valid for DImode, then we can't handle it.  */
+  if (!arm_legitimate_address_p (DImode, addr,
+				 reload_in_progress || reload_completed))
+    return false;
+
   if (REG_P (addr))
     {
       *base = addr;