| Message ID | 1469093774-25485-1-git-send-email-aurelien@aurel32.net |
|---|---|
| State | Committed |
| Delegated to: | Tulio Magno Quites Machado Filho |
| Headers |
Received: (qmail 111881 invoked by alias); 21 Jul 2016 09:36:40 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 111868 invoked by uid 89); 21 Jul 2016 09:36:40 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=H*MI:send, 2016-07-21 X-HELO: hall.aurel32.net From: Aurelien Jarno <aurelien@aurel32.net> To: libc-alpha@sourceware.org Cc: Aurelien Jarno <aurelien@aurel32.net> Subject: [PATCH] powerpc: fix ifunc-sel.h with GCC 6 Date: Thu, 21 Jul 2016 11:36:14 +0200 Message-Id: <1469093774-25485-1-git-send-email-aurelien@aurel32.net> |
Commit Message
Aurelien Jarno
July 21, 2016, 9:36 a.m. UTC
On 32-bit PowerPC GCC 6 always saves the PIC register on the stack in the prologue and adjust the stack in the epilogue. It is therefore not possible anymore to just exit the function in the inline asm code, otherwise it corrupts the stack pointer. This causes the following tests to fail when using GCC 6: FAIL: elf/ifuncmain1 FAIL: elf/ifuncmain1pic FAIL: elf/ifuncmain1picstatic FAIL: elf/ifuncmain1pie FAIL: elf/ifuncmain1staticpic FAIL: elf/ifuncmain1staticpie FAIL: elf/ifuncmain1vis FAIL: elf/ifuncmain1vispic FAIL: elf/ifuncmain1vispie FAIL: elf/ifuncmain2pic FAIL: elf/ifuncmain2picstatic FAIL: elf/ifuncmain3 FAIL: elf/ifuncmain4picstatic FAIL: elf/ifuncmain5 FAIL: elf/ifuncmain5picstatic FAIL: elf/ifuncmain5staticpic The solution is to replace the beqlr instructions by a beq to the end of the inline asm code. This fixes all the above failures. ChangeLog: * sysdeps/powerpc/ifunc-sel.h (ifunc_sel): Replace beqlr instructions by beq instructions jumping to the end of the function. --- ChangeLog | 5 +++++ sysdeps/powerpc/ifunc-sel.h | 7 ++++--- 2 files changed, 9 insertions(+), 3 deletions(-)
Comments
On 2016-07-21 11:36, Aurelien Jarno wrote: > On 32-bit PowerPC GCC 6 always saves the PIC register on the stack in > the prologue and adjust the stack in the epilogue. It is therefore not > possible anymore to just exit the function in the inline asm code, > otherwise it corrupts the stack pointer. This causes the following tests > to fail when using GCC 6: > > FAIL: elf/ifuncmain1 > FAIL: elf/ifuncmain1pic > FAIL: elf/ifuncmain1picstatic > FAIL: elf/ifuncmain1pie > FAIL: elf/ifuncmain1staticpic > FAIL: elf/ifuncmain1staticpie > FAIL: elf/ifuncmain1vis > FAIL: elf/ifuncmain1vispic > FAIL: elf/ifuncmain1vispie > FAIL: elf/ifuncmain2pic > FAIL: elf/ifuncmain2picstatic > FAIL: elf/ifuncmain3 > FAIL: elf/ifuncmain4picstatic > FAIL: elf/ifuncmain5 > FAIL: elf/ifuncmain5picstatic > FAIL: elf/ifuncmain5staticpic > > The solution is to replace the beqlr instructions by a beq to the end > of the inline asm code. This fixes all the above failures. > > ChangeLog: > * sysdeps/powerpc/ifunc-sel.h (ifunc_sel): Replace beqlr instructions > by beq instructions jumping to the end of the function. > --- > ChangeLog | 5 +++++ > sysdeps/powerpc/ifunc-sel.h | 7 ++++--- > 2 files changed, 9 insertions(+), 3 deletions(-) I forgot to say that I have tested this patch on ppc, ppc64 and ppc64le. No regression in the testsuite.
On 07/21/2016 11:36 AM, Aurelien Jarno wrote: > On 32-bit PowerPC GCC 6 always saves the PIC register on the stack in > the prologue and adjust the stack in the epilogue. It is therefore not > possible anymore to just exit the function in the inline asm code, > otherwise it corrupts the stack pointer. This causes the following tests > to fail when using GCC 6: > : "=r" (ret) > : "X" (&global), "X" (f1), "X" (f2), "X" (f3)); > return ret; The inline assembly still lacks clobbers for registers 11 and 12, and the "X" constraint is incorrect (it should be "i", I think). Florian
On 07/21/2016 01:53 PM, Florian Weimer wrote: > On 07/21/2016 11:36 AM, Aurelien Jarno wrote: >> On 32-bit PowerPC GCC 6 always saves the PIC register on the stack in >> the prologue and adjust the stack in the epilogue. It is therefore not >> possible anymore to just exit the function in the inline asm code, >> otherwise it corrupts the stack pointer. This causes the following tests >> to fail when using GCC 6: > >> : "=r" (ret) >> : "X" (&global), "X" (f1), "X" (f2), "X" (f3)); >> return ret; > > The inline assembly still lacks clobbers for registers 11 and 12, and > the "X" constraint is incorrect (it should be "i", I think). And a comment why bcl 20,31 is used instead of bl wouldn't hurt, either. A colleague explained to me it's a hint to the implementation not to perform return stack optimization, although the observable effect is identical to bl. Florian
Florian Weimer <fweimer@redhat.com> writes: > And a comment why bcl 20,31 is used instead of bl wouldn't hurt, > either. That's the canonical way to setup a reference to the TOC. Andreas.
On 2016-07-21 13:53, Florian Weimer wrote: > On 07/21/2016 11:36 AM, Aurelien Jarno wrote: > > On 32-bit PowerPC GCC 6 always saves the PIC register on the stack in > > the prologue and adjust the stack in the epilogue. It is therefore not > > possible anymore to just exit the function in the inline asm code, > > otherwise it corrupts the stack pointer. This causes the following tests > > to fail when using GCC 6: > > > : "=r" (ret) > > : "X" (&global), "X" (f1), "X" (f2), "X" (f3)); > > return ret; > > The inline assembly still lacks clobbers for registers 11 and 12, and the > "X" constraint is incorrect (it should be "i", I think). I agree that this should be fixed, and I'll try to work on that. That said it should probably be done as an additional patch, my patch doesn't touch that part. Aurelien
On 07/21/2016 02:21 PM, Andreas Schwab wrote: > Florian Weimer <fweimer@redhat.com> writes: > >> And a comment why bcl 20,31 is used instead of bl wouldn't hurt, >> either. > > That's the canonical way to setup a reference to the TOC. Yes, I figured that out in the meantime. Is “bcl 20,31” always used for that? The second opcode argument doesn't really matter, after all. Florian
On 07/21/2016 02:57 PM, Aurelien Jarno wrote: > On 2016-07-21 13:53, Florian Weimer wrote: >> On 07/21/2016 11:36 AM, Aurelien Jarno wrote: >>> On 32-bit PowerPC GCC 6 always saves the PIC register on the stack in >>> the prologue and adjust the stack in the epilogue. It is therefore not >>> possible anymore to just exit the function in the inline asm code, >>> otherwise it corrupts the stack pointer. This causes the following tests >>> to fail when using GCC 6: >> >>> : "=r" (ret) >>> : "X" (&global), "X" (f1), "X" (f2), "X" (f3)); >>> return ret; >> >> The inline assembly still lacks clobbers for registers 11 and 12, and the >> "X" constraint is incorrect (it should be "i", I think). > > I agree that this should be fixed, and I'll try to work on that. That > said it should probably be done as an additional patch, my patch doesn't > touch that part. Your patch looks good to me, but one of the POWER maintainers should comment. Thanks, Florian
Florian Weimer <fweimer@redhat.com> writes: > Yes, I figured that out in the meantime. Is “bcl 20,31” always used for > that? The second opcode argument doesn't really matter, after all. It's what gcc uses. I don't know if there is any deeper meaning behind that particular encoding. Andreas.
Aurelien Jarno <aurelien@aurel32.net> writes: > On 32-bit PowerPC GCC 6 always saves the PIC register on the stack in > the prologue and adjust the stack in the epilogue. It is therefore not > possible anymore to just exit the function in the inline asm code, > otherwise it corrupts the stack pointer. This causes the following tests > to fail when using GCC 6: > > FAIL: elf/ifuncmain1 > FAIL: elf/ifuncmain1pic > FAIL: elf/ifuncmain1picstatic > FAIL: elf/ifuncmain1pie > FAIL: elf/ifuncmain1staticpic > FAIL: elf/ifuncmain1staticpie > FAIL: elf/ifuncmain1vis > FAIL: elf/ifuncmain1vispic > FAIL: elf/ifuncmain1vispie > FAIL: elf/ifuncmain2pic > FAIL: elf/ifuncmain2picstatic > FAIL: elf/ifuncmain3 > FAIL: elf/ifuncmain4picstatic > FAIL: elf/ifuncmain5 > FAIL: elf/ifuncmain5picstatic > FAIL: elf/ifuncmain5staticpic > > The solution is to replace the beqlr instructions by a beq to the end > of the inline asm code. This fixes all the above failures. > > ChangeLog: > * sysdeps/powerpc/ifunc-sel.h (ifunc_sel): Replace beqlr instructions > by beq instructions jumping to the end of the function. I'd prefer to remove this file. Alan, do we still need this file? > --- > ChangeLog | 5 +++++ > sysdeps/powerpc/ifunc-sel.h | 7 ++++--- > 2 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index 97c46a1..b18a8cd 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,8 @@ > +2016-07-21 Aurelien Jarno <aurelien@aurel32.net> > + > + * sysdeps/powerpc/ifunc-sel.h (ifunc_sel): Replace beqlr instructions > + by beq instructions jumping to the end of the function. > + > 2016-07-21 Szabolcs Nagy <szabolcs.nagy@arm.com> > > * sysdeps/aarch64/libm-test-ulps: Updated. > diff --git a/sysdeps/powerpc/ifunc-sel.h b/sysdeps/powerpc/ifunc-sel.h > index 526d8ed..79d110f 100644 > --- a/sysdeps/powerpc/ifunc-sel.h > +++ b/sysdeps/powerpc/ifunc-sel.h > @@ -17,13 +17,14 @@ ifunc_sel (int (*f1) (void), int (*f2) (void), int (*f3) (void)) > "addis %0,11,%2-1b@ha\n\t" > "addi %0,%0,%2-1b@l\n\t" > "cmpwi 12,1\n\t" > - "beqlr\n\t" > + "beq 2f\n\t" > "addis %0,11,%3-1b@ha\n\t" > "addi %0,%0,%3-1b@l\n\t" > "cmpwi 12,-1\n\t" > - "beqlr\n\t" > + "beq 2f\n\t" > "addis %0,11,%4-1b@ha\n\t" > - "addi %0,%0,%4-1b@l" > + "addi %0,%0,%4-1b@l\n\t" > + "2:" > : "=r" (ret) > : "X" (&global), "X" (f1), "X" (f2), "X" (f3)); > return ret; > -- > 2.8.1 >
On Thu, Jul 21, 2016 at 02:48:09PM -0300, Tulio Magno Quites Machado Filho wrote: > Aurelien Jarno <aurelien@aurel32.net> writes: > > * sysdeps/powerpc/ifunc-sel.h (ifunc_sel): Replace beqlr instructions > > by beq instructions jumping to the end of the function. > > I'd prefer to remove this file. > > Alan, do we still need this file? To do without it, I think you'd need to bump the glibc binutils requirement to 2.24, in order to have this linker fix: https://sourceware.org/ml/binutils/2013-03/msg00299.html
On 2016-07-22 13:06, Alan Modra wrote: > On Thu, Jul 21, 2016 at 02:48:09PM -0300, Tulio Magno Quites Machado Filho wrote: > > Aurelien Jarno <aurelien@aurel32.net> writes: > > > * sysdeps/powerpc/ifunc-sel.h (ifunc_sel): Replace beqlr instructions > > > by beq instructions jumping to the end of the function. > > > > I'd prefer to remove this file. > > > > Alan, do we still need this file? > > To do without it, I think you'd need to bump the glibc binutils > requirement to 2.24, in order to have this linker fix: > https://sourceware.org/ml/binutils/2013-03/msg00299.html In that case, one possibility is to support ifunc only when the installed binutils is at least 2.24. This is basically just changing the minimum version in the "Prevent building POWER8 code without support in assembler" patch. Aurelien
On 21/07/2016 14:48, Tulio Magno Quites Machado Filho wrote: > Aurelien Jarno <aurelien@aurel32.net> writes: > >> On 32-bit PowerPC GCC 6 always saves the PIC register on the stack in >> the prologue and adjust the stack in the epilogue. It is therefore not >> possible anymore to just exit the function in the inline asm code, >> otherwise it corrupts the stack pointer. This causes the following tests >> to fail when using GCC 6: >> >> FAIL: elf/ifuncmain1 >> FAIL: elf/ifuncmain1pic >> FAIL: elf/ifuncmain1picstatic >> FAIL: elf/ifuncmain1pie >> FAIL: elf/ifuncmain1staticpic >> FAIL: elf/ifuncmain1staticpie >> FAIL: elf/ifuncmain1vis >> FAIL: elf/ifuncmain1vispic >> FAIL: elf/ifuncmain1vispie >> FAIL: elf/ifuncmain2pic >> FAIL: elf/ifuncmain2picstatic >> FAIL: elf/ifuncmain3 >> FAIL: elf/ifuncmain4picstatic >> FAIL: elf/ifuncmain5 >> FAIL: elf/ifuncmain5picstatic >> FAIL: elf/ifuncmain5staticpic >> >> The solution is to replace the beqlr instructions by a beq to the end >> of the inline asm code. This fixes all the above failures. >> >> ChangeLog: >> * sysdeps/powerpc/ifunc-sel.h (ifunc_sel): Replace beqlr instructions >> by beq instructions jumping to the end of the function. > > I'd prefer to remove this file. > > Alan, do we still need this file? > >> --- >> ChangeLog | 5 +++++ >> sysdeps/powerpc/ifunc-sel.h | 7 ++++--- >> 2 files changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/ChangeLog b/ChangeLog >> index 97c46a1..b18a8cd 100644 >> --- a/ChangeLog >> +++ b/ChangeLog >> @@ -1,3 +1,8 @@ >> +2016-07-21 Aurelien Jarno <aurelien@aurel32.net> >> + >> + * sysdeps/powerpc/ifunc-sel.h (ifunc_sel): Replace beqlr instructions >> + by beq instructions jumping to the end of the function. >> + >> 2016-07-21 Szabolcs Nagy <szabolcs.nagy@arm.com> >> >> * sysdeps/aarch64/libm-test-ulps: Updated. >> diff --git a/sysdeps/powerpc/ifunc-sel.h b/sysdeps/powerpc/ifunc-sel.h >> index 526d8ed..79d110f 100644 >> --- a/sysdeps/powerpc/ifunc-sel.h >> +++ b/sysdeps/powerpc/ifunc-sel.h >> @@ -17,13 +17,14 @@ ifunc_sel (int (*f1) (void), int (*f2) (void), int (*f3) (void)) >> "addis %0,11,%2-1b@ha\n\t" >> "addi %0,%0,%2-1b@l\n\t" >> "cmpwi 12,1\n\t" >> - "beqlr\n\t" >> + "beq 2f\n\t" >> "addis %0,11,%3-1b@ha\n\t" >> "addi %0,%0,%3-1b@l\n\t" >> "cmpwi 12,-1\n\t" >> - "beqlr\n\t" >> + "beq 2f\n\t" >> "addis %0,11,%4-1b@ha\n\t" >> - "addi %0,%0,%4-1b@l" >> + "addi %0,%0,%4-1b@l\n\t" >> + "2:" >> : "=r" (ret) >> : "X" (&global), "X" (f1), "X" (f2), "X" (f3)); >> return ret; >> -- >> 2.8.1 >> > I think the idea of these function are to solely test ifunc implementation and I think we should keep them. Also, if we are fixing it here it would be a good idea to also fix the same implementation on gold/binutils (gold/testsuite/ifunc-sel.h).
Aurelien Jarno <aurelien@aurel32.net> writes: > On 2016-07-22 13:06, Alan Modra wrote: >> On Thu, Jul 21, 2016 at 02:48:09PM -0300, Tulio Magno Quites Machado Filho wrote: >> > Aurelien Jarno <aurelien@aurel32.net> writes: >> > > * sysdeps/powerpc/ifunc-sel.h (ifunc_sel): Replace beqlr instructions >> > > by beq instructions jumping to the end of the function. >> > >> > I'd prefer to remove this file. >> > >> > Alan, do we still need this file? >> >> To do without it, I think you'd need to bump the glibc binutils >> requirement to 2.24, in order to have this linker fix: >> https://sourceware.org/ml/binutils/2013-03/msg00299.html Great! Thanks! I run some tests on ppc, ppc64 and ppc64le without this file and I noticed just one failure from elf/ifuncmain6pie using Binutils 2.26.1 on ppc. Are there any other requirements to get it working? > In that case, one possibility is to support ifunc only when the > installed binutils is at least 2.24. This is basically just changing > the minimum version in the "Prevent building POWER8 code without support > in assembler" patch. In that case and due to the error I mentioned previously, I prefer to carry this file a little longer with your fix and a source code comment, i.e.: /* TODO: Remove this file when the minimum required Binutils is 2.24. The following implementation is required when using a linker that doesn't have the following fix: https://sourceware.org/ml/binutils/2013-03/msg00299.html */
On Fri, Jul 22, 2016 at 11:25:38AM -0300, Tulio Magno Quites Machado Filho wrote: > Aurelien Jarno <aurelien@aurel32.net> writes: > > > On 2016-07-22 13:06, Alan Modra wrote: > >> On Thu, Jul 21, 2016 at 02:48:09PM -0300, Tulio Magno Quites Machado Filho wrote: > >> > Aurelien Jarno <aurelien@aurel32.net> writes: > >> > > * sysdeps/powerpc/ifunc-sel.h (ifunc_sel): Replace beqlr instructions > >> > > by beq instructions jumping to the end of the function. > >> > > >> > I'd prefer to remove this file. > >> > > >> > Alan, do we still need this file? > >> > >> To do without it, I think you'd need to bump the glibc binutils > >> requirement to 2.24, in order to have this linker fix: > >> https://sourceware.org/ml/binutils/2013-03/msg00299.html > > Great! Thanks! > I run some tests on ppc, ppc64 and ppc64le without this file and I noticed > just one failure from elf/ifuncmain6pie using Binutils 2.26.1 on ppc. > Are there any other requirements to get it working? Yes. Rewrite ld.so ifunc handling to process ifunc relocs last, globally. Without that, ifuncmain6pie will fail on any target that uses the GOT to return the address of "one" from foo_ifunc. Currently, foo_ifunc is called during processing of ifuncmod6.so dynamic relocations, which happens before ifuncmain6pie has been relocated. That means foo_ifunc returns an unrelocated GOT entry.
On Thu, 2016-07-21 at 17:23 +0200, Andreas Schwab wrote: > Florian Weimer <fweimer@redhat.com> writes: > > > > > Yes, I figured that out in the meantime. Is “bcl 20,31” always > > used for > > that? The second opcode argument doesn't really matter, after > > all. > > It's what gcc uses. I don't know if there is any deeper meaning > behind that particular encoding. > There is. It tells the CPU not to push the address onto the link stack. Without this, you end up with an out of sync return stack and so your subsequent real returns get mispredicted. Cheers, Ben.
On 07/24/2016 10:14 PM, Benjamin Herrenschmidt wrote: > On Thu, 2016-07-21 at 17:23 +0200, Andreas Schwab wrote: >> Florian Weimer <fweimer@redhat.com> writes: >> >>> >>> Yes, I figured that out in the meantime. Is “bcl 20,31” always >>> used for >>> that? The second opcode argument doesn't really matter, after >>> all. >> >> It's what gcc uses. I don't know if there is any deeper meaning >> behind that particular encoding. >> > There is. It tells the CPU not to push the address onto the link > stack. Without this, you end up with an out of sync return stack > and so your subsequent real returns get mispredicted. Yes, a colleague told me that as well, thanks. Do you know if only this specific encoding has this property, or are there many equivalent encodings which have the same effect? Thanks, Florian
On Mo, Jul 25 2016, Florian Weimer <fweimer@redhat.com> wrote: > Yes, a colleague told me that as well, thanks. Do you know if only this > specific encoding has this property, or are there many equivalent > encodings which have the same effect? See the Programming Note near the end of section 2.4 in the PowerISA Book I. It mentions only this special form. Andreas.
On 2016-07-21 14:48, Tulio Magno Quites Machado Filho wrote: > Aurelien Jarno <aurelien@aurel32.net> writes: > > > On 32-bit PowerPC GCC 6 always saves the PIC register on the stack in > > the prologue and adjust the stack in the epilogue. It is therefore not > > possible anymore to just exit the function in the inline asm code, > > otherwise it corrupts the stack pointer. This causes the following tests > > to fail when using GCC 6: > > > > FAIL: elf/ifuncmain1 > > FAIL: elf/ifuncmain1pic > > FAIL: elf/ifuncmain1picstatic > > FAIL: elf/ifuncmain1pie > > FAIL: elf/ifuncmain1staticpic > > FAIL: elf/ifuncmain1staticpie > > FAIL: elf/ifuncmain1vis > > FAIL: elf/ifuncmain1vispic > > FAIL: elf/ifuncmain1vispie > > FAIL: elf/ifuncmain2pic > > FAIL: elf/ifuncmain2picstatic > > FAIL: elf/ifuncmain3 > > FAIL: elf/ifuncmain4picstatic > > FAIL: elf/ifuncmain5 > > FAIL: elf/ifuncmain5picstatic > > FAIL: elf/ifuncmain5staticpic > > > > The solution is to replace the beqlr instructions by a beq to the end > > of the inline asm code. This fixes all the above failures. > > > > ChangeLog: > > * sysdeps/powerpc/ifunc-sel.h (ifunc_sel): Replace beqlr instructions > > by beq instructions jumping to the end of the function. > > I'd prefer to remove this file. > > Alan, do we still need this file? Now that 2.25 is open for development, what should we do? I think we can get the patch in, and try to remove this file in a second step. Aurelien
Aurelien Jarno <aurelien@aurel32.net> writes: > On 2016-07-21 14:48, Tulio Magno Quites Machado Filho wrote: >> Aurelien Jarno <aurelien@aurel32.net> writes: >> >> > On 32-bit PowerPC GCC 6 always saves the PIC register on the stack in >> > the prologue and adjust the stack in the epilogue. It is therefore not >> > possible anymore to just exit the function in the inline asm code, >> > otherwise it corrupts the stack pointer. This causes the following tests >> > to fail when using GCC 6: >> > >> > FAIL: elf/ifuncmain1 >> > FAIL: elf/ifuncmain1pic >> > FAIL: elf/ifuncmain1picstatic >> > FAIL: elf/ifuncmain1pie >> > FAIL: elf/ifuncmain1staticpic >> > FAIL: elf/ifuncmain1staticpie >> > FAIL: elf/ifuncmain1vis >> > FAIL: elf/ifuncmain1vispic >> > FAIL: elf/ifuncmain1vispie >> > FAIL: elf/ifuncmain2pic >> > FAIL: elf/ifuncmain2picstatic >> > FAIL: elf/ifuncmain3 >> > FAIL: elf/ifuncmain4picstatic >> > FAIL: elf/ifuncmain5 >> > FAIL: elf/ifuncmain5picstatic >> > FAIL: elf/ifuncmain5staticpic >> > >> > The solution is to replace the beqlr instructions by a beq to the end >> > of the inline asm code. This fixes all the above failures. >> > >> > ChangeLog: >> > * sysdeps/powerpc/ifunc-sel.h (ifunc_sel): Replace beqlr instructions >> > by beq instructions jumping to the end of the function. >> >> I'd prefer to remove this file. >> >> Alan, do we still need this file? > > Now that 2.25 is open for development, what should we do? I think we can > get the patch in, and try to remove this file in a second step. I agree. As Alan explained, we still need it. Thanks!
On 2016-08-02 11:44, Tulio Magno Quites Machado Filho wrote: > Aurelien Jarno <aurelien@aurel32.net> writes: > > > On 2016-07-21 14:48, Tulio Magno Quites Machado Filho wrote: > >> Aurelien Jarno <aurelien@aurel32.net> writes: > >> > >> > On 32-bit PowerPC GCC 6 always saves the PIC register on the stack in > >> > the prologue and adjust the stack in the epilogue. It is therefore not > >> > possible anymore to just exit the function in the inline asm code, > >> > otherwise it corrupts the stack pointer. This causes the following tests > >> > to fail when using GCC 6: > >> > > >> > FAIL: elf/ifuncmain1 > >> > FAIL: elf/ifuncmain1pic > >> > FAIL: elf/ifuncmain1picstatic > >> > FAIL: elf/ifuncmain1pie > >> > FAIL: elf/ifuncmain1staticpic > >> > FAIL: elf/ifuncmain1staticpie > >> > FAIL: elf/ifuncmain1vis > >> > FAIL: elf/ifuncmain1vispic > >> > FAIL: elf/ifuncmain1vispie > >> > FAIL: elf/ifuncmain2pic > >> > FAIL: elf/ifuncmain2picstatic > >> > FAIL: elf/ifuncmain3 > >> > FAIL: elf/ifuncmain4picstatic > >> > FAIL: elf/ifuncmain5 > >> > FAIL: elf/ifuncmain5picstatic > >> > FAIL: elf/ifuncmain5staticpic > >> > > >> > The solution is to replace the beqlr instructions by a beq to the end > >> > of the inline asm code. This fixes all the above failures. > >> > > >> > ChangeLog: > >> > * sysdeps/powerpc/ifunc-sel.h (ifunc_sel): Replace beqlr instructions > >> > by beq instructions jumping to the end of the function. > >> > >> I'd prefer to remove this file. > >> > >> Alan, do we still need this file? > > > > Now that 2.25 is open for development, what should we do? I think we can > > get the patch in, and try to remove this file in a second step. > > I agree. As Alan explained, we still need it. Thanks, I have pushed both patches. Aurelien
diff --git a/ChangeLog b/ChangeLog index 97c46a1..b18a8cd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2016-07-21 Aurelien Jarno <aurelien@aurel32.net> + + * sysdeps/powerpc/ifunc-sel.h (ifunc_sel): Replace beqlr instructions + by beq instructions jumping to the end of the function. + 2016-07-21 Szabolcs Nagy <szabolcs.nagy@arm.com> * sysdeps/aarch64/libm-test-ulps: Updated. diff --git a/sysdeps/powerpc/ifunc-sel.h b/sysdeps/powerpc/ifunc-sel.h index 526d8ed..79d110f 100644 --- a/sysdeps/powerpc/ifunc-sel.h +++ b/sysdeps/powerpc/ifunc-sel.h @@ -17,13 +17,14 @@ ifunc_sel (int (*f1) (void), int (*f2) (void), int (*f3) (void)) "addis %0,11,%2-1b@ha\n\t" "addi %0,%0,%2-1b@l\n\t" "cmpwi 12,1\n\t" - "beqlr\n\t" + "beq 2f\n\t" "addis %0,11,%3-1b@ha\n\t" "addi %0,%0,%3-1b@l\n\t" "cmpwi 12,-1\n\t" - "beqlr\n\t" + "beq 2f\n\t" "addis %0,11,%4-1b@ha\n\t" - "addi %0,%0,%4-1b@l" + "addi %0,%0,%4-1b@l\n\t" + "2:" : "=r" (ret) : "X" (&global), "X" (f1), "X" (f2), "X" (f3)); return ret;