[Bug target/35907] New: [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[Bug target/35907] New: [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[jakub at gcc dot gnu dot org]

build_trtable function from posix/regex.c is miscompiled on ppc64-linux with
-mcpu=power6.  The $v31 register is used within the function and is therefore
saved in prologue and restored in epilogue, but as the function uses alloca,
it is restored from different memory slot than it was saved into.
Options used:
-fpreprocessed -quiet -m64 -mcpu=power6 -mno-minimal-toc -mnew-mnemonics
-mlong-double-128 -g -O3 -std=gnu99 -fgnu89-inline -fasynchronous-unwind-tables
-fmerge-all-constants -fpic
The interesting lines in the assembly are:
.build_trtable:
...
        li 0,320
...
        stdu 1,-496(1)
        stvx 31,1,0
        mfvrsave 0
        mr 31,1
...
        ld 0,0(1)
...
        stdu 0,-12304(1)
...
        li 0,320
        lvx 31,1,0
        ld 1,0(1)
...
        blr

The above is with the trunk gcc, but 4.3 is similar.
Unlike this, gcc 4.1 saved the vector register early:
        li 0,-176
        stvx 31,1,0
as first two insns in the routine, before first stdu, and restored after
incrementing stack pointer back:
        ld 1,0(1)
        mr 3,0
        li 0,-176
        lwz 12,-148(1)
        lvx 31,1,0

So, with 4.1.x this works, but with 4.3/trunk $v31 will get the value of a
memory slot 12304 bytes (i.e. the size of alloca) below where it was actually
saved.


-- 
           Summary: [4.3/4.4 Regression] 64-bit power6 glibc miscompilation
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: jakub at gcc dot gnu dot org
GCC target triplet: powerpc64-linux


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #1 from jakub at gcc dot gnu dot org  2008-04-11 14:01 -------
Created an attachment (id=15466)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=15466&action=view)
regex.i.bz2

bzip2ed testcase


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #2 from jakub at gcc dot gnu dot org  2008-04-11 14:25 -------
Smaller testcase:
/* { dg-do run } */
/* { dg-options "-O2 -mcpu=power6" } */

#define vector __attribute__((vector_size (16)))
union
{
  vector int k;
  int c[16];
} u, v, w;
vector int m;

void __attribute__((noinline))
bar (void *i, vector int j)
{
  asm volatile ("" : : "r" (i), "r" (&j) : "memory");
}

int __attribute__((noinline))
foo (int i, vector int j)
{
  char *p = __builtin_alloca (64 + i);
  m += u.k;
  v.k = m;
  w.k = j;
  if (__builtin_memcmp (&v.c, &w.c, 16) != 0)
    __builtin_abort ();
  j += u.k;
  bar (p, j);
  j += u.k;
  bar (p, j);
  return 0;
}

int
main (void)
{
  vector int l;
  int i;
  for (i = 0; i < 4; i++)
    u.c[i] = i;
  l = u.k;
  if (foo (64, l))
    __builtin_abort ();
  l += u.k;
  if (foo (64, l))
    __builtin_abort ();
  return 0;
}


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #3 from jakub at gcc dot gnu dot org  2008-04-11 14:31 -------
On this smaller testcase and supposedly on the large testcase too:
@@ -90,7 +90,7 @@ foo:
        li 3,0
        lvx 30,1,0
        li 0,128
-       lvx 31,1,0
+       lvx 31,31,0
        ld 1,0(1)
        lwz 12,-28(1)
        mtvrsave 12
fixes this (r31 is frame pointer, set to r1 around the stvx that saved v31,
but at the lvx 31,1,0 instruction r1 is r31 decreased by alloca calls).


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[pinskia at gcc dot gnu dot org]

------- Comment #4 from pinskia at gcc dot gnu dot org  2008-04-11 16:26 -------
I wonder if this was caused by:
2007-05-16  Eric Christopher  <echristo@apple.com>

        * config/rs6000/rs6000.c (rs6000_emit_prologue): Move altivec register
        saving after stack push. Set sp_offset whenever we push.
        (rs6000_emit_epilogue): Move altivec register restore before
        stack push.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pinskia at gcc dot gnu dot
                   |                            |org
           Keywords|                            |wrong-code
   Target Milestone|---                         |4.3.1


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[bergner at gcc dot gnu dot org]

------- Comment #5 from bergner at gcc dot gnu dot org  2008-04-11 17:20 -------
Created an attachment (id=15467)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=15467&action=view)
Restore altivec regs after frame pointer setup

