diff mbox

[PR46204] sel-sched: delete empty BBs more carefully

Message ID alpine.LNX.2.00.1011101603160.6883@monoid.intra.ispras.ru
State New
Headers show

Commit Message

Alexander Monakov Nov. 10, 2010, 5:26 p.m. UTC 
Hello,

When deleting an empty basic block, we redirect all incoming non-fallthrough
edges to its successor.  However, we must also take care of the incoming
fallthrough edge if the preceding basic block ends with a conditional jump to
the next instruction (otherwise, by deleting the empty basic block we can
remove the label that the jump refers to).  Fixed in maybe_tidy_empty_bb by
trying to remove the jump or redirecting it to the successor of the empty block.

The patch also removes recomputing of topological order in that function.  We
can not invalidate it by moving an edge destination along an existing edge.  I
have tried to add the corresponding assert, but it would be quite verbose,
since the fallthrough edge may be a back edge, in which case it will look like
we are invalidating toporder, even though we are legally creating a new back
edge in the region, and will delete the old back edge soon (we can create
multiple back edges that way, but we'd only care when pipelining, and we
wouldn't have a fallthrough back edge then).

Bootstrapped and regtested on ia64 with sel-sched enabled at -O2, also tested
on x86-64 with sel-sched enabled for bootstrap.  OK?

2010-11-10  Alexander Monakov  <amonakov@ispras.ru>

	PR rtl-optimization/46204
	* sel-sched-ir.c (maybe_tidy_empty_bb): Remove second argument.
	Update all callers.  Do not recompute topological order.  Adjust
	fallthrough edges following a degenerate conditional jump.

Comments

Vladimir Makarov Nov. 12, 2010, 10:15 p.m. UTC | #1 
On 11/10/2010 12:26 PM, Alexander Monakov wrote:
> Hello,
>
> When deleting an empty basic block, we redirect all incoming non-fallthrough
> edges to its successor.  However, we must also take care of the incoming
> fallthrough edge if the preceding basic block ends with a conditional jump to
> the next instruction (otherwise, by deleting the empty basic block we can
> remove the label that the jump refers to).  Fixed in maybe_tidy_empty_bb by
> trying to remove the jump or redirecting it to the successor of the empty block.
>
> The patch also removes recomputing of topological order in that function.  We
> can not invalidate it by moving an edge destination along an existing edge.  I
> have tried to add the corresponding assert, but it would be quite verbose,
> since the fallthrough edge may be a back edge, in which case it will look like
> we are invalidating toporder, even though we are legally creating a new back
> edge in the region, and will delete the old back edge soon (we can create
> multiple back edges that way, but we'd only care when pipelining, and we
> wouldn't have a fallthrough back edge then).
>
> Bootstrapped and regtested on ia64 with sel-sched enabled at -O2, also tested
> on x86-64 with sel-sched enabled for bootstrap.  OK?
>
As I understand g++.dg/torture/stackalign/throw-1.C is the test.

The patch looks ok for me.  Thank you for the patch, Alexander.
diff mbox

Patch

diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
index 141c935..e169276 100644
--- a/gcc/sel-sched-ir.c
+++ b/gcc/sel-sched-ir.c
@@ -3562,7 +3562,7 @@  sel_recompute_toporder (void)
 
 /* Tidy the possibly empty block BB.  */
 static bool
-maybe_tidy_empty_bb (basic_block bb, bool recompute_toporder_p)
+maybe_tidy_empty_bb (basic_block bb)
 {
   basic_block succ_bb, pred_bb;
   edge e;
@@ -3612,10 +3612,29 @@  maybe_tidy_empty_bb (basic_block bb, bool recompute_toporder_p)
 
           if (!(e->flags & EDGE_FALLTHRU))
             {
-              recompute_toporder_p |= sel_redirect_edge_and_branch (e, succ_bb);
+	      /* We can not invalidate computed topological order by moving
+	         the edge destination block (E->SUCC) along a fallthru edge.  */
+              sel_redirect_edge_and_branch (e, succ_bb);
               rescan_p = true;
               break;
             }
+	  /* If the edge is fallthru, but PRED_BB ends in a conditional jump
+	     to BB (so there is no non-fallthru edge from PRED_BB to BB), we
+	     still have to adjust it.  */
+	  else if (single_succ_p (pred_bb) && any_condjump_p (BB_END (pred_bb)))
+	    {
+	      /* If possible, try to remove the unneeded conditional jump.  */
+	      if (INSN_SCHED_TIMES (BB_END (pred_bb)) == 0
+		  && !IN_CURRENT_FENCE_P (BB_END (pred_bb)))
+		{
+		  if (!sel_remove_insn (BB_END (pred_bb), false, false))
+		    tidy_fallthru_edge (e);
+		}
+	      else
+		sel_redirect_edge_and_branch (e, succ_bb);
+	      rescan_p = true;
+	      break;
+	    }
         }
     }
 
@@ -3631,9 +3650,6 @@  maybe_tidy_empty_bb (basic_block bb, bool recompute_toporder_p)
       remove_empty_bb (bb, true);
     }
 
-  if (recompute_toporder_p)
-    sel_recompute_toporder ();
-
 #ifdef ENABLE_CHECKING
   verify_backedges ();
 #endif
@@ -3651,7 +3667,7 @@  tidy_control_flow (basic_block xbb, bool full_tidying)
   insn_t first, last;
 
   /* First check whether XBB is empty.  */
-  changed = maybe_tidy_empty_bb (xbb, false);
+  changed = maybe_tidy_empty_bb (xbb);
   if (changed || !full_tidying)
     return changed;
 
@@ -3715,8 +3731,8 @@  tidy_control_flow (basic_block xbb, bool full_tidying)
          that contained that jump, becomes empty too.  In such case
          remove it too.  */
       if (sel_bb_empty_p (xbb->prev_bb))
-        changed = maybe_tidy_empty_bb (xbb->prev_bb, recompute_toporder_p);
-      else if (recompute_toporder_p)
+        changed = maybe_tidy_empty_bb (xbb->prev_bb);
+      if (recompute_toporder_p)
 	sel_recompute_toporder ();
     }
   return changed;
@@ -3733,7 +3749,7 @@  purge_empty_blocks (void)
     {
       basic_block b = BASIC_BLOCK (BB_TO_BLOCK (i));
 
-      if (maybe_tidy_empty_bb (b, false))
+      if (maybe_tidy_empty_bb (b))
 	continue;
 
       i++;