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]

Re: PR 27073: invalid gcse manipulation of REG_EQUIV notes


Richard Sandiford wrote:
This patch fixes PR rtl-optimization/27073, which is about
"arm-eabi-gcc -Os -mthumb" producing wrong code for the following
testcase:

     void __attribute__((noinline))
     foo (int *p, int d1, int d2, int d3,
          short count, int s1, int s2, int s3, int s4, int s5)
     {
       int n = count;
       while (n--)
         {
           *p++ = s1;
           *p++ = s2;
           *p++ = s3;
           *p++ = s4;
           *p++ = s5;
         }
     }

The important things about the testcase are:

    (a) There must be a subword stack parameter P1 that we do not assume
        is already promoted.  This is "count" in the testcase above.

    (b) There must be later stack parameters whose pseudos do not get
        allocated a hard register.

Because Thumb doesn't have subword stack loads, the sequence to set up
P1's pseudo register must first load the argument pointer into a pseudo
(let's call it PA).  The gcse local-prop pass then replaces the argument
pointer with PA in the instructions related to later stack parameters.
The problem is that gcse does this in both the instruction bodies _and_
their REG_EQUIV notes, so we get things like:

     (set (reg P2) (mem (plus (reg PA) (const_int 4))))
     REG_EQUIV: (mem (plus (reg PA) (const_int 4)))

The change to the insn body is almost immediately undone by CSE,
but the REG_EQUIV note remains.

The change to the note is bogus though.  A REG_EQUIV note is supposed
to say that all occurences of the register could be replaced with the
equivalent expression without affecting semantics.  However, when
gcse.c:try_replace_reg replaces FROM with TO in INSN, the equivalence
between FROM and TO is a local rather than a global property.

I'm not sure the replacement would be valid even if the equivalence
were global though.  Nothing ensures (or is supposed to ensure) that
PA's lifetime is extended to cover the lifetime of P2.

I therefore think the right fix is to restrict try_replace_reg
to REG_EQUAL rather than REG_EQUIV notes, which is what the patch
below does.  I looked at the other users of find_reg_equal_equiv_note
in gcse.c, but try_replace_reg seemed to be the only suspicious one.

Bootstrapped & regression tested on i686-pc-linux-gnu.  Also regression
tested on arm-eabi.  OK to install?

(2) above means that this is bound to be a regression for _some_
testcase, due to differences in register allocation.  I just don't
know which testcase.  I'll leave it to others to decide whether
that's sufficient grounds for applying it to 4.1. ;)

Richard


PR rtl-optimization/27073 * gcse.c (try_replace_reg): Just propagate into REG_EQUAL notes, not REG_EQUIVs.

gcc/testsuite/
	* gcc.c-torture/execute/pr27073.c: New test.

Index: gcc/testsuite/gcc.c-torture/execute/pr27073.c
===================================================================
--- gcc/testsuite/gcc.c-torture/execute/pr27073.c (revision 0)
+++ gcc/testsuite/gcc.c-torture/execute/pr27073.c (revision 0)
@@ -0,0 +1,25 @@
+void __attribute__((noinline))
+foo (int *p, int d1, int d2, int d3,
+ short count, int s1, int s2, int s3, int s4, int s5)
+{
+ int n = count;
+ while (n--)
+ {
+ *p++ = s1;
+ *p++ = s2;
+ *p++ = s3;
+ *p++ = s4;
+ *p++ = s5;
+ }
+}
+
+int main()
+{
+ int x[10], i;
+
+ foo (x, 0, 0, 0, 2, 100, 200, 300, 400, 500);
+ for (i = 0; i < 10; i++)
+ if (x[i] != (i % 5 + 1) * 100)
+ abort ();
+ exit (0);
+}
Index: gcc/gcse.c
===================================================================
--- gcc/gcse.c (revision 112700)
+++ gcc/gcse.c (working copy)
@@ -2642,7 +2642,7 @@ find_used_regs (rtx *xptr, void *data AT
static int
try_replace_reg (rtx from, rtx to, rtx insn)
{
- rtx note = find_reg_equal_equiv_note (insn);
+ rtx note = find_reg_note (insn, REG_EQUAL, NULL);
rtx src = 0;
int success = 0;
rtx set = single_set (insn);
This also fixes PR25595

Thanks

Khem


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