[PATCH][GCC][AArch64] Rename stack-clash CFA register to avoid clash.

[James Greenhalgh]

On Wed, Jan 16, 2019 at 11:03:41AM -0600, Tamar Christina wrote:
> Hi All,
> 
> We had multiple patches in flight that required used of scratch registers in
> frame layout code.  As it happens two of these features picked the same register
> and landed at around the same time.  As such there is a clash when both are used
> at the same time.   This patch changes the temporary r15 to r11 for stack clash
> and documents the "reserved" registers in the frame layout comment.
> 
> Cross compiled and regtested on aarch64-none-elf with SVE on by default and no
> issues.
> Bootstrapped on aarch64-none-linux-gnu and no issues.
> 
> Ok for trunk?

My comments are all on your new comment detailing the register allocations,
which I fully support.

This patch is OK with those changes.

> gcc/ChangeLog:
> 
> 2019-01-16  Tamar Christina  <tamar.christina@arm.com>
> 
> 	PR target/88851
> 	* config/aarch64/aarch64.md (STACK_CLASH_SVE_CFA_REGNUM): New.
> 	* config/aarch64/aarch64.c (aarch64_allocate_and_probe_stack_space): Use
> 	it and document registers.
> 
> gcc/testsuite/ChangeLog:
> 
> 2019-01-16  Tamar Christina  <tamar.christina@arm.com>
> 
> 	PR target/88851
> 	* gcc.target/aarch64/stack-check-cfa-3.c: Update test.
> 
> -- 

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index fd60bddb1e1cbcb3dd46c319ccd182c7b9d1cd41..6a5f4956247b89932f955abbe96776a1b1ffb9cb 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -5317,11 +5317,11 @@ aarch64_allocate_and_probe_stack_space (rtx temp1, rtx temp2,
>  	{
>  	  /* This is done to provide unwinding information for the stack
>  	     adjustments we're about to do, however to prevent the optimizers
> -	     from removing the R15 move and leaving the CFA note (which would be
> +	     from removing the R11 move and leaving the CFA note (which would be
>  	     very wrong) we tie the old and new stack pointer together.
>  	     The tie will expand to nothing but the optimizers will not touch
>  	     the instruction.  */
> -	  rtx stack_ptr_copy = gen_rtx_REG (Pmode, R15_REGNUM);
> +	  rtx stack_ptr_copy = gen_rtx_REG (Pmode, STACK_CLASH_SVE_CFA_REGNUM);
>  	  emit_move_insn (stack_ptr_copy, stack_pointer_rtx);
>  	  emit_insn (gen_stack_tie (stack_ptr_copy, stack_pointer_rtx));
>  
> @@ -5548,7 +5548,18 @@ aarch64_add_cfa_expression (rtx_insn *insn, unsigned int reg,
>     to the stack we track as implicit probes are the FP/LR stores.
>  
>     For outgoing arguments we probe if the size is larger than 1KB, such that
> -   the ABI specified buffer is maintained for the next callee.  */
> +   the ABI specified buffer is maintained for the next callee.
> +
> +   Aside from LR, FP, IP1 and IP0 there are a few other registers that if used
> +   would clash with other features:

How about...

 The following registers are reserved during frame layout and should not be
 used for any other purpose.

  - LR <and so on>

> +
> +   - r14 and r15: Used by mitigation code.

"Used for speculation tracking." seems more correct. 'Mitigation Code' is
too broad I think.

> +   - r16 and r17: Used by indirect tailcalls
> +   - r12 and r13: Used as temporaries for stack adjustment
> +     (EP0_REGNUM/EP1_REGNUM)
> +   - r11: Used by stack clash protection when SVE is enabled.

Put them in numerical (or other logical) order?

> +
> +   These registers should be avoided in frame layout related code.  */

s/should/must/

>  
>  /* Generate the prologue instructions for entry into a function.
>     Establish the stack frame by decreasing the stack pointer with a