public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Fix pr41883 + 2 dups
@ 2010-01-06 18:47 Richard Henderson
  2010-02-19 18:21 ` Andrew Pinski
  0 siblings, 1 reply; 3+ messages in thread
From: Richard Henderson @ 2010-01-06 18:47 UTC (permalink / raw)
  To: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 1338 bytes --]

The EPILOGUE_BEG note is used by the dwarf2 stuff to trigger saving and 
restoring the CFA state around an epilogue that appears in the middle of 
the function.  When the note is missing, the CFA state isn't saved and 
restored, which will then lead to a crash when a subsequent epilogue is 
encountered, as the CFA state doesn't match what's being unwound.

Normally, reposition_prologue_epilogue_notes searches the blocks with 
direct edges to EXIT, in order to fixup note misplacement caused by 
scheduling.  We do not search beyond the one basic block, because if 
there are multiple epilogues we could be moving the note from a 
different epilogue.  When I rewrote r_p_e_n for epilogue unwind, I 
expected that the scheduler would not move the note farther than the 
beginning of the block actually containing the epilogue.  That 
assumption was false for sched-ebb -- the epilogue note would get moved 
to the head of the ebb.

It turns out there was existing (but unused) support for not moving a 
note "too far".  This was for the old^3 eh implementation, and hadn't 
quite been removed.  Well, that's handy, I'll just reuse the bits I need 
to keep the EPILOGUE_BEG note near the (original) first epilogue insn, 
at which point r_p_e_n can fix things up if epilogue insns get moved around.

Tested on i386 and x86_64 linux.


r~

[-- Attachment #2: z --]
[-- Type: text/plain, Size: 6430 bytes --]

	PR middle-end/41883
	* haifa-sched.c (add_to_note_list): Merge into ...
	(concat_note_lists): ... here, and ...
	(unlink_other_notes, rm_other_notes): Merge into...
	(remove_notes): ... here.  Create REG_SAVE_NOTEs for
	NOTE_INSN_EPILOGUE_BEG.  


diff --git a/gcc/haifa-sched.c b/gcc/haifa-sched.c
index 204fab6..e3efcd8 100644
--- a/gcc/haifa-sched.c
+++ b/gcc/haifa-sched.c
@@ -1803,89 +1803,87 @@ schedule_insn (rtx insn)
 
 /* Functions for handling of notes.  */
 
-/* Insert the INSN note at the end of the notes list.  */
-static void
-add_to_note_list (rtx insn, rtx *note_list_end_p)
-{
-  PREV_INSN (insn) = *note_list_end_p;
-  if (*note_list_end_p)
-    NEXT_INSN (*note_list_end_p) = insn;
-  *note_list_end_p = insn;
-}
-
 /* Add note list that ends on FROM_END to the end of TO_ENDP.  */
 void
 concat_note_lists (rtx from_end, rtx *to_endp)
 {
   rtx from_start;
 
+  /* It's easy when have nothing to concat.  */
   if (from_end == NULL)
-    /* It's easy when have nothing to concat.  */
     return;
 
+  /* It's also easy when destination is empty.  */
   if (*to_endp == NULL)
-    /* It's also easy when destination is empty.  */
     {
       *to_endp = from_end;
       return;
     }
 
   from_start = from_end;
-  /* A note list should be traversed via PREV_INSN.  */
   while (PREV_INSN (from_start) != NULL)
     from_start = PREV_INSN (from_start);
 
-  add_to_note_list (from_start, to_endp);
+  PREV_INSN (from_start) = *to_endp;
+  NEXT_INSN (*to_endp) = from_start;
   *to_endp = from_end;
 }
 
-/* Delete notes beginning with INSN and put them in the chain
-   of notes ended by NOTE_LIST.
-   Returns the insn following the notes.  */
-static rtx
-unlink_other_notes (rtx insn, rtx tail)
+/* Delete notes between HEAD and TAIL and put them in the chain
+   of notes ended by NOTE_LIST.  */
+void
+remove_notes (rtx head, rtx tail)
 {
-  rtx prev = PREV_INSN (insn);
+  rtx next_tail, prev, insn, next;
 
-  while (insn != tail && NOTE_NOT_BB_P (insn))
-    {
-      rtx next = NEXT_INSN (insn);
-      basic_block bb = BLOCK_FOR_INSN (insn);
-
-      /* Delete the note from its current position.  */
-      if (prev)
-	NEXT_INSN (prev) = next;
-      if (next)
-	PREV_INSN (next) = prev;
+  note_list = 0;
+  if (head == tail && !INSN_P (head))
+    return;
 
-      if (bb)
-        {
-          /* Basic block can begin with either LABEL or
-             NOTE_INSN_BASIC_BLOCK.  */
-          gcc_assert (BB_HEAD (bb) != insn);
+  next_tail = NEXT_INSN (tail);
+  prev = PREV_INSN (head);
+  for (insn = head; insn != next_tail; insn = next)
+    {
+      next = NEXT_INSN (insn);
+      if (!NOTE_P (insn))
+	{
+	  prev = insn;
+	  continue;
+	}
 
-          /* Check if we are removing last insn in the BB.  */
-          if (BB_END (bb) == insn)
-            BB_END (bb) = prev;
-        }
+      switch (NOTE_KIND (insn))
+	{
+	case NOTE_INSN_BASIC_BLOCK:
+	  prev = insn;
+	  continue;
 
-      /* See sched_analyze to see how these are handled.  */
-      if (NOTE_KIND (insn) != NOTE_INSN_EH_REGION_BEG
-	  && NOTE_KIND (insn) != NOTE_INSN_EH_REGION_END)
-        add_to_note_list (insn, &note_list);
+	case NOTE_INSN_EPILOGUE_BEG:
+	  if (insn != tail)
+	    {
+	      remove_insn (insn);
+	      add_reg_note (next, REG_SAVE_NOTE,
+			    GEN_INT (NOTE_INSN_EPILOGUE_BEG));
+	      break;
+	    }
+	  /* FALLTHRU */
 
-      insn = next;
-    }
+	default:
+	  remove_insn (insn);
+
+	  /* Add the note to list that ends at NOTE_LIST.  */
+	  PREV_INSN (insn) = note_list;
+	  NEXT_INSN (insn) = NULL_RTX;
+	  if (note_list)
+	    NEXT_INSN (note_list) = insn;
+	  note_list = insn;
+	  break;
+	}
 
-  if (insn == tail)
-    {
-      gcc_assert (sel_sched_p ());
-      return prev;
+      gcc_assert ((sel_sched_p () || insn != tail) && insn != head);
     }
-
-  return insn;
 }
 
+
 /* Return the head and tail pointers of ebb starting at BEG and ending
    at END.  */
 void
@@ -1939,62 +1937,6 @@ no_real_insns_p (const_rtx head, const_rtx tail)
   return 1;
 }
 
-/* Delete notes between HEAD and TAIL and put them in the chain
-   of notes ended by NOTE_LIST.  */
-static void
-rm_other_notes (rtx head, rtx tail)
-{
-  rtx next_tail;
-  rtx insn;
-
-  note_list = 0;
-  if (head == tail && (! INSN_P (head)))
-    return;
-
-  next_tail = NEXT_INSN (tail);
-  for (insn = head; insn != next_tail; insn = NEXT_INSN (insn))
-    {
-      rtx prev;
-
-      /* Farm out notes, and maybe save them in NOTE_LIST.
-         This is needed to keep the debugger from
-         getting completely deranged.  */
-      if (NOTE_NOT_BB_P (insn))
-	{
-	  prev = insn;
-	  insn = unlink_other_notes (insn, next_tail);
-
-	  gcc_assert ((sel_sched_p ()
-		       || prev != tail) && prev != head && insn != next_tail);
-	}
-    }
-}
-
-/* Same as above, but also process REG_SAVE_NOTEs of HEAD.  */
-void
-remove_notes (rtx head, rtx tail)
-{
-  /* rm_other_notes only removes notes which are _inside_ the
-     block---that is, it won't remove notes before the first real insn
-     or after the last real insn of the block.  So if the first insn
-     has a REG_SAVE_NOTE which would otherwise be emitted before the
-     insn, it is redundant with the note before the start of the
-     block, and so we have to take it out.  */
-  if (INSN_P (head))
-    {
-      rtx note;
-
-      for (note = REG_NOTES (head); note; note = XEXP (note, 1))
-	if (REG_NOTE_KIND (note) == REG_SAVE_NOTE)
-	  remove_note (head, note);
-    }
-
-  /* Remove remaining note insns from the block, save them in
-     note_list.  These notes are restored at the end of
-     schedule_block ().  */
-  rm_other_notes (head, tail);
-}
-
 /* Restore-other-notes: NOTE_LIST is the end of a chain of notes
    previously found among the insns.  Insert them just before HEAD.  */
 rtx
@@ -2315,11 +2257,9 @@ debug_ready_list (struct ready_list *ready)
   fprintf (sched_dump, "\n");
 }
 
-/* Search INSN for REG_SAVE_NOTE note pairs for
-   NOTE_INSN_EHREGION_{BEG,END}; and convert them back into
-   NOTEs.  The REG_SAVE_NOTE note following first one is contains the
-   saved value for NOTE_BLOCK_NUMBER which is useful for
-   NOTE_INSN_EH_REGION_{BEG,END} NOTEs.  */
+/* Search INSN for REG_SAVE_NOTE notes and convert them back into insn
+   NOTEs.  This is used for NOTE_INSN_EPILOGUE_BEG, so that sched-ebb
+   replaces the epilogue note in the correct basic block.  */
 void
 reemit_notes (rtx insn)
 {

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Fix pr41883 + 2 dups
  2010-01-06 18:47 Fix pr41883 + 2 dups Richard Henderson
@ 2010-02-19 18:21 ` Andrew Pinski
  2010-02-19 18:40   ` Andrew Pinski
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Pinski @ 2010-02-19 18:21 UTC (permalink / raw)
  To: Richard Henderson; +Cc: GCC Patches

On Wed, Jan 6, 2010 at 10:47 AM, Richard Henderson <rth@redhat.com> wrote:
> Tested on i386 and x86_64 linux.

This patch causes an ICE with a cross of spu-elf
(host=x86_64-linux-gnu), though it seems a generic issue.
When add_reg_note is called, we have:
(note 21 12 22 2 NOTE_INSN_DELETED)
But REG_NOTES modifies a random location (in this case insn 25) and we
end up with insn 25 with a code of 45560.

Thanks,
Andrew Pinski

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Fix pr41883 + 2 dups
  2010-02-19 18:21 ` Andrew Pinski
@ 2010-02-19 18:40   ` Andrew Pinski
  0 siblings, 0 replies; 3+ messages in thread
From: Andrew Pinski @ 2010-02-19 18:40 UTC (permalink / raw)
  To: Richard Henderson; +Cc: GCC Patches

On Fri, Feb 19, 2010 at 10:21 AM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Wed, Jan 6, 2010 at 10:47 AM, Richard Henderson <rth@redhat.com> wrote:
>> Tested on i386 and x86_64 linux.
>
> This patch causes an ICE with a cross of spu-elf
> (host=x86_64-linux-gnu), though it seems a generic issue.
> When add_reg_note is called, we have:
> (note 21 12 22 2 NOTE_INSN_DELETED)
> But REG_NOTES modifies a random location (in this case insn 25) and we
> end up with insn 25 with a code of 45560.

I should mention a simple program:
int main(void) { return 0; }
Compiled at -O2 is enough to reproduce the ICE.

Thanks,
Andrew Pinski

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-02-19 18:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-01-06 18:47 Fix pr41883 + 2 dups Richard Henderson
2010-02-19 18:21 ` Andrew Pinski
2010-02-19 18:40   ` Andrew Pinski

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).