Patchwork Fix PR rtl-optimization/45593

login
register
mail settings
Submitter Eric Botcazou
Date Sept. 15, 2010, 10:54 p.m.
Message ID <201009160054.56596.ebotcazou@adacore.com>
Download mbox | patch
Permalink /patch/64925/
State New
Headers show

Comments

Eric Botcazou - Sept. 15, 2010, 10:54 p.m.
This is a segfault compiling the kernel at -Os for the SPARC, a regression 
present on the mainline and 4.5 branch.  The crash occurs during delay slot 
scheduling, aka reorg.

We start with an insn that is put into a delay slot by fill_simple_delay_slots 
during the first reorg pass; it is only copied and not deleted.  Then it is 
put into a second delay slot by fill_eager_delay_slots, re-copied and still 
not deleted.  Then fill_eager_delay_slots, upon attempting to put it into a 
third delay slot, notices that the insn is now redundant with the copy of 
itself(!) put into the second delay slot and finally deletes it from its 
original place.  Then relax_delay_slots kicks in and remarks that the cond 
jump owning the second delay slot jumps to the next instruction; therefore it 
re-emits the insn that was in the second delay slot as a stand-alone insn and 
deletes the jump.  In the end, the insn has been copied into a delay slot and 
hoisted in the stream; in particular, its basic block changed.

These transformations are interleaved with queries to mark_target_live_regs 
which needs to know for each insn the basic block it can use to start 
computing liveness info from.  The results are cached in a hash table indexed 
by the INSN_UID of the insns.  Although insns are copied as a whole when they 
are put into delay slots, INSN_UID included, this works because m_t_l_r 
doesn't look within SEQUENCEs to compute the basic block; only the enclosing 
insn is looked at and these insns don't move.

Things go awry when relax_delay_slots re-emits insns that were in delay slots 
as stand-alone insns, because they are re-emitted as-is and thus we end up 
with several top-level insns with the same INSN_UID in the stream, which of 
course mightily confuses the caching done for mark_target_live_regs.  That's 
what happens here and causes the crash during the second reorg pass.

A radical fix could be to hash the insns themselves instead of their INSN_UID 
but that's probably overkill.  So the patch just makes sure the INSN_UID of 
the insns is changed where they are re-emitted in relax_delay_slots.

Bootstrapped/regtesed on sparc-sun-solaris2.8 and sparc64-sun-solaris2.9, 
applied on the mainline and 4.5 branch.


2010-09-15  Eric Botcazou  <ebotcazou@adacore.com>

	PR rtl-optimization/45593
	* reorg.c (relax_delay_slots): Use emit_copy_of_insn_after to re-emit
	insns that were in delay slots as stand-alone insns.


2010-09-15  Eric Botcazou  <ebotcazou@adacore.com>

	* gcc.c-torture/compile/20100915-1.c: New test.

Patch

Index: reorg.c
===================================================================
--- reorg.c	(revision 164211)
+++ reorg.c	(working copy)
@@ -3459,9 +3459,13 @@  relax_delay_slots (rtx first)
 	     We do this by deleting the INSN containing the SEQUENCE, then
 	     re-emitting the insns separately, and then deleting the RETURN.
 	     This allows the count of the jump target to be properly
-	     decremented.  */
+	     decremented.
 
-	  /* Clear the from target bit, since these insns are no longer
+	     Note that we need to change the INSN_UID of the re-emitted insns
+	     since it is used to hash the insns for mark_target_live_regs and
+	     the re-emitted insns will no longer be wrapped up in a SEQUENCE.
+
+	     Clear the from target bit, since these insns are no longer
 	     in delay slots.  */
 	  for (i = 0; i < XVECLEN (pat, 0); i++)
 	    INSN_FROM_TARGET_P (XVECEXP (pat, 0, i)) = 0;
@@ -3469,13 +3473,10 @@  relax_delay_slots (rtx first)
 	  trial = PREV_INSN (insn);
 	  delete_related_insns (insn);
 	  gcc_assert (GET_CODE (pat) == SEQUENCE);
-	  after = trial;
-	  for (i = 0; i < XVECLEN (pat, 0); i++)
-	    {
-	      rtx this_insn = XVECEXP (pat, 0, i);
-	      add_insn_after (this_insn, after, NULL);
-	      after = this_insn;
-	    }
+	  add_insn_after (delay_insn, trial, NULL);
+	  after = delay_insn;
+	  for (i = 1; i < XVECLEN (pat, 0); i++)
+	    after = emit_copy_of_insn_after (XVECEXP (pat, 0, i), after);
 	  delete_scheduled_jump (delay_insn);
 	  continue;
 	}
@@ -3580,9 +3581,13 @@  relax_delay_slots (rtx first)
 	     We do this by deleting the INSN containing the SEQUENCE, then
 	     re-emitting the insns separately, and then deleting the jump.
 	     This allows the count of the jump target to be properly
-	     decremented.  */
+	     decremented.
 
-	  /* Clear the from target bit, since these insns are no longer
+	     Note that we need to change the INSN_UID of the re-emitted insns
+	     since it is used to hash the insns for mark_target_live_regs and
+	     the re-emitted insns will no longer be wrapped up in a SEQUENCE.
+
+	     Clear the from target bit, since these insns are no longer
 	     in delay slots.  */
 	  for (i = 0; i < XVECLEN (pat, 0); i++)
 	    INSN_FROM_TARGET_P (XVECEXP (pat, 0, i)) = 0;
@@ -3590,13 +3595,10 @@  relax_delay_slots (rtx first)
 	  trial = PREV_INSN (insn);
 	  delete_related_insns (insn);
 	  gcc_assert (GET_CODE (pat) == SEQUENCE);
-	  after = trial;
-	  for (i = 0; i < XVECLEN (pat, 0); i++)
-	    {
-	      rtx this_insn = XVECEXP (pat, 0, i);
-	      add_insn_after (this_insn, after, NULL);
-	      after = this_insn;
-	    }
+	  add_insn_after (delay_insn, trial, NULL);
+	  after = delay_insn;
+	  for (i = 1; i < XVECLEN (pat, 0); i++)
+	    after = emit_copy_of_insn_after (XVECEXP (pat, 0, i), after);
 	  delete_scheduled_jump (delay_insn);
 	  continue;
 	}