[Bug target/34302] New: [4.3 regression] Invalid code reordering

[Bug target/34302] New: [4.3 regression] Invalid code reordering

[schwab at suse dot de]

((c_addr)=(Char *)(sp[2]));
((u1)=(UCell)(sp[1]));
((wfileid)=(Cell)((sp[0])));
sp += -1;
{
struct Cellquad r = read_line(c_addr, u1, wfileid);

        lwz 6,0(19)      #* sp, wfileid         ((wfileid)=(Cell)((sp[0])));
        addi 19,19,-4    # sp, sp,              sp += -1;
        mr 29,19         # temp.1416, sp
        mr 28,19         # temp.1418, sp
        addi 3,1,72      #,,
        lwzu 4,8(29)     #,                     ((c_addr)=(Char *)(sp[2]));
        lwzu 5,4(28)     #,                     ((u1)=(UCell)(sp[1]));
        bl read_line     #


-- 
           Summary: [4.3 regression] Invalid code reordering
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: schwab at suse dot de
GCC target triplet: powerpc*-suse-linux


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302

[Bug target/34302] [4.3 regression] Invalid code reordering

[schwab at suse dot de]

------- Comment #1 from schwab at suse dot de  2007-11-30 13:21 -------
Created an attachment (id=14672)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=14672&action=view)
Preprocessed source

Compile with -O or higher.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302

[Bug rtl-optimization/34302] [4.3 regression] Invalid code reordering

[schwab at suse dot de]

------- Comment #2 from schwab at suse dot de  2007-11-30 14:16 -------
The bug is introduced during auto-inc-dec.


-- 

schwab at suse dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|target                      |rtl-optimization


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302

[Bug rtl-optimization/34302] [4.3 regression] Invalid code reordering

[zadeck at naturalbridge dot com]

------- Comment #3 from zadeck at naturalbridge dot com  2007-11-30 15:33 -------
32 or 64 bit?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302

[Bug rtl-optimization/34302] [4.3 regression] Invalid code reordering

[schwab at suse dot de]

------- Comment #4 from schwab at suse dot de  2007-11-30 15:37 -------
Both.  The testcase is for 32 bit, but the equivalent source for 64 bit
produces basically the same bad code.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302

[Bug rtl-optimization/34302] [4.3 regression] Invalid code reordering

[zadeck at naturalbridge dot com]

------- Comment #5 from zadeck at naturalbridge dot com  2007-12-01 13:55 -------
Created an attachment (id=14677)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=14677&action=view)
possible patch to fix this

andreas,

would you try this patch?  I will try to make a small test case if this works. 
I believe that this should fix the problem.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302

[Bug rtl-optimization/34302] [4.3 regression] Invalid code reordering

[schwab at suse dot de]

------- Comment #6 from schwab at suse dot de  2007-12-01 17:21 -------
Looks good.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302

[Bug rtl-optimization/34302] [4.3 regression] Invalid code reordering

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pinskia at gcc dot gnu dot
                   |                            |org
   Target Milestone|---                         |4.3.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302

[Bug rtl-optimization/34302] [4.3 regression] Invalid code reordering

[jakub at gcc dot gnu dot org]

------- Comment #7 from jakub at gcc dot gnu dot org  2007-12-10 18:01 -------
Created an attachment (id=14719)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=14719&action=view)
engine.i

Smaller testcase, will try to turn that into executable testcase later tonight.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302

[Bug rtl-optimization/34302] [4.3 regression] Invalid code reordering

[zadeck at naturalbridge dot com]

------- Comment #8 from zadeck at naturalbridge dot com  2007-12-10 18:15 -------
Subject: Re:  [4.3 regression] Invalid code reordering

jakub at gcc dot gnu dot org wrote:
> ------- Comment #7 from jakub at gcc dot gnu dot org  2007-12-10 18:01 -------
> Created an attachment (id=14719)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=14719&action=view)
>  --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=14719&action=view)
> engine.i
>
> Smaller testcase, will try to turn that into executable testcase later tonight.
>
>
>   
thanks

i have been busy with other things and this did not hit the top of the
stack yet.

kenny


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302

[Bug rtl-optimization/34302] [4.3 regression] Invalid code reordering

[jakub at gcc dot gnu dot org]

------- Comment #9 from jakub at gcc dot gnu dot org  2007-12-10 19:35 -------
Created an attachment (id=14721)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=14721&action=view)
gcc43-pr34302-test.patch

And here is an executable testcase, fails without your auto-inc-dec.c
fix for -O and higher with -m{32,64} on ppc*-linux, succeeds with it.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302

