diff mbox

PR 48826: NOTE_INSN_CALL_ARG_LOCATION vs. define_split

Message ID 87fwnyhh4o.fsf@firetop.home
State New
Headers show

Commit Message

Richard Sandiford May 28, 2011, 5:25 p.m. UTC 
Richard Sandiford <rdsandiford@googlemail.com> writes:
> gcc/
> 	PR rtl-optimization/48826
> 	* emit-rtl.c (try_split): When splitting a call that is followed
> 	by a NOTE_INSN_CALL_ARG_LOCATION, move the note after the new call.

It turns out that this isn't enough.  In the attached testcase,
we have a:

    (var_location pc (nil))

note between the call and the CALL_ARG_LOCATION.  A quick look
at the var-tracking flow suggests that this is legitimate,
so we need to look at every note before the next real insn,
rather than just looking at the first.

I assume the ARM, SH and S390 reorg code would also need to cope
with this case.

If this usage isn't legitimate, and the CALL_ARG_LOCATION is supposed
to come directly after the call, then it would be great if someone more
familiar with the var-tracking code could have a look at it.

Otherwise, patch bootstrapped & regression tested on x86_64-linux-gnu.
Also tested on mips-linux-gnu.  OK to install?

Richard


gcc/
	* emit-rtl.c (try_split): Use a loop to search for
	NOTE_INSN_CALL_ARG_LOCATIONs.

gcc/testsuite/
	From Ryan Mansfield
	* gcc.dg/pr48826.c: New test.

Comments

Eric Botcazou May 29, 2011, 8:44 a.m. UTC | #1 
> If this usage isn't legitimate, and the CALL_ARG_LOCATION is supposed
> to come directly after the call, then it would be great if someone more
> familiar with the var-tracking code could have a look at it.

"Be liberal in what you accept" says the common wisdom.

> gcc/
> 	* emit-rtl.c (try_split): Use a loop to search for
> 	NOTE_INSN_CALL_ARG_LOCATIONs.
>
> gcc/testsuite/
> 	From Ryan Mansfield
> 	* gcc.dg/pr48826.c: New test.

OK, thanks.
diff mbox

Patch

Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c	2011-05-28 17:08:48.000000000 +0100
+++ gcc/emit-rtl.c	2011-05-28 18:23:28.000000000 +0100
@@ -3494,16 +3494,15 @@  try_split (rtx pat, rtx trial, int last)
 	       we must move any following NOTE_INSN_CALL_ARG_LOCATION note
 	       so that it comes immediately after the new call.  */
 	    if (NEXT_INSN (insn))
-	      {
-		next = NEXT_INSN (trial);
-		if (next
-		    && NOTE_P (next)
-		    && NOTE_KIND (next) == NOTE_INSN_CALL_ARG_LOCATION)
+	      for (next = NEXT_INSN (trial);
+		   next && NOTE_P (next);
+		   next = NEXT_INSN (next))
+		if (NOTE_KIND (next) == NOTE_INSN_CALL_ARG_LOCATION)
 		  {
 		    remove_insn (next);
 		    add_insn_after (next, insn, NULL);
+		    break;
 		  }
-	      }
 	  }
     }
 
Index: gcc/testsuite/gcc.dg/pr48826.c
===================================================================
--- /dev/null	2011-05-28 09:03:52.070295797 +0100
+++ gcc/testsuite/gcc.dg/pr48826.c	2011-05-28 18:12:00.000000000 +0100
@@ -0,0 +1,10 @@ 
+/* { dg-options "-O -g -w" } */
+
+void bar (int *);
+
+void
+foo ()
+{
+  int *const pc = __builtin_return_address (0);
+  bar (pc);
+}