diff mbox

Copy over REG_ARGS_SIZE notes in sel-sched (PR debug/51557)

Message ID 20111215173250.GQ1957@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Dec. 15, 2011, 5:32 p.m. UTC 
Hi!

On the following testcase we ICE on x86_64-linux because sel-sched
decides to duplicate a push insn, the original one remains in one bb,
the copy in a different bb which have both a common successor.
Unfortunately the REG_ARGS_SIZE note isn't copied over, so during the
dwarf2 pass we don't notice the needed adjustment of the argument
size in one of the bb's and on the merge label we have different
CFA expressions.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
ok for trunk?

2011-12-15  Jakub Jelinek  <jakub@redhat.com>

	PR debug/51557
	* sel-sched-ir.c (create_copy_of_insn_rtx): Copy REG_ARGS_SIZE
	notes.

	* gcc.dg/pr51557.c: New test.


	Jakub

Comments

Richard Henderson Dec. 15, 2011, 5:37 p.m. UTC | #1 
On 12/15/2011 09:32 AM, Jakub Jelinek wrote:
> 	PR debug/51557
> 	* sel-sched-ir.c (create_copy_of_insn_rtx): Copy REG_ARGS_SIZE
> 	notes.
> 
> 	* gcc.dg/pr51557.c: New test.

There are plenty of other notes that could as well need duplication.
E.g. REG_INC, REG_FRAME_RELATED_EXPR, REG_CFA_*, REG_EH_REGION.

And that's assuming that call insns are never duplicated.

Why not simply copy all of the notes?


r~
Jakub Jelinek Dec. 15, 2011, 8:42 p.m. UTC | #2 
On Thu, Dec 15, 2011 at 09:37:45AM -0800, Richard Henderson wrote:
> On 12/15/2011 09:32 AM, Jakub Jelinek wrote:
> > 	PR debug/51557
> > 	* sel-sched-ir.c (create_copy_of_insn_rtx): Copy REG_ARGS_SIZE
> > 	notes.
> > 
> > 	* gcc.dg/pr51557.c: New test.
> 
> There are plenty of other notes that could as well need duplication.
> E.g. REG_INC, REG_FRAME_RELATED_EXPR, REG_CFA_*, REG_EH_REGION.

I wasn't sure if it is safe to duplicate them.  I guess most of them are
safe, not sure about REG_EQUAL/REG_EQUIV.
I can certainly try to copy all the notes, e.g.
emit_copy_of_insn_after seems to copy all of them but REG_LABEL_OPERAND.

	Jakub
Andrey Belevantsev Dec. 16, 2011, 5:47 a.m. UTC | #3 
On 16.12.2011 0:42, Jakub Jelinek wrote:
> On Thu, Dec 15, 2011 at 09:37:45AM -0800, Richard Henderson wrote:
>> On 12/15/2011 09:32 AM, Jakub Jelinek wrote:
>>> 	PR debug/51557
>>> 	* sel-sched-ir.c (create_copy_of_insn_rtx): Copy REG_ARGS_SIZE
>>> 	notes.
>>>
>>> 	* gcc.dg/pr51557.c: New test.
>>
>> There are plenty of other notes that could as well need duplication.
>> E.g. REG_INC, REG_FRAME_RELATED_EXPR, REG_CFA_*, REG_EH_REGION.
>
> I wasn't sure if it is safe to duplicate them.  I guess most of them are
> safe, not sure about REG_EQUAL/REG_EQUIV.
> I can certainly try to copy all the notes, e.g.
> emit_copy_of_insn_after seems to copy all of them but REG_LABEL_OPERAND.
We need to clone only those insns that are safe.  In 
sel-sched-ir.c:init_global_and_expr_for_insn we check a number of 
conditions that indicate the insn being un-copyable (force_unique_p is set 
to true for those; the relevant code starts from line 2954).  Previously I 
fixed this about a year ago wrt prologue and epilogue insns, you can check 
the thread with the http://gcc.gnu.org/ml/gcc-patches/2010-10/msg00013.html 
message.

Thus, if an insn has any note that prevents it from being cloned, we need 
to fix the above place.  (If an insn should not even be moved from its 
block, then CANT_MOVE should be set to 1 in the same function.)  Then 
create_copy_of_insn_rtx can safely copy all the notes as we already know 
that only clonable insns get to this function. The REG_EQUAL/REG_EQUIV 
notes may be dropped -- I thought these are supposed to be recomputed by df 
these days, am I wrong?

Jakub, I am happy to prepare a patch along these lines and to adjust it to 
mark un-copyable the insns with any notes you or Richard think may be 
problematic.  It would also be great to have this information recorded in a 
general helper in rtlanal.c or somewhere, so we don't have to guess every 
time this comes up; what do you think?

Andrey

>
> 	Jakub
diff mbox

Patch

--- gcc/sel-sched-ir.c.jj	2011-10-20 14:13:43.000000000 +0200
+++ gcc/sel-sched-ir.c	2011-12-15 00:35:05.968011345 +0100
@@ -5723,7 +5723,7 @@  create_vinsn_from_insn_rtx (rtx insn_rtx
 rtx
 create_copy_of_insn_rtx (rtx insn_rtx)
 {
-  rtx res;
+  rtx res, note;
 
   if (DEBUG_INSN_P (insn_rtx))
     return create_insn_rtx_from_pattern (copy_rtx (PATTERN (insn_rtx)),
@@ -5733,6 +5733,10 @@  create_copy_of_insn_rtx (rtx insn_rtx)
 
   res = create_insn_rtx_from_pattern (copy_rtx (PATTERN (insn_rtx)),
                                       NULL_RTX);
+  /* Duplicate REG_ARGS_SIZE note if any.  */
+  note = find_reg_note (insn_rtx, REG_ARGS_SIZE, NULL_RTX);
+  if (note)
+    add_reg_note (res, REG_ARGS_SIZE, XEXP (note, 0));
   return res;
 }
 
--- gcc/testsuite/gcc.dg/pr51557.c.jj	2011-12-15 00:38:36.417105928 +0100
+++ gcc/testsuite/gcc.dg/pr51557.c	2011-12-15 00:38:27.000000000 +0100
@@ -0,0 +1,17 @@ 
+/* PR debug/51557 */
+/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+/* { dg-options "-Os -fno-asynchronous-unwind-tables -g -fsel-sched-pipelining -fselective-scheduling2" } */
+
+extern int baz (void);
+extern void bar (int, int, int, int, int, int, int);
+
+void
+synth (int *values, int n_values, int ci, int s1, int v, int s2)
+{
+  while (--s1)
+    {
+      int r1 = values[s1];
+      int co = ci ? r1 : baz () < r1;
+      bar (0, n_values, s1, s2, v, co, 0);
+    }
+}