I agree with Andrew, it looks like Eric's patch moved the restoring of the
altivec registers to before the frame pointer code.  The attached (non
bootstrapped or regtested) fixes the problem for me.  Does this look ok to
everyone?


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[bergner at gcc dot gnu dot org]

------- Comment #6 from bergner at gcc dot gnu dot org  2008-04-11 17:33 -------
FYI, it results in the updated asm:

--- glibc02-bad.s       2008-04-11 12:21:48.000000000 -0500
+++ glibc02-good.s      2008-04-11 12:21:33.000000000 -0500
@@ -66,12 +66,12 @@
        mr 3,30
        vadduwm 2,31,2
        bl bar
-       li 0,16
+       li 0,-64
        li 3,0
        lwz 11,0(1)
-       lvx 30,1,0
-       li 0,32
-       lvx 31,1,0
+       lvx 30,11,0
+       li 0,-48
+       lvx 31,11,0
        lwz 12,-16(11)
        mtvrsave 12
        lwz 0,4(11)


-- 

bergner at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2008-04-11 17:33:25
               date|                            |


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #7 from jakub at gcc dot gnu dot org  2008-04-11 18:37 -------
Is it safe to have live data in memory below stack pointer on ppc/ppc64?


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[pinskia at gcc dot gnu dot org]

------- Comment #8 from pinskia at gcc dot gnu dot org  2008-04-11 19:07 -------
(In reply to comment #7)
> Is it safe to have live data in memory below stack pointer on ppc/ppc64?

powerpc64-*-* yes , powerpc-linux-gnu no.  the SysV abi does not define a red
zone.

--Pinski


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[bergner at gcc dot gnu dot org]

------- Comment #9 from bergner at gcc dot gnu dot org  2008-04-11 19:08 -------
This isn't accessing data below the stack pointer, it's accessing below the
previous stack pointer value which is in the current stack frame.

>From a technical standpoint, the ppc64 ABI allows us to access some number of
bytes (144?) below the stack pointer, but the ppc32 ABI does not.


-- 

bergner at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |bergner at gcc dot gnu dot
                   |dot org                     |org
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2008-04-11 17:33:25         |2008-04-11 19:08:38
               date|                            |


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #10 from jakub at gcc dot gnu dot org  2008-04-11 19:19 -------
Well, for -m64 the code in between the old and new Altivec reg restoring spot
when use_backchain_to_restore_sp will ld 1,0(1).  Is altivec_save_offset
guaranteed to be not lower than 288 bytes below this?  For -m32, I see that it
doesn't restore sp right away when alloca is used and the info->push_p
increment
doesn't apply to ABI_V4 either.


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[bergner at gcc dot gnu dot org]

------- Comment #11 from bergner at gcc dot gnu dot org  2008-04-11 22:04 -------
Hacking the test case to use up more stack space, I did get it to access more
than 288 bytes below the stack frame for -m64, so we obviously need something
more here.


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[amodra at bigpond dot net dot au]

------- Comment #12 from amodra at bigpond dot net dot au  2008-04-14 07:19 -------
Created an attachment (id=15476)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=15476&action=view)
proposed patch

I think we want something like the attached patch (as yet untested).  Besides
the epilogue bug exposed by alloca, there is another bug with vrsave.  If all
non-volatile gp and fp regs are saved/restored by a function, then vrsave will
be stored at frame-288-4 for 64-bit.  After sp is adjusted, this location may
be trashed by an interrupt routine.


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #13 from jakub at gcc dot gnu dot org  2008-04-14 07:30 -------
Couldn't it use hard_frame_pointer_rtx if frame_pointer_needed, to avoid the
load from 0(1) ?


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[amodra at bigpond dot net dot au]

------- Comment #14 from amodra at bigpond dot net dot au  2008-04-14 10:25 -------
Yes, I think you could use hard_frame_pointer_rtx in rs6000_emit_epilogue if
frame_pointer_needed, anywhere up to the point where gprs are restored. 
Probably done best as a separate patch.


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[jakub at gcc dot gnu dot org]

