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: -frename-registers and COND_EXECs


This patch stops regrename from making an invalid transformation in
the presence of conditionally-executed instructions.  The problem
shows up as a verify_local_live_at_start failure for the attached
testcase on arm-elf.

If an instruction is conditionally executed, regrename will treat
each output operand as OP_INOUT:

	  predicated = GET_CODE (PATTERN (insn)) == COND_EXEC;
	  for (i = 0; i < n_ops; ++i)
	    {
              [...]
	      if (...
	          || (predicated && recog_data.operand_type[i] == OP_OUT))
		recog_data.operand_type[i] = OP_INOUT;
	    }

...thereby indicating that the value is live before and after the
instruction.  This is good.  However, the code doesn't trigger for
the ARM ldm/stm patterns, whose output operands have no constraints
(meaning that the operands are not marked as OP_OUT to begin with).
For example, the two-register ldm pattern looks like this:

    (define_insn "*ldmsi2"
      [(match_parallel 0 "load_multiple_operation"
        [(set (match_operand:SI 2 "arm_hard_register_operand" "")
              (mem:SI (match_operand:SI 1 "s_register_operand" "r")))
         (set (match_operand:SI 3 "arm_hard_register_operand" "")
              (mem:SI (plus:SI (match_dup 1) (const_int 4))))])]
      "TARGET_ARM && XVECLEN (operands[0], 0) == 2"
      "ldm%?ia\\t%1, {%2, %3}"
      [(set_attr "type" "load2")
       (set_attr "predicable" "yes")]
    )

Going back to the testcase, the rtl before regrename looks like this:

   A: (set (reg:SI r3) (mem:SI ...))
           ^^^^^^^^^^^
   B: (set (reg:CC cc) (compare:CC (reg:SI r3) (const_int 0)))
                                   ^^^^^^^^^^^
   C: (cond_exec (le (reg:CC cc) (const_int 0))
         (parallel
            [(set (reg:SI r0) (mem:SI ...))
             (set (reg:SI r1) (mem:SI ...))
             (set (reg:SI r2) (mem:SI ...))
             (set (reg:SI r3) (mem:SI ...))]))
                  ^^^^^^^^^^^
   D: (cond_exec (le (reg:CC cc) (const_int 0))
         (parallel
            [(set (mem:SI ...) (reg:SI r0))
             (set (mem:SI ...) (reg:SI r1))
             (set (mem:SI ...) (reg:SI r2))
             (set (mem:SI ...) (reg:SI r3))]))
                               ^^^^^^^^^^^
   E: (cond_exed (gt (reg:CC cc) (const_int 0))
         (set (reg:SI ...) (... (reg:SI r3) ...)))
                                ^^^^^^^^^^^

and, when processing C, scan_rtx terminates r3's chain with terminate_write,
just as it would if C were unconditional.  The pass hen goes on to rename
r3 in A and B without changing the use in E.

The ldm patterns don't have output constraints because the operands have
to be consecutive and in ascending order.  If the constraints were changed
to "=r", regrename would feel free to rename one of the output operands
in isolation.

A similar problem occured for call insns on ia64:

    http://gcc.gnu.org/ml/gcc-patches/2004-04/msg01328.html

and that was fixed by adding "=X" constraints.  However, regrename
knows that calls are special:

	  /* ??? Many targets have output constraints on the SET_DEST
	     of a call insn, which is stupid, since these are certainly
	     ABI defined hard registers.  Don't change calls at all.

and I didn't think a similar fix was appropriate here.  The patch
instead treats the destinations of conditionally-executed SETs and
CLOBBERs as OP_INOUT regardless of whether they're part of a canonical
output operand.

Tested on arm-elf, no regressions.  That probably isn't saying much now
that -frename-registers is disabled by default, but the patch was also
tested on an early 3.4 derivative in which -frename-registers was still
enabled at -O3.  OK to install?

Richard


	* regrename.c (scan_rtx): Treat the destinations of SETs and CLOBBERs
	as OP_INOUT if the instruction is predicated.

testsuite/
	* gcc.dg/20041103-1.c: New test.

Index: regrename.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/regrename.c,v
retrieving revision 1.89
diff -u -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.89 regrename.c
--- regrename.c	12 Oct 2004 19:28:55 -0000	1.89
+++ regrename.c	4 Nov 2004 08:59:21 -0000
@@ -667,7 +667,8 @@ scan_rtx (rtx insn, rtx *loc, enum reg_c
 
     case SET:
       scan_rtx (insn, &SET_SRC (x), cl, action, OP_IN, 0);
-      scan_rtx (insn, &SET_DEST (x), cl, action, OP_OUT, 0);
+      scan_rtx (insn, &SET_DEST (x), cl, action,
+		GET_CODE (PATTERN (insn)) == COND_EXEC ? OP_INOUT : OP_OUT, 0);
       return;
 
     case STRICT_LOW_PART:
@@ -692,7 +693,8 @@ scan_rtx (rtx insn, rtx *loc, enum reg_c
       gcc_unreachable ();
 
     case CLOBBER:
-      scan_rtx (insn, &SET_DEST (x), cl, action, OP_OUT, 1);
+      scan_rtx (insn, &SET_DEST (x), cl, action,
+		GET_CODE (PATTERN (insn)) == COND_EXEC ? OP_INOUT : OP_OUT, 0);
       return;
 
     case EXPR_LIST:
diff -u /dev/null testsuite/gcc.dg/20041103-1.c
--- /dev/null	2003-09-15 14:40:47.000000000 +0100
+++ testsuite/gcc.dg/20041103-1.c	2004-11-04 08:58:13.000000000 +0000
@@ -0,0 +1,17 @@
+/* { dg-options "-O2 -frename-registers -fno-schedule-insns" } */
+
+void *memcpy (void *, const void *, __SIZE_TYPE__);
+
+void f (int n, int (*x)[4])
+{
+  while (n--)
+    {
+      int f = x[0][0];
+      if (f <= 0)
+	memcpy (&x[1], &x[0], sizeof (x[0]));
+      else
+	memcpy (&x[f], &x[0], sizeof (x[0]));
+      f = x[0][2];
+      x[0][1] = f;
+    }
+}


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