[Bug rtl-optimization/34302] [4.3 regression] Invalid code reordering

[zadeck at naturalbridge dot com]

------- Comment #10 from zadeck at naturalbridge dot com  2007-12-10 20:02 -------
Subject: Re:  [4.3 regression] Invalid code reordering

jakub at gcc dot gnu dot org wrote:
> ------- Comment #9 from jakub at gcc dot gnu dot org  2007-12-10 19:35 -------
> Created an attachment (id=14721)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=14721&action=view)
>  --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=14721&action=view)
> gcc43-pr34302-test.patch
>
> And here is an executable testcase, fails without your auto-inc-dec.c
> fix for -O and higher with -m{32,64} on ppc*-linux, succeeds with it.
>
>
>   
thanks, i will add this to my patch an get it approved.

kenny


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302

[Bug rtl-optimization/34302] [4.3 regression] Invalid code reordering

[zadeck at naturalbridge dot com]

------- Comment #11 from zadeck at naturalbridge dot com  2007-12-10 20:46 -------
Subject:  [4.3 regression] Invalid code reordering

This patch fixes where the move insn is inserted on pre increments.  it
had been inserted before the auto inc but this is not correct.  it needs
to replace the existing add to properly handle the case where the
operands of the add change between the add and the load.

Tested on ppc-32.  The patch will only effect architectures with pre
increment.

Ok for commit?

Jakub, note that i changed the name of your testcase from 20071012-1.c
to 20071210-1.c.
I believe that this is the correct.

Kenny


2007-12-10  Kenneth Zadeck <zadeck@naturalbridge.com>

    PR rtl-optimization/34302
    * auto-inc-dec.c (attempt_change): Change place where move is
    inserted.

2007-12-10  Jakub Jelinek  <jakub@redhat.com>

    PR rtl-optimization/34302
    * gcc.c-torture/execute/20071210-1.c: New test.