-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #15 from jakub at gcc dot gnu dot org  2008-04-15 12:34 -------
Ok, let's leave that for a follow-up.  Have you managed to test this?
I could bootstrap/regtest on ppc-linu and ppc64-linux, but don't have time
right now for further testing.


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[bergner at gcc dot gnu dot org]

------- Comment #16 from bergner at gcc dot gnu dot org  2008-04-15 14:36 -------
I'll fire off a bootstrap and regtest of Alan's patch on powerpc64-linux and
running the test suite in 32-bit and 64-bit modes (ie, -m32 and -m64).


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[bergner at gcc dot gnu dot org]

------- Comment #17 from bergner at gcc dot gnu dot org  2008-04-15 21:16 -------
Created an attachment (id=15480)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=15480&action=view)
Updated test case ready for inclusion in gcc's testsuite.

Ok, I bootstrapped (powerpc64-linux) and regtested Alan's patch and saw no
regressions in either 32-bit or 64-bit modes.  I have also attached an updated
version of Jakub's test case ready for inclusion into GCC's testsuite.


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[amodra at bigpond dot net dot au]

------- Comment #18 from amodra at bigpond dot net dot au  2008-04-16 00:41 -------
My bootstrap and regression tests also pass.  It took a while...  I didn't have
a recent baseline to compare against so needed to do two bootstraps.


-- 

amodra at bigpond dot net dot au changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P1                          |P3


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #19 from jakub at gcc dot gnu dot org  2008-04-17 07:20 -------
Bootstrapped/regtested on gcc-4_3-branch as well.


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[amodra at gcc dot gnu dot org]

------- Comment #21 from amodra at gcc dot gnu dot org  2008-04-17 12:28 -------
Subject: Bug 35907

Author: amodra
Date: Thu Apr 17 12:27:56 2008
New Revision: 134388

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=134388
Log:
        PR target/35907
        * config/rs6000/rs6000.c (rs6000_emit_epilogue): Restore vr and vrsave
        regs before frame pop when needed.  If use_backchain_to_restore_sp
        then load backchain into a temp reg to restore vr and vrsave.  Add
        code to restore vr after frame pop if possible.


Modified:
    branches/gcc-4_3-branch/gcc/ChangeLog
    branches/gcc-4_3-branch/gcc/config/rs6000/rs6000.c


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[amodra at gcc dot gnu dot org]

------- Comment #20 from amodra at gcc dot gnu dot org  2008-04-17 12:28 -------
Subject: Bug 35907

Author: amodra
Date: Thu Apr 17 12:27:31 2008
New Revision: 134387

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=134387
Log:
        PR target/35907
        * config/rs6000/rs6000.c (rs6000_emit_epilogue): Restore vr and vrsave
        regs before frame pop when needed.  If use_backchain_to_restore_sp
        then load backchain into a temp reg to restore vr and vrsave.  Add
        code to restore vr after frame pop if possible.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/rs6000.c


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[amodra at bigpond dot net dot au]

------- Comment #22 from amodra at bigpond dot net dot au  2008-04-17 12:29 -------
Fixed


-- 

amodra at bigpond dot net dot au changed:

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


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #23 from jakub at gcc dot gnu dot org  2008-04-18 19:12 -------
Subject: Bug 35907

Author: jakub
Date: Fri Apr 18 19:11:57 2008
New Revision: 134447

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=134447
Log:
        PR target/35907
        * gcc.target/powerpc/pr35907.c: New test.

Added:
    trunk/gcc/testsuite/gcc.target/powerpc/pr35907.c
Modified:
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug target/35907] [4.3/4.4 Regression] 64-bit power6 glibc miscompilation

[jakub at gcc dot gnu dot org]

------- Comment #24 from jakub at gcc dot gnu dot org  2008-04-18 19:14 -------
Subject: Bug 35907

Author: jakub
Date: Fri Apr 18 19:13:35 2008
New Revision: 134448

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=134448
Log:
        PR target/35907
        * gcc.target/powerpc/pr35907.c: New test.

Added:
    branches/gcc-4_3-branch/gcc/testsuite/gcc.target/powerpc/pr35907.c
Modified:
    branches/gcc-4_3-branch/gcc/testsuite/ChangeLog


-- 


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