This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RFA: Avoid automodification in ARM call addresses


"arm-eabi-gcc -O2 -fno-omit-frame-pointer" miscompiles the
following loop:

    typedef void (*callback) (void);
    static void
    foo (callback *first, callback *p)
    {
      while (p > first)
        {
          (*--p) ();
          asm ("" : "=r" (p) : "0" (p)
               : "r4", "r5", "r6", "r7", "r8", "r9", "r10");
        }   
    }

The problem is that we have a call insn of the form:

    (call (mem (mem (pre_inc (reg pseudo)))) ...)

and (reg pseudo) gets spilled.  The call therefore requires
an output reload to write back the new value of (reg pseudo).
I think this is a constraint violation on the part of arm.md;
calls (like jumps) are not allowed to have output reloads.

In the testcase, we end up with a loop in which the new value is not
written back at all.  The loop will therefore continually call the last
callback in the array:

.L2:
        str     r3, [fp, #-56]
        ldr     ip, [r3, #-4]!
        mov     lr, pc
        bx      ip
        ldr     r2, [fp, #-64]
        ldr     r3, [fp, #-56]
        cmp     r3, r2
        bhi     .L2

(FWIW, I originally saw this on a 3.4-era compiler, where it failed
even without the asm or the addition of -fno-omit-frame-pointer.)

Tested on arm-eabi.  OK to install?

Richard


	* gcc/config/arm/arm.md (*call_mem): Exclude automodified addresses.
	(*call_value_mem): Likewise.

testsuite/
	* gcc.dg/20051206-1.c: New file.

Index: gcc/config/arm/arm.md
===================================================================
--- gcc/config/arm/arm.md	(revision 108106)
+++ gcc/config/arm/arm.md	(working copy)
@@ -7550,12 +7560,15 @@ (define_insn "*call_reg_arm"
    (set_attr "type" "call")]
 )
 
+;; Exclude automodified addresses.  If the address register is spilled,
+;; reload will want to use an output reload to write back the new value.
 (define_insn "*call_mem"
   [(call (mem:SI (match_operand:SI 0 "memory_operand" "m"))
 	 (match_operand 1 "" ""))
    (use (match_operand 2 "" ""))
    (clobber (reg:SI LR_REGNUM))]
-  "TARGET_ARM"
+  "TARGET_ARM
+   && GET_RTX_CLASS (GET_CODE (XEXP (operands[0], 0))) != RTX_AUTOINC"
   "*
   return output_call_mem (operands);
   "
@@ -7643,13 +7656,17 @@ (define_insn "*call_value_reg_arm"
    (set_attr "type" "call")]
 )
 
+;; Exclude automodified addresses.  If the address register is spilled,
+;; reload will want to use an output reload to write back the new value.
 (define_insn "*call_value_mem"
   [(set (match_operand 0 "" "")
 	(call (mem:SI (match_operand:SI 1 "memory_operand" "m"))
 	      (match_operand 2 "" "")))
    (use (match_operand 3 "" ""))
    (clobber (reg:SI LR_REGNUM))]
-  "TARGET_ARM && (!CONSTANT_ADDRESS_P (XEXP (operands[1], 0)))"
+  "TARGET_ARM
+    && GET_RTX_CLASS (GET_CODE (XEXP (operands[1], 0))) != RTX_AUTOINC
+    && (!CONSTANT_ADDRESS_P (XEXP (operands[1], 0)))"
   "*
   return output_call_mem (&operands[1]);
   "
Index: gcc/testsuite/gcc.dg/20051206-1.c
===================================================================
--- gcc/testsuite/gcc.dg/20051206-1.c	(revision 0)
+++ gcc/testsuite/gcc.dg/20051206-1.c	(revision 0)
@@ -0,0 +1,31 @@
+/* { dg-do run { target arm*-*-* } } */
+/* { dg-options "-O2 -fno-omit-frame-pointer" } */
+extern void abort (void);
+typedef void (*callback) (void);
+
+static void
+foo (callback *first, callback *p)
+{
+  while (p > first)
+    {
+      (*--p) ();
+      asm ("" : "=r" (p) : "0" (p)
+	   : "r4", "r5", "r6", "r7", "r8", "r9", "r10");
+    }   
+}
+
+static void
+dummy (void)
+{
+  static int count;
+  if (count++ == 1)
+    abort ();
+}
+
+int
+main (void)
+{
+  callback list[1] = { dummy };
+  foo (&list[0], &list[1]);
+  return 0;
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]