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

[Peter Bergner <bergner@linux.ibm.com>]

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, \