diff mbox

rs6000: Save the PIC reg when needed

Message ID a8799e74ff81ce04beb5b0786e7beac7218781bb.1446136869.git.segher@kernel.crashing.org
State New
Headers show

Commit Message

Segher Boessenkool Oct. 29, 2015, 4:52 p.m. UTC 
The PIC reg (r30) needs to be saved whenever the prologue sets it up,
not just if it is used elsewhere in the function.  Without this patch
the prologue for such a function will modify r30 without saving it
first, leading to disaster back in its callers.  This happened in the
testsuite for -m32 libgfortran and libstdc++, bootstrapped with -mlra,
many hundred times.

Tested on powerpc64-linux, with LRA enabled by default (so that the
target libraries are built with -mlra, the case that failed before
this patch), {-m32/-mlra,-m32/-mno-lra,-m32/-mpowerpc64,-m64/-mlra,-m64/-mno-lra}.
With this and the previous LRA patch there now are no differences
between -mlra and -mno-lra testsuite runs (on big-endian power7, gcc110).

Is this okay for trunk?


Segher


2015-10-29  Segher Boessenkool  <segher@kernel.crashing.org>

	* config/rs6000/rs6000.c (rs6000_reg_live_or_pic_offset_p): Move this
	function earlier in the file.
	(first_reg_to_save): Use rs6000_reg_live_or_pic_offset_p instead of
	df_regs_ever_live_p.

---
 gcc/config/rs6000/rs6000.c | 44 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

Comments

David Edelsohn Oct. 29, 2015, 5:08 p.m. UTC | #1 
On Thu, Oct 29, 2015 at 12:52 PM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> The PIC reg (r30) needs to be saved whenever the prologue sets it up,
> not just if it is used elsewhere in the function.  Without this patch
> the prologue for such a function will modify r30 without saving it
> first, leading to disaster back in its callers.  This happened in the
> testsuite for -m32 libgfortran and libstdc++, bootstrapped with -mlra,
> many hundred times.
>
> Tested on powerpc64-linux, with LRA enabled by default (so that the
> target libraries are built with -mlra, the case that failed before
> this patch), {-m32/-mlra,-m32/-mno-lra,-m32/-mpowerpc64,-m64/-mlra,-m64/-mno-lra}.
> With this and the previous LRA patch there now are no differences
> between -mlra and -mno-lra testsuite runs (on big-endian power7, gcc110).
>
> Is this okay for trunk?
>
>
> Segher
>
>
> 2015-10-29  Segher Boessenkool  <segher@kernel.crashing.org>
>
>         * config/rs6000/rs6000.c (rs6000_reg_live_or_pic_offset_p): Move this
>         function earlier in the file.
>         (first_reg_to_save): Use rs6000_reg_live_or_pic_offset_p instead of
>         df_regs_ever_live_p.

Good catch!

Thanks, David
diff mbox

Patch

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 6ee8d5c..9610625 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -21775,6 +21775,27 @@  save_reg_p (int r)
   return !call_used_regs[r] && df_regs_ever_live_p (r);
 }
 
+/* Determine whether the gp REG is really used.  */
+
+static bool
+rs6000_reg_live_or_pic_offset_p (int reg)
+{
+  /* If the function calls eh_return, claim used all the registers that would
+     be checked for liveness otherwise.  This is required for the PIC offset
+     register with -mminimal-toc on AIX, as it is advertised as "fixed" for
+     register allocation purposes in this case.  */
+
+  return (((crtl->calls_eh_return || df_regs_ever_live_p (reg))
+           && (!call_used_regs[reg]
+               || (reg == RS6000_PIC_OFFSET_TABLE_REGNUM
+		   && !TARGET_SINGLE_PIC_BASE
+                   && TARGET_TOC && TARGET_MINIMAL_TOC)))
+          || (reg == RS6000_PIC_OFFSET_TABLE_REGNUM
+	      && !TARGET_SINGLE_PIC_BASE
+              && ((DEFAULT_ABI == ABI_V4 && flag_pic != 0)
+                  || (DEFAULT_ABI == ABI_DARWIN && flag_pic))));
+}
+
 /* Return the first fixed-point register that is required to be
    saved. 32 if none.  */
 
@@ -21792,7 +21813,7 @@  first_reg_to_save (void)
       && ((DEFAULT_ABI == ABI_V4 && flag_pic != 0)
 	  || (DEFAULT_ABI == ABI_DARWIN && flag_pic)
 	  || (TARGET_TOC && TARGET_MINIMAL_TOC))
-      && df_regs_ever_live_p (RS6000_PIC_OFFSET_TABLE_REGNUM))
+      && rs6000_reg_live_or_pic_offset_p (RS6000_PIC_OFFSET_TABLE_REGNUM))
     first_reg = RS6000_PIC_OFFSET_TABLE_REGNUM;
 
 #if TARGET_MACHO
@@ -23991,27 +24012,6 @@  rs6000_emit_move_from_cr (rtx reg)
   emit_insn (gen_movesi_from_cr (reg));
 }
 
-/* Determine whether the gp REG is really used.  */
-
-static bool
-rs6000_reg_live_or_pic_offset_p (int reg)
-{
-  /* If the function calls eh_return, claim used all the registers that would
-     be checked for liveness otherwise.  This is required for the PIC offset
-     register with -mminimal-toc on AIX, as it is advertised as "fixed" for
-     register allocation purposes in this case.  */
-
-  return (((crtl->calls_eh_return || df_regs_ever_live_p (reg))
-           && (!call_used_regs[reg]
-               || (reg == RS6000_PIC_OFFSET_TABLE_REGNUM
-		   && !TARGET_SINGLE_PIC_BASE
-                   && TARGET_TOC && TARGET_MINIMAL_TOC)))
-          || (reg == RS6000_PIC_OFFSET_TABLE_REGNUM
-	      && !TARGET_SINGLE_PIC_BASE
-              && ((DEFAULT_ABI == ABI_V4 && flag_pic != 0)
-                  || (DEFAULT_ABI == ABI_DARWIN && flag_pic))));
-}
-
 /* Return whether the split-stack arg pointer (r12) is used.  */
 
 static bool