* [PATCH] rs6000: Change GPR2 to volatile & non-fixed register for function that does not use TOC [PR110320]
@ 2023-06-23 16:49 P Jeevitha
2023-06-28 8:07 ` Kewen.Lin
0 siblings, 1 reply; 4+ messages in thread
From: P Jeevitha @ 2023-06-23 16:49 UTC (permalink / raw)
To: Segher Boessenkool, linkw, gcc-patches; +Cc: Peter Bergner
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,
if the code is PCREL and the user is not explicitly requesting TOC addressing,
then the register r2 can be changed to volatile and non-fixed register. Changes
in register preservation roles can be accomplished with the help of available
target hooks (TARGET_CONDITIONAL_REGISTER_USAGE).
2023-06-23 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 pc-relative code.
gcc/testsuite/
PR target/PR110320
* gcc.target/powerpc/pr110320_1.c: New testcase.
* gcc.target/powerpc/pr110320_2.c: New testcase.
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index 546c353029b..9e978f85f9d 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -10169,6 +10169,35 @@ rs6000_conditional_register_usage (void)
if (DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2)
call_used_regs[2] = 0;
+ /* The TOC register is not needed for functions using the PC-relative ABI
+ extension, so make it available for register allocation as a volatile
+ register. */
+ if (FIXED_R2 && rs6000_pcrel_p ())
+ {
+ bool cli_fixedr2 = false;
+
+ /* Verify the user has not explicitly asked for GPR2 to be fixed. */
+ if (common_deferred_options)
+ {
+ unsigned int idx;
+ cl_deferred_option *opt;
+ vec<cl_deferred_option> v;
+ v = *((vec<cl_deferred_option> *) common_deferred_options);
+ FOR_EACH_VEC_ELT (v, idx, opt)
+ if (opt->opt_index == OPT_ffixed_ && strcmp (opt->arg,"r2") == 0)
+ {
+ cli_fixedr2 = true;
+ break;
+ }
+ }
+
+ /* If GPR2 is not FIXED (eg, not a TOC register), then it is volatile. */
+ if (!cli_fixedr2)
+ {
+ fixed_regs[2] = 0;
+ call_used_regs[2] = 1;
+ }
+ }
if (DEFAULT_ABI == ABI_V4 && flag_pic == 2)
fixed_regs[RS6000_PIC_OFFSET_TABLE_REGNUM] = 1;
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..42143fbf889
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr110320_1.c
@@ -0,0 +1,23 @@
+/* PR target/110320 */
+/* { dg-require-effective-target power10_ok } */
+/* { 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..9d0da5b9695
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr110320_2.c
@@ -0,0 +1,22 @@
+/* PR target/110320 */
+/* { dg-require-effective-target power10_ok } */
+/* { 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} } } */
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] rs6000: Change GPR2 to volatile & non-fixed register for function that does not use TOC [PR110320]
2023-06-23 16:49 [PATCH] rs6000: Change GPR2 to volatile & non-fixed register for function that does not use TOC [PR110320] P Jeevitha
@ 2023-06-28 8:07 ` Kewen.Lin
2023-07-06 18:41 ` Peter Bergner
0 siblings, 1 reply; 4+ messages in thread
From: Kewen.Lin @ 2023-06-28 8:07 UTC (permalink / raw)
To: P Jeevitha; +Cc: Peter Bergner, Segher Boessenkool, gcc-patches, David Edelsohn
Hi Jeevitha,
on 2023/6/24 00:49, P Jeevitha wrote:
> 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,
> if the code is PCREL and the user is not explicitly requesting TOC addressing,
> then the register r2 can be changed to volatile and non-fixed register. Changes
> in register preservation roles can be accomplished with the help of available
> target hooks (TARGET_CONDITIONAL_REGISTER_USAGE).
Nice, thanks for improving this.
>
> 2023-06-23 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 pc-relative code.
>
> gcc/testsuite/
> PR target/PR110320
> * gcc.target/powerpc/pr110320_1.c: New testcase.
> * gcc.target/powerpc/pr110320_2.c: New testcase.
>
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index 546c353029b..9e978f85f9d 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -10169,6 +10169,35 @@ rs6000_conditional_register_usage (void)
> if (DEFAULT_ABI == ABI_AIX || DEFAULT_ABI == ABI_ELFv2)
> call_used_regs[2] = 0;
>
> + /* The TOC register is not needed for functions using the PC-relative ABI
> + extension, so make it available for register allocation as a volatile
> + register. */
> + if (FIXED_R2 && rs6000_pcrel_p ())
> + {
> + bool cli_fixedr2 = false;
> +
> + /* Verify the user has not explicitly asked for GPR2 to be fixed. */
> + if (common_deferred_options)
> + {
> + unsigned int idx;
> + cl_deferred_option *opt;
> + vec<cl_deferred_option> v;
> + v = *((vec<cl_deferred_option> *) common_deferred_options);
> + FOR_EACH_VEC_ELT (v, idx, opt)
> + if (opt->opt_index == OPT_ffixed_ && strcmp (opt->arg,"r2") == 0)
> + {
> + cli_fixedr2 = true;
> + break;
> + }
> + }
I think the reason why we need to check common_deferred_options is at this time
we can't distinguish the fixed_regs[2] is from the initialization or command line
user explicit specification. But could we just update the FIXED_REGISTERS without
FIXED_R2 and set FIXED_R2 when it's needed in this function instead? Then I'd
expect that when we find fixed_regs[2] is set at the beginning of this function, it
would mean users specify it explicitly and then we don't need this option checking?
Besides, IMHO we need a corresponding test case to cover this -ffixed-r2 handling.
> +
> + /* If GPR2 is not FIXED (eg, not a TOC register), then it is volatile. */
> + if (!cli_fixedr2)
> + {
> + fixed_regs[2] = 0;
> + call_used_regs[2] = 1;
> + }
> + }
> if (DEFAULT_ABI == ABI_V4 && flag_pic == 2)
> fixed_regs[RS6000_PIC_OFFSET_TABLE_REGNUM] = 1;
>
> 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..42143fbf889
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr110320_1.c
> @@ -0,0 +1,23 @@
> +/* PR target/110320 */
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-require-effective-target powerpc_pcrel } */
Do we have some environment combination which supports powerpc_pcrel but not
power10_ok? I'd expect that only powerpc_pcrel is enough.
> +/* { 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..9d0da5b9695
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr110320_2.c
> @@ -0,0 +1,22 @@
> +/* PR target/110320 */
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-require-effective-target powerpc_pcrel } */
Ditto.
The others look good to me, thanks!
BR,
Kewen
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] rs6000: Change GPR2 to volatile & non-fixed register for function that does not use TOC [PR110320]
2023-06-28 8:07 ` Kewen.Lin
@ 2023-07-06 18:41 ` Peter Bergner
2023-07-11 10:57 ` P Jeevitha
0 siblings, 1 reply; 4+ messages in thread
From: Peter Bergner @ 2023-07-06 18:41 UTC (permalink / raw)
To: Kewen.Lin, P Jeevitha; +Cc: Segher Boessenkool, gcc-patches, David Edelsohn
On 6/28/23 3:07 AM, Kewen.Lin wrote:
> I think the reason why we need to check common_deferred_options is at this time
> we can't distinguish the fixed_regs[2] is from the initialization or command line
> user explicit specification. But could we just update the FIXED_REGISTERS without
> FIXED_R2 and set FIXED_R2 when it's needed in this function instead? Then I'd
> expect that when we find fixed_regs[2] is set at the beginning of this function, it
> would mean users specify it explicitly and then we don't need this option checking?
Correct, rs6000_conditional_register_usage() is called after the handling of the
-ffixed-* options, so looking at fixed_regs[2] cannot tell us whether the user
used the -ffixed-r2 option or not if we initialize the FIXED_REGISTERS[2] slot
to 1. I think we went this route for two reasons:
1) We don't have to worry about anyone in the future adding more uses of
FIXED_REGISTERS and needing to update the value depending on ABI, options,
etc.
2) The options in common_deferred_options are "rare" options, so the common
case is that common_deferred_options will be NULL and we'll never drop
into that section.
I believe the untested patch below should also work, without having to scan
the (uncommonly used) options. Jeevitha, can you bootstrap and regtest the
patch below?
> Besides, IMHO we need a corresponding test case to cover this -ffixed-r2 handling.
Good idea. I think we can duplicate the pr110320_2.c test case, replacing the
-mno-pcrel option with -ffixed-r2. Jeevitha, can you give that a try?
>> +/* { dg-require-effective-target power10_ok } */
>> +/* { dg-require-effective-target powerpc_pcrel } */
>
> Do we have some environment combination which supports powerpc_pcrel but not
> power10_ok? I'd expect that only powerpc_pcrel is enough.
I think I agree testing for powerpc_pcrel should be enough.
Peter
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index d197c3f3289..7c356a73ac6 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -10160,9 +10160,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, \
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] rs6000: Change GPR2 to volatile & non-fixed register for function that does not use TOC [PR110320]
2023-07-06 18:41 ` Peter Bergner
@ 2023-07-11 10:57 ` P Jeevitha
0 siblings, 0 replies; 4+ messages in thread
From: P Jeevitha @ 2023-07-11 10:57 UTC (permalink / raw)
To: Peter Bergner, Kewen.Lin; +Cc: Segher Boessenkool, gcc-patches, David Edelsohn
On 07/07/2023 A 12:11 am, Peter Bergner wrote:
> I believe the untested patch below should also work, without having to scan
> the (uncommonly used) options. Jeevitha, can you bootstrap and regtest the
> patch below?
Yeah Peter, Bootstrapped and regtested the below patch on powerpc64le-linux there was no regression.
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index d197c3f3289..7c356a73ac6 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -10160,9 +10160,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, \
>
>
>
>> Besides, IMHO we need a corresponding test case to cover this -ffixed-r2 handling.
>
> Good idea. I think we can duplicate the pr110320_2.c test case, replacing the
> -mno-pcrel option with -ffixed-r2. Jeevitha, can you give that a try?
Yeah, adding the new test cases along with the mentioned changes for the older ones below,
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-r2 -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, 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} } } */
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-07-11 10:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-23 16:49 [PATCH] rs6000: Change GPR2 to volatile & non-fixed register for function that does not use TOC [PR110320] P Jeevitha
2023-06-28 8:07 ` Kewen.Lin
2023-07-06 18:41 ` Peter Bergner
2023-07-11 10:57 ` P Jeevitha
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).