This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] rs6000: Don't touch below the stack pointer (PR77687)
- From: Segher Boessenkool <segher at kernel dot crashing dot org>
- To: gcc-patches at gcc dot gnu dot org
- Cc: dje dot gcc at gmail dot com, hainque at adacore dot com
- Date: Mon, 20 Nov 2017 14:16:21 -0600
- Subject: Re: [PATCH] rs6000: Don't touch below the stack pointer (PR77687)
- Authentication-results: sourceware.org; auth=none
- References: <262f09c4d25789b9f277b789e4a13e689ef02c7a.1505854731.git.segher@kernel.crashing.org>
On Tue, Sep 19, 2017 at 09:21:50PM +0000, Segher Boessenkool wrote:
> With the 32-bit SVR4 ABI we don't have a red zone, so we have to restore
> the callee-saved registers before we restore the stack pointer.
>
> The previous fix for this PR failed in two ways, for huge frames: first,
> we use a negative offset from r11 in that case, so the (mem:BLK 11) access
> does no good; second, sched does not handle accesses to mem:BLK correctly
> in this case (does not make dependencies).
>
> This patch fixes it by doing a store to (mem:BLK (scratch)) instead.
> This means no unrelated (not to stack) loads/stores can be moved over the
> stack restore either, but so be it.
I backported this to GCC 7 now.
Segher
> 2017-09-19 Segher Boessenkool <segher@kernel.crashing.org>
>
> PR target/77687
> * config/rs6000/rs6000.md (stack_restore_tie): Store to a scratch
> address instead of to r1 and r11.
>
> gcc/testsuite/
> PR target/77687
> * gcc.target/powerpc/pr77687.c: New testcase.
>
>
> ---
> gcc/config/rs6000/rs6000.md | 6 ++----
> gcc/testsuite/gcc.target/powerpc/pr77687.c | 20 ++++++++++++++++++++
> 2 files changed, 22 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr77687.c
>
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index 7f17628..13ba743 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -13161,14 +13161,12 @@ (define_insn "stack_tie"
>
> ; Some 32-bit ABIs do not have a red zone, so the stack deallocation has to
> ; stay behind all restores from the stack, it cannot be reordered to before
> -; one. See PR77687. This insn is an add or mr, and a stack_tie on the
> -; operands of that.
> +; one. See PR77687. This insn is an add or mr, and a memory clobber.
> (define_insn "stack_restore_tie"
> [(set (match_operand:SI 0 "gpc_reg_operand" "=r,r")
> (plus:SI (match_operand:SI 1 "gpc_reg_operand" "r,r")
> (match_operand:SI 2 "reg_or_cint_operand" "O,rI")))
> - (set (mem:BLK (match_dup 0)) (const_int 0))
> - (set (mem:BLK (match_dup 1)) (const_int 0))]
> + (set (mem:BLK (scratch)) (const_int 0))]
> "TARGET_32BIT"
> "@
> mr %0,%1
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr77687.c b/gcc/testsuite/gcc.target/powerpc/pr77687.c
> new file mode 100644
> index 0000000..95b27ae
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr77687.c
> @@ -0,0 +1,20 @@
> +/* { dg-options "-std=gnu99 -O2" } */
> +/* { dg-final { scan-assembler-not {\mmr r?1,r?11\M.*11.*\mblr\M} } } */
> +
> +/* PR77687: We used to do stack accesses (via r11) after restoring r1. */
> +
> +void g(int, char *);
> +const char * dum = "hello";
> +
> +void f(int x)
> +{
> + char big[200000];
> + start:
> + g(x, big);
> + g(x, big);
> + register void *p asm("r11") = &&start;
> + asm("" : : "r"(p));
> + asm("" : : :"r28");
> + asm("" : : :"r29");
> + asm("" : : :"r30");
> +}
> --
> 1.8.3.1