This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH], PR target/79434, fix PowerPC recursive calls that can replaced at runtime
- From: Segher Boessenkool <segher at kernel dot crashing dot org>
- To: Michael Meissner <meissner at linux dot vnet dot ibm dot com>, gcc-patches at gcc dot gnu dot org, David Edelsohn <dje dot gcc at gmail dot com>, Bill Schmidt <wschmidt at linux dot vnet dot ibm dot com>
- Date: Wed, 1 Mar 2017 05:21:44 -0600
- Subject: Re: [PATCH], PR target/79434, fix PowerPC recursive calls that can replaced at runtime
- Authentication-results: sourceware.org; auth=none
- References: <20170301063714.GA14043@ibm-tiger.the-meissners.org>
On Wed, Mar 01, 2017 at 01:37:14AM -0500, Michael Meissner wrote:
> This patch fixes PR target/79439, which is a recursive call when the 64-bit
> code is compiled with -fpic doesn't have the NOP after the call. It is
> possible for the function to be overriden at link time. In such a case, the
> call should call the module that is overriding the call, rather than itself.
>
> The following patch was tested on a little endian Power8 Linux system (64-bit
> only), a big endian Power8 Linux system (both 32-bit and 64-bit), and a big
> endian Power7 Linux system (both 32-bit and 64-bit). There were no regressions
> in the test suite, and I verified that the new test ran successfully in 64-bit
> mode. Can I check this patch into the trunk?
Yes, thanks!
> Since the bug was reported against GCC 6, can I apply the patch to GCC 6
> assuming the patch applies cleanly and has no regressions after a burn in
> period on the GCC 7 trunk?
Of course. Also for GCC 5, if it is worth fixing it there?
Some questions/comments about the testcase:
> Index: gcc/testsuite/gcc.target/powerpc/pr79439.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/pr79439.c (revision 0)
> +++ gcc/testsuite/gcc.target/powerpc/pr79439.c (revision 0)
> @@ -0,0 +1,26 @@
> +/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
Is this enough? Do all 64-bit ABIs have the insn to be patched after
call instructions?
> +/* { dg-require-effective-target powerpc_p8vector_ok } */
Why this?
> +/* Bug 79439 -- we should not eliminate NOP in 'rec' call because it can be
> + interposed at link time for 64-bit ABIs. We need -fpic to tell the compiler
> + functions may be interposed. */
That reads as "cannot be interposed on 32-bit ABIs", which isn't what
you mean I think.
> +/* { dg-final { scan-assembler-times {\mnop\M} 3 } } */
You can also check they follow a "bl" insn immediately (scan-assembler
does not scan single lines, but the whole output). Something like
{ scan-assembler-times {\mbl \S+\s+nop\M} 3 }
Or maybe this is overkill here :-)
Segher