Index: auto-inc-dec.c
===================================================================
--- auto-inc-dec.c      (revision 130748)
+++ auto-inc-dec.c      (working copy)
@@ -550,7 +550,10 @@ attempt_change (rtx new_addr, rtx inc_re
   switch (inc_insn.form)
     {
     case FORM_PRE_ADD:
-      mov_insn = insert_move_insn_before (mem_insn.insn, 
+      /* Replace the addition with a move.  Do it at the location of
+        the addition since the operand of the addition may change
+        before the memory reference.  */
+      mov_insn = insert_move_insn_before (inc_insn.insn, 
                                          inc_insn.reg_res, inc_insn.reg0);
       move_dead_notes (mov_insn, inc_insn.insn, inc_insn.reg0);

@@ -673,7 +676,7 @@ try_merge (void)
     }

   /* Look to see if the inc register is dead after the memory
-     reference.  If it is do not do the combination.  */
+     reference.  If it is, do not do the combination.  */
   if (find_regno_note (last_insn, REG_DEAD, REGNO (inc_reg)))
     {
       if (dump_file)
Index: testsuite/gcc.c-torture/execute/20071210-1.c
===================================================================
--- testsuite/gcc.c-torture/execute/20071210-1.c        (revision 0)
+++ testsuite/gcc.c-torture/execute/20071210-1.c        (revision 0)
@@ -0,0 +1,67 @@
+/* PR rtl-optimization/34302 */
+
+extern void abort (void);
+
+struct S
+{
+  int n1, n2, n3, n4;
+};
+
+__attribute__((noinline)) struct S
+foo (int x, int y, int z)
+{
+  if (x != 10 || y != 9 || z != 8)
+    abort ();
+  struct S s = { 1, 2, 3, 4 };
+  return s;
+}
+
+__attribute__((noinline)) void **
+bar (void **u, int *v)
+{
+  void **w = u;
+  int *s = v, x, y, z;
+  void **p, **q;
+  static void *l[] = { &&lab1, &&lab1, &&lab2, &&lab3, &&lab4 };
+
+  if (!u)
+    return l;
+
+  q = *w++;
+  goto *q;
+lab2:
+  p = q;
+  q = *w++;
+  x = s[2];
+  y = s[1];
+  z = s[0];
+  s -= 1;
+  struct S r = foo (x, y, z);
+  s[3] = r.n1;
+  s[2] = r.n2;
+  s[1] = r.n3;
+  s[0] = r.n4;
+  goto *q;
+lab3:
+  p = q;
+  q = *w++;
+  s += 1;
+  s[0] = 23;
+lab1:
+  goto *q;
+lab4:
+  return 0;
+}
+
+int
+main (void)
+{
+  void **u = bar ((void **) 0, (int *) 0);
+  void *t[] = { u[2], u[4] };
+  int s[] = { 7, 8, 9, 10, 11, 12 };
+  if (bar (t, &s[1]) != (void **) 0
+      || s[0] != 4 || s[1] != 3 || s[2] != 2 || s[3] != 1
+      || s[4] != 11 || s[5] != 12)
+    abort ();
+  return 0;
+}


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302

[Bug rtl-optimization/34302] [4.3 regression] Invalid code reordering

[richard dot guenther at gmail dot com]

------- Comment #12 from richard dot guenther at gmail dot com  2007-12-10 20:56 -------
Subject: Re:  [4.3 regression] Invalid code reordering

On Dec 10, 2007 9:46 PM, Kenneth Zadeck <zadeck@naturalbridge.com> wrote:
> This patch fixes where the move insn is inserted on pre increments.  it
> had been inserted before the auto inc but this is not correct.  it needs
> to replace the existing add to properly handle the case where the
> operands of the add change between the add and the load.
>
> Tested on ppc-32.  The patch will only effect architectures with pre
> increment.
>
> Ok for commit?

This is ok.

Thanks,
Richard.

> Jakub, note that i changed the name of your testcase from 20071012-1.c
> to 20071210-1.c.
> I believe that this is the correct.
>
> Kenny
>
>
> 2007-12-10  Kenneth Zadeck <zadeck@naturalbridge.com>
>
>     PR rtl-optimization/34302
>     * auto-inc-dec.c (attempt_change): Change place where move is
>     inserted.
>
> 2007-12-10  Jakub Jelinek  <jakub@redhat.com>
>
>     PR rtl-optimization/34302
>     * gcc.c-torture/execute/20071210-1.c: New test.
>
>


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302

[Bug rtl-optimization/34302] [4.3 regression] Invalid code reordering

[zadeck at gcc dot gnu dot org]

------- Comment #13 from zadeck at gcc dot gnu dot org  2007-12-10 21:32 -------
Subject: Bug 34302

Author: zadeck
Date: Mon Dec 10 21:31:59 2007
New Revision: 130751

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=130751
Log:
2007-12-10  Kenneth Zadeck <zadeck@naturalbridge.com>

        PR rtl-optimization/34302
        * auto-inc-dec.c (attempt_change): Change place where move is
        inserted.

2007-12-10  Jakub Jelinek  <jakub@redhat.com>

        PR rtl-optimization/34302
        * gcc.c-torture/execute/20071210-1.c: New test.


Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/20071210-1.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/auto-inc-dec.c
    trunk/gcc/testsuite/ChangeLog


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302

[Bug rtl-optimization/34302] [4.3 regression] Invalid code reordering

[zadeck at naturalbridge dot com]

------- Comment #14 from zadeck at naturalbridge dot com  2007-12-10 21:32 -------
Subject: Re:  [4.3 regression] Invalid code reordering

Richard Guenther wrote:
> On Dec 10, 2007 9:46 PM, Kenneth Zadeck <zadeck@naturalbridge.com> wrote:
>   
>> This patch fixes where the move insn is inserted on pre increments.  it
>> had been inserted before the auto inc but this is not correct.  it needs
>> to replace the existing add to properly handle the case where the
>> operands of the add change between the add and the load.
>>
>> Tested on ppc-32.  The patch will only effect architectures with pre
>> increment.
>>
>> Ok for commit?
>>     
>
> This is ok.
>
> Thanks,
> Richard.
>
>   
>> Jakub, note that i changed the name of your testcase from 20071012-1.c
>> to 20071210-1.c.
>> I believe that this is the correct.
>>
>> Kenny
>>
>>
>> 2007-12-10  Kenneth Zadeck <zadeck@naturalbridge.com>
>>
>>     PR rtl-optimization/34302
>>     * auto-inc-dec.c (attempt_change): Change place where move is
>>     inserted.
>>
>> 2007-12-10  Jakub Jelinek  <jakub@redhat.com>
>>
>>     PR rtl-optimization/34302
>>     * gcc.c-torture/execute/20071210-1.c: New test.
>>
>>
>>     
committed as revision 130751

Kenny


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302

[Bug rtl-optimization/34302] [4.3 regression] Invalid code reordering

[zadeck at naturalbridge dot com]

------- Comment #15 from zadeck at naturalbridge dot com  2007-12-10 21:33 -------
committed


-- 

zadeck at naturalbridge dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34302