Re: [PATCH V2] rs6000: Change GPR2 to volatile & non-fixed register for function that does not use TOC [PR110320]

["Kewen.Lin" <linkw@linux.ibm.com>]

Hi Jeevitha,

on 2023/7/17 11:40, P Jeevitha wrote:
> 
> Hi All,
> 
> The following patch has been bootstrapped and regtested on powerpc64le-linux.

Since one line touched has (DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2)
and powerpc64le-linux only adopts ABI_ELFv2, could you also test this on
powerpc64-linux or aix to ensure it doesn't break ABI_AIX as we expected?

And Peter made the diff on rs6000.cc, I guess you want to put him as co-author,
i.e maybe adding one line with:

Co-authored-by: Peter Bergner <bergner@linux.ibm.com>

The others look good to me, okay for trunk if the suggested testings go well
as expected.  Thanks!

BR,
Kewen

> 
> Normally, GPR2 is the TOC pointer and is defined as a fixed and non-volatile
> register. However, it can be used as volatile for PCREL addressing. Therefore,
> modified r2 to be non-fixed in FIXED_REGISTERS and set it to fixed if it is not
> PCREL and also when the user explicitly requests TOC or fixed. If the register
> r2 is fixed, it is made as non-volatile. Changes in register preservation roles
> can be accomplished with the help of available target hooks
> (TARGET_CONDITIONAL_REGISTER_USAGE).
> 
> 2023-07-12  Jeevitha Palanisamy  <jeevitha@linux.ibm.com>
> 
> gcc/
> 	PR target/PR110320
> 	* config/rs6000/rs6000.cc (rs6000_conditional_register_usage): Change
> 	GPR2 to volatile and non-fixed register for PCREL.
> 
> gcc/testsuite/
> 	PR target/PR110320
> 	* gcc.target/powerpc/pr110320-1.c: New testcase.
> 	* gcc.target/powerpc/pr110320-2.c: New testcase.
> 	* gcc.target/powerpc/pr110320-3.c: New testcase.
> 
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index 44b448d2ba6..9aa04ec5d57 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -10193,9 +10193,13 @@ rs6000_conditional_register_usage (void)
>      for (i = 32; i < 64; i++)
>        fixed_regs[i] = call_used_regs[i] = 1;
>  
> +  /* For non PC-relative code, GPR2 is unavailable for register allocation.  */
> +  if (FIXED_R2 && !rs6000_pcrel_p ())
> +    fixed_regs[2] = 1;
> +
>    /* The TOC register is not killed across calls in a way that is
>       visible to the compiler.  */
> -  if (DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2)
> +  if (fixed_regs[2] && (DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2))
>      call_used_regs[2] = 0;
>  
>    if (DEFAULT_ABI == ABI_V4 && flag_pic == 2)
> diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
> index 3503614efbd..2a24fbdf9fd 100644
> --- a/gcc/config/rs6000/rs6000.h
> +++ b/gcc/config/rs6000/rs6000.h
> @@ -812,7 +812,7 @@ enum data_align { align_abi, align_opt, align_both };
>  
>  #define FIXED_REGISTERS  \
>    {/* GPRs */					   \
> -   0, 1, FIXED_R2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, FIXED_R13, 0, 0, \
> +   0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, FIXED_R13, 0, 0, \
>     0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
>     /* FPRs */					   \
>     0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr110320-1.c b/gcc/testsuite/gcc.target/powerpc/pr110320-1.c
> new file mode 100644
> index 00000000000..a4ad34d9303
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr110320-1.c
> @@ -0,0 +1,22 @@
> +/* PR target/110320 */
> +/* { dg-require-effective-target powerpc_pcrel } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power10 -ffixed-r0 -ffixed-r11 -ffixed-r12" } */
> +
> +/* Ensure we use r2 as a normal volatile register for the code below.
> +   The test case ensures all of the parameter registers r3 - r10 are used
> +   and needed after we compute the expression "x + y" which requires a
> +   temporary.  The -ffixed-r* options disallow using the other volatile
> +   registers r0, r11 and r12.  That leaves RA to choose from r2 and the more
> +   expensive non-volatile registers for the temporary to be assigned to, and
> +   RA will always chooses the cheaper volatile r2 register.  */
> +
> +extern long bar (long, long, long, long, long, long, long, long *);
> +
> +long
> +foo (long r3, long r4, long r5, long r6, long r7, long r8, long r9, long *r10)
> +{
> +  *r10 = r3 + r4;
> +  return bar (r3, r4, r5, r6, r7, r8, r9, r10);
> +}
> +
> +/* { dg-final { scan-assembler {\madd 2,3,4\M} } } */
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr110320-2.c b/gcc/testsuite/gcc.target/powerpc/pr110320-2.c
> new file mode 100644
> index 00000000000..9d6aefedd2e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr110320-2.c
> @@ -0,0 +1,21 @@
> +/* PR target/110320 */
> +/* { dg-require-effective-target powerpc_pcrel } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power10 -mno-pcrel -ffixed-r0 -ffixed-r11 -ffixed-r12" } */
> +
> +/* Ensure we don't use r2 as a normal volatile register for the code below.
> +   The test case ensures all of the parameter registers r3 - r10 are used
> +   and needed after we compute the expression "x + y" which requires a
> +   temporary.  The -ffixed-r* options disallow using the other volatile
> +   registers r0, r11 and r12.  That only leaves RA to choose from the more
> +   expensive non-volatile registers for the temporary to be assigned to.  */
> +
> +extern long bar (long, long, long, long, long, long, long, long *);
> +
> +long
> +foo (long r3, long r4, long r5, long r6, long r7, long r8, long r9, long *r10)
> +{
> +  *r10 = r3 + r4;
> +  return bar (r3, r4, r5, r6, r7, r8, r9, r10);
> +}
> +
> +/* { dg-final { scan-assembler-not {\madd 2,3,4\M} } } */
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr110320-3.c b/gcc/testsuite/gcc.target/powerpc/pr110320-3.c
> new file mode 100644
> index 00000000000..ea6c6188c8d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr110320-3.c
> @@ -0,0 +1,21 @@
> +/* PR target/110320 */
> +/* { dg-require-effective-target powerpc_pcrel } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power10 -ffixed-r0 -ffixed-r2 -ffixed-r11 -ffixed-r12" } */
> +
> +/* Ensure we don't use r2 as a normal volatile register for the code below.
> +   The test case ensures all of the parameter registers r3 - r10 are used
> +   and needed after we compute the expression "x + y" which requires a
> +   temporary.  The -ffixed-r* options disallow using the other volatile
> +   registers r0, r2, r11 and r12.  That only leaves RA to choose from the more
> +   expensive non-volatile registers for the temporary to be assigned to.  */
> +
> +extern long bar (long, long, long, long, long, long, long, long *);
> +
> +long
> +foo (long r3, long r4, long r5, long r6, long r7, long r8, long r9, long *r10)
> +{
> +  *r10 = r3 + r4;
> +  return bar (r3, r4, r5, r6, r7, r8, r9, r10);
> +}
> +
> +/* { dg-final { scan-assembler-not {\madd 2,3,4\M} } } */
> 
>