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

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

[P Jeevitha]

Hi All,

The following patch has been bootstrapped and regtested on powerpc64le-linux.

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} } } */

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

[Kewen.Lin]

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} } } */
> 
>

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

[Peter Bergner]

On 7/16/23 10:40 PM, P Jeevitha via Gcc-patches wrote:
> 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;

Segher and Ke Wen,

As we discussed on my PR111045 patch that disabled PCREL on everything other
than ELFv2 and Segher said, well, it should work on ELFv1, modulo fixing bugs.
Segher said we should attempt to fix those bugs before we ship the next release
and if we miss that, we can push my PR111045 patch then to disable it.

On a related note, Jeevitha's patch above allows using r2 for normal register
allocation if r2 is not fixed and pcrel is enabled.  Given pcrel with this patch
enables pcrel on ELFv1, that means this patch can also enable using r2 for normal
register allocation on ELFv1.  Is that safe?  Should we add a check above where
we set fixed_regs[2] = 1, to also check for whether this is not an ELFv2 compile?
...or Segher, should we leave this as is and add it to the things to check for
non-ELFv2 compiles before the next release and possible disable it then if we
know/aren't sure whether it legal?

So I guessing I'm wondering, should Jeevitha push the above approved patch as
is, or should we modify it so r2 is only available for RA on ELFv2 and pcrel?

Peter

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

[Peter Bergner]

On 12/14/23 9:57 PM, Peter Bergner wrote:
> On 7/16/23 10:40 PM, P Jeevitha via Gcc-patches wrote:
>> +  /* For non PC-relative code, GPR2 is unavailable for register allocation.  */
>> +  if (FIXED_R2 && !rs6000_pcrel_p ())
>> +    fixed_regs[2] = 1;
[snip]
> On a related note, Jeevitha's patch above allows using r2 for normal register
> allocation if r2 is not fixed and pcrel is enabled.  Given pcrel with this patch
> enables pcrel on ELFv1, that means this patch can also enable using r2 for normal
> register allocation on ELFv1.

Nevermind, I'm daft and r2 usage is not allowed on ELFv1.  The rs6000_pcrel_p()
call above is always false for non ELFv2 compiles, so we'll mark r2 as fixed
for ELFv1.  Move along, nothing to see. :-)

That said, I think we need a "dg-require-effective-target powerpc_elfv2" for
the first test case where we're checking that we do use r2 for normal RA.
That'll only be true on ELFv2 compiles, hence the need for the extra target
requirement.  I've asked Jeevitha to add that to the pr111045-1.c test
case and verify it fixes the failure of that test case on her BE run.

Peter