Patchwork bb-reorder maintenance [2/n]

login
register
mail settings
Submitter Richard Henderson
Date July 18, 2011, 6:48 p.m.
Message ID <4E248011.20209@redhat.com>
Download mbox | patch
Permalink /patch/105345/
State New
Headers show

Comments

Richard Henderson - July 18, 2011, 6:48 p.m.
Split out a helper subroutine to insert a barrier,
after we've modified a basic block to not fallthru.

Tested on x86_64-linux and committed.


r~
* bb-reorder.c (emit_barrier_after_bb): Split out of ...
	(add_labels_and_missing_jumps): ... here.
	(fix_up_fall_thru_edges, fix_crossing_conditional_branches): Use it.
Steven Bosscher - July 18, 2011, 7:09 p.m.
On Mon, Jul 18, 2011 at 8:48 PM, Richard Henderson <rth@redhat.com> wrote:
> Split out a helper subroutine to insert a barrier,
> after we've modified a basic block to not fallthru.

Huh, aren't barriers emitted automatically when going out of cfglayout mode??

Ciao!
Steven
Richard Henderson - July 18, 2011, 7:40 p.m.
On 07/18/2011 12:09 PM, Steven Bosscher wrote:
> On Mon, Jul 18, 2011 at 8:48 PM, Richard Henderson <rth@redhat.com> wrote:
>> Split out a helper subroutine to insert a barrier,
>> after we've modified a basic block to not fallthru.
> 
> Huh, aren't barriers emitted automatically when going out of cfglayout mode??

Are they?  I didn't even think to look...

I wonder how much of this code is completely redundant with cfglayout
mode.  Certainly there seems to be code in force_nonfallthru that seems
to do the same job as two of the bb-reorder-and-partition routines...


r~
Richard Henderson - July 18, 2011, 7:50 p.m.
On 07/18/2011 12:09 PM, Steven Bosscher wrote:
> On Mon, Jul 18, 2011 at 8:48 PM, Richard Henderson <rth@redhat.com> wrote:
>> Split out a helper subroutine to insert a barrier,
>> after we've modified a basic block to not fallthru.
> 
> Huh, aren't barriers emitted automatically when going out of cfglayout mode??

... only for conditional branches.  We appear to assume that unconditional
branches already have a barrier in the footer.

See fixup_reorder_chain, and the location of the only call to emit_barrier_after.

I'm really not keen on how cfglayout.c introduces new REG_CROSSING_JUMP notes.
Given the fixups we perform in bb-reorder.c for !HAS_LONG_COND_BRANCH and
!HAS_LONG_UNCOND_BRANCH, it seems like the simplistic code in cfglayout.c is
likely only going to work for targets like x86...

I wonder what happens if I simply assert that pass_bbpart has already done
the right thing...


r~

Patch

diff --git a/gcc/bb-reorder.c b/gcc/bb-reorder.c
index 81369ea..2660551 100644
--- a/gcc/bb-reorder.c
+++ b/gcc/bb-reorder.c
@@ -1249,6 +1249,15 @@  find_rarely_executed_basic_blocks_and_crossing_edges (void)
   return crossing_edges;
 }
 
+/* Emit a barrier into the footer of BB.  */
+
+static void
+emit_barrier_after_bb (basic_block bb)
+{
+  rtx barrier = emit_barrier_after (BB_END (bb));
+  bb->il.rtl->footer = unlink_insn_chain (barrier, barrier);
+}
+
 /* If any destination of a crossing edge does not have a label, add label;
    Convert any easy fall-through crossing edges to unconditional jumps.  */
 
@@ -1262,7 +1271,7 @@  add_labels_and_missing_jumps (VEC(edge, heap) *crossing_edges)
     {
       basic_block src = e->src;
       basic_block dest = e->dest;
-      rtx label, barrier, new_jump;
+      rtx label, new_jump;
 
       if (dest == EXIT_BLOCK_PTR)
 	continue;
@@ -1288,10 +1297,10 @@  add_labels_and_missing_jumps (VEC(edge, heap) *crossing_edges)
 
       new_jump = emit_jump_insn_after (gen_jump (label), BB_END (src));
       BB_END (src) = new_jump;
-      barrier = emit_barrier_after (new_jump);
       JUMP_LABEL (new_jump) = label;
       LABEL_NUSES (label) += 1;
-      src->il.rtl->footer = unlink_insn_chain (barrier, barrier);
+
+      emit_barrier_after_bb (src);
 
       /* Mark edge as non-fallthru.  */
       e->flags &= ~EDGE_FALLTHRU;
@@ -1321,7 +1330,6 @@  fix_up_fall_thru_edges (void)
   int invert_worked;
   rtx old_jump;
   rtx fall_thru_label;
-  rtx barrier;
 
   FOR_EACH_BB (cur_bb)
     {
@@ -1451,19 +1459,7 @@  fix_up_fall_thru_edges (void)
                     }
 
 		  /* Add barrier after new jump */
-
-		  if (new_bb)
-		    {
-		      barrier = emit_barrier_after (BB_END (new_bb));
-		      new_bb->il.rtl->footer = unlink_insn_chain (barrier,
-							       barrier);
-		    }
-		  else
-		    {
-		      barrier = emit_barrier_after (BB_END (cur_bb));
-		      cur_bb->il.rtl->footer = unlink_insn_chain (barrier,
-							       barrier);
-		    }
+		  emit_barrier_after_bb (new_bb ? new_bb : cur_bb);
 		}
 	    }
 	}
@@ -1537,7 +1533,6 @@  fix_crossing_conditional_branches (void)
   rtx old_label = NULL_RTX;
   rtx new_label;
   rtx new_jump;
-  rtx barrier;
 
  last_bb = EXIT_BLOCK_PTR->prev_bb;
 
@@ -1629,11 +1624,10 @@  fix_crossing_conditional_branches (void)
 		      new_jump = emit_jump_insn_after (gen_return (),
 						       BB_END (new_bb));
 		    }
-
-		  barrier = emit_barrier_after (new_jump);
 		  JUMP_LABEL (new_jump) = old_label;
-		  new_bb->il.rtl->footer = unlink_insn_chain (barrier,
-							   barrier);
+		  BB_END (new_bb) = new_jump;
+
+		  emit_barrier_after_bb (new_bb);
 
 		  /* Make sure new bb is in same partition as source
 		     of conditional branch.  */