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 <jeevi...@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
