public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Sandiford <rsandifo@nildram.co.uk>
To: gcc-patches@gcc.gnu.org
Subject: RFA: PR 32406: GP initialisation removed for nonlocal jumps
Date: Mon, 12 Nov 2007 21:19:00 -0000	[thread overview]
Message-ID: <87y7d3urun.fsf@firetop.home> (raw)

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"

             reply	other threads:[~2007-11-12 20:33 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-12 21:19 Richard Sandiford [this message]
2007-11-13 20:15 ` David Daney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87y7d3urun.fsf@firetop.home \
    --to=rsandifo@nildram.co.uk \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).