Patchwork [ARM] More strictly validate LDRD/STRD peepholes

login
register
mail settings
Submitter Richard Earnshaw
Date Dec. 9, 2013, 2:58 p.m.
Message ID <52A5DA87.4060800@arm.com>
Download mbox | patch
Permalink /patch/299078/
State New
Headers show

Comments

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

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;