This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH]: Fix PR29201 - [4.2 Regression] ICE in create_recovery_block, at haifa-sched.c:3692 at -O3


Ian Lance Taylor wrote:

...

What I mean is: the test for BLOCK_FOR_INSN in your get_barrier
function makes me uncomfortable.  It leaves another issue for us to
clean up later.  I'd like to fix this problem while minimizing such
issues.

...


Yes.  We have already hardcoded this structure in other places, e.g.,
expand_gimple_basic_block in cfgexpand.c, rtl_delete_block in
cfgrtl.c, etc.  I agree that if we go this route, we should push into
a small function somewhere else, not in haifa-sched.c.

Here are two patches which do the same thing in a slightly different way. Pick your choice and, please, comment.


Ian


Thanks,

Maxim
--- haifa-sched.c	(/local/trunk/gcc)	(revision 546)
+++ haifa-sched.c	(/local/fix-pr29201/gcc)	(revision 546)
@@ -3679,6 +3679,7 @@ static basic_block
 create_recovery_block (void)
 {
   rtx label;
+  rtx barrier;
   basic_block rec;
   
   added_recovery_block_p = true;
@@ -3686,11 +3687,27 @@ create_recovery_block (void)
   if (!before_recovery)
     init_before_recovery ();
  
-  label = gen_label_rtx ();
-  gcc_assert (BARRIER_P (NEXT_INSN (BB_END (before_recovery))));
-  label = emit_label_after (label, NEXT_INSN (BB_END (before_recovery)));
+  /* Get the control flow barrier.
+     Typically next insn (let's say B) after the bb end is that barrier.
+     But in case of a jump table B can be something else like a label or a
+     note and this helper returns the barrier after the jump table.
+     NB: table jump consists of a jump insn and an addr_vec expression which
+     contains the labels the jump can branch to.  Everything that follows
+     the jump insn is not in any basic block.  */
+  barrier = NEXT_INSN (BB_END (before_recovery));
+  while (!BARRIER_P (barrier))
+    {
+      /* We must encounter a barrier before we reach the next bb.  */
+      gcc_assert (BLOCK_FOR_INSN (barrier) == NULL);
+
+      barrier = NEXT_INSN (barrier);
+    }
 
-  rec = create_basic_block (label, label, before_recovery); 
+  label = emit_label_after (gen_label_rtx (), barrier);
+
+  rec = create_basic_block (label, label, before_recovery);
+
+  /* Recovery block always end with an unconditional jump.  */
   emit_barrier_after (BB_END (rec));
 
   if (BB_PARTITION (before_recovery) != BB_UNPARTITIONED)
@@ -4253,14 +4270,20 @@ extend_bb (basic_block bb)
 
   insn = BB_END (EXIT_BLOCK_PTR->prev_bb);
   if (NEXT_INSN (insn) == 0
+      /* Or INSN is not an empty basic block followed by a barrier.
+	 ??? This condition looks redundant to me.  More than that, it
+	 triggers a note between a table jump and a description of that jump.
+      */
       || (!NOTE_P (insn)
 	  && !LABEL_P (insn)
 	  /* Don't emit a NOTE if it would end up before a BARRIER.  */
 	  && !BARRIER_P (NEXT_INSN (insn))))
     {
-      emit_note_after (NOTE_INSN_DELETED, insn);
-      /* Make insn to appear outside BB.  */
+      rtx note = emit_note_after (NOTE_INSN_DELETED, insn);
+
+      /* Make NOTE to appear outside bb.  */
       BB_END (EXIT_BLOCK_PTR->prev_bb) = insn;
+      BLOCK_FOR_INSN (note) = NULL;
     }
 }
 
--- haifa-sched.c	(/local/trunk/gcc)	(revision 548)
+++ haifa-sched.c	(/local/fix-pr29201-1/gcc)	(revision 548)
@@ -3679,6 +3679,7 @@ static basic_block
 create_recovery_block (void)
 {
   rtx label;
+  rtx barrier;
   basic_block rec;
   
   added_recovery_block_p = true;
@@ -3686,11 +3687,23 @@ create_recovery_block (void)
   if (!before_recovery)
     init_before_recovery ();
  
-  label = gen_label_rtx ();
-  gcc_assert (BARRIER_P (NEXT_INSN (BB_END (before_recovery))));
-  label = emit_label_after (label, NEXT_INSN (BB_END (before_recovery)));
+  /* Get the control flow barrier.
+     Typically next insn (let's say B) after the bb end is that barrier.
+     But in case of a jump table B can be something else like a label or a
+     note and this helper returns the barrier after the jump table.
+     NB: table jump consists of a jump insn and an addr_vec expression which
+     contains the labels the jump can branch to.  Everything that follows
+     the jump insn is not in any basic block.  */
+  barrier = next_nonnote_insn (BB_END (before_recovery));
+  if (LABEL_P (barrier) && JUMP_TABLE_DATA_P (NEXT_INSN (barrier)))
+    barrier = NEXT_INSN (NEXT_INSN (barrier));
+  gcc_assert (BARRIER_P (barrier));
 
-  rec = create_basic_block (label, label, before_recovery); 
+  label = emit_label_after (gen_label_rtx (), barrier);
+
+  rec = create_basic_block (label, label, before_recovery);
+
+  /* Recovery block always end with an unconditional jump.  */
   emit_barrier_after (BB_END (rec));
 
   if (BB_PARTITION (before_recovery) != BB_UNPARTITIONED)

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]