RFA: PR 32406: GP initialisation removed for nonlocal jumps

RFA: PR 32406: GP initialisation removed for nonlocal jumps

[Richard Sandiford]

This patch fixes nestfunc-6.c for MIPS -mabi={n32,64} -mabicalls
-mno-shared.

nestfunc-6.c checks that nonlocal gotos work correctly on targets with
module-local GPs, even if the value of the GP register on entry to the
nested function is not the right one for that module.  As RTH noted when
adding this testcase:

   http://gcc.gnu.org/ml/gcc-patches/2003-12/msg00573.html

the onus is on the nested function to set up the GP for the target
of the jump, and the MIPS prologue code ensures that this happens.
Unfortunately, nothing in the dataflow exposes the fact that the target
needs the GP register to be valid, so the prologue initialisation gets
deleted as dead.

I think this is a dataflow bug.  (Dataflow generally, that is, not the
new df machinery.)  The code that emits the nonlocal goto records that
the target of the jump uses the frame and stack pointers:

static rtx
expand_builtin_nonlocal_goto (tree exp)
{
  ...
      /* USE of hard_frame_pointer_rtx added for consistency;
         not clear if really needed.  */
      emit_insn (gen_rtx_USE (VOIDmode, hard_frame_pointer_rtx));
      emit_insn (gen_rtx_USE (VOIDmode, stack_pointer_rtx));

but it doesn't record that it also uses the GP register.  The target-
independent code does handle the liveness of GP in other situations,
thanks originally to flow and now to the df machinery, so I think this
is simply an oversight.  (MIPS -mno-shared code is probably an unusual
case here.  Most targets that use a GP register and module-local GP
values will need the GP register to load the target of the jump,
so the initialisation of the GP register is kept live that way.)

Long-term, it would be nice to get rid of these sorts of USEs and
handle them entirely in the df machinery.  However, given that we
don't do that yet, I think the best fix is to emit a use of
pic_offset_table_rtx:

      if ((unsigned) PIC_OFFSET_TABLE_REGNUM != INVALID_REGNUM
          && fixed_regs[PIC_OFFSET_TABLE_REGNUM])
        emit_insn (gen_rtx_USE (VOIDmode, pic_offset_table_rtx));

with the condition being the same as that used in flow and df.

There was an earlier patch to fix this for o32 in the MIPS backend
itself, but I'd forgotten when reviewing that how this was supposed
to work.  The change to expand_builtin_nonlocal_goto makes the earlier 
patch redundant, so I've reverted that.

Bootstrapped & regression-tested on x86_64-linux-gnu.  Also
regression-tested on mips-linux-gnu (which showed the o32 bug)
and mipsisa64-elfoabi.  OK to install?

Richard


gcc/
	PR target/32406
	* builtins.c (expand_builtin_nonlocal_goto): Also emit a use
	of GP register, if valid and fixed.

	Revert:
	2007-06-21  David Daney  <ddaney@avtrex.com>

	PR target/32406
	* config/mips/mips.md (define_constants): Rename UNSPEC_EH_RECEIVER
	to UNSPEC_NONLOCAL_GOTO_RECEIVER globally.
	(exception_receiver): Renamed to ...
	(nonlocal_goto_receiver): ... this.

Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c	(revision 130038)
+++ gcc/builtins.c	(working copy)
@@ -909,6 +909,20 @@ expand_builtin_nonlocal_goto (tree exp)
 	 not clear if really needed.  */
       emit_insn (gen_rtx_USE (VOIDmode, hard_frame_pointer_rtx));
       emit_insn (gen_rtx_USE (VOIDmode, stack_pointer_rtx));
+
+      /* If the architecture is using a GP register, we must
+	 conservatively assume that the target function makes use of it.
+	 The prologue of functions with nonlocal gotos must therefore
+	 initialize the GP register to the appropriate value, and we
+	 must then make sure that this value is live at the point
+	 of the jump.  (Note that this doesn't necessarily apply
+	 to targets with a nonlocal_goto pattern; they are free
+	 to implement it in their own way.  Note also that this is
+	 a no-op if the GP register is a global invariant.)  */
+      if ((unsigned) PIC_OFFSET_TABLE_REGNUM != INVALID_REGNUM
+	  && fixed_regs[PIC_OFFSET_TABLE_REGNUM])
+	emit_insn (gen_rtx_USE (VOIDmode, pic_offset_table_rtx));
+
       emit_indirect_jump (r_label);
     }
 
Index: gcc/config/mips/mips.md
===================================================================
--- gcc/config/mips/mips.md	(revision 130038)
+++ gcc/config/mips/mips.md	(working copy)
@@ -30,7 +30,7 @@ (define_constants
    (UNSPEC_GET_FNADDR		 3)
    (UNSPEC_BLOCKAGE		 4)
    (UNSPEC_CPRESTORE		 5)
-   (UNSPEC_NONLOCAL_GOTO_RECEIVER 6)
+   (UNSPEC_EH_RECEIVER		 6)
    (UNSPEC_EH_RETURN		 7)
    (UNSPEC_CONSTTABLE_INT	 8)
    (UNSPEC_CONSTTABLE_FLOAT	 9)
@@ -5598,9 +5598,9 @@ (define_split
   DONE;
 })
 
-(define_insn_and_split "nonlocal_goto_receiver"
+(define_insn_and_split "exception_receiver"
   [(set (reg:SI 28)
-	(unspec_volatile:SI [(const_int 0)] UNSPEC_NONLOCAL_GOTO_RECEIVER))]
+	(unspec_volatile:SI [(const_int 0)] UNSPEC_EH_RECEIVER))]
   "TARGET_CALL_CLOBBERED_GP"
   "#"
   "&& reload_completed"

Re: RFA: PR 32406: GP initialisation removed for nonlocal jumps

[David Daney]

Richard Sandiford wrote:

> gcc/
> 	PR target/32406
> 	* builtins.c (expand_builtin_nonlocal_goto): Also emit a use
> 	of GP register, if valid and fixed.
> 
> 	Revert:
> 	2007-06-21  David Daney  <ddaney@avtrex.com>
> 
> 	PR target/32406
> 	* config/mips/mips.md (define_constants): Rename UNSPEC_EH_RECEIVER
> 	to UNSPEC_NONLOCAL_GOTO_RECEIVER globally.
> 	(exception_receiver): Renamed to ...
> 	(nonlocal_goto_receiver): ... this.
> 

FWIW, I just bootstrapped this patch on mipsel-linux-gnu with all 
default languages and there were no regressions.

David Daney