diff mbox

PATCH RFA: Split stack [3/7]: CFG build probability

Message ID mcrocbp77xw.fsf@google.com
State New
Headers show

Commit Message

Ian Lance Taylor Sept. 22, 2010, 10:02 p.m. UTC 
This is the third of the -fsplit-stack patches.  This adds a new feature
to the CFG builder: if an edge is created because of a jump insn, and
the jump insn has a REG_BR_PROB note, then we copy the probability from
the note to the edge.  This is useful because the split stack prologue
code has a branch, and the branch has a probability.  This permits the
code to set a probability on the branch and have that probability be
carried into the CFG.

An alternative would be to have the split stack prologue code create
basic blocks itself.  However at present there is other RTL level code
which creates branches on edges, which the CFG code knows how to
incorporate into the CFG, and I see no harm in continuing this existing
facility.

This code is in the middle-end and as such I do not require approval,
but I would like to see any comments or objections to this approach.

Ian


2010-09-21  Ian Lance Taylor  <iant@google.com>

	* cfgbuild.c (make_label_edge): Add insn parameter.  Change all
	callers.  If insn has a REG_BR_PROB note, copy it to the edge
	probability.

Comments

Steven Bosscher Sept. 22, 2010, 10:08 p.m. UTC | #1 
On Thu, Sep 23, 2010 at 12:02 AM, Ian Lance Taylor <iant@google.com> wrote:
> An alternative would be to have the split stack prologue code create
> basic blocks itself.  However at present there is other RTL level code
> which creates branches on edges, which the CFG code knows how to
> incorporate into the CFG, and I see no harm in continuing this existing
> facility.

I do. It breaks when you work in cfglayout mode.

Ciao!
Steven
Ian Lance Taylor Sept. 22, 2010, 10:56 p.m. UTC | #2 
Steven Bosscher <stevenb.gcc@gmail.com> writes:

> On Thu, Sep 23, 2010 at 12:02 AM, Ian Lance Taylor <iant@google.com> wrote:
>> An alternative would be to have the split stack prologue code create
>> basic blocks itself.  However at present there is other RTL level code
>> which creates branches on edges, which the CFG code knows how to
>> incorporate into the CFG, and I see no harm in continuing this existing
>> facility.
>
> I do. It breaks when you work in cfglayout mode.

Yeah, that's what I said in the comment in the code.  This code is only
called by pass_thread_prologue_and_epilogue, which is long after we
leave CFG layout mode.  The same issue arises if the general prologue or
epilogue code has a branch, which I admit is much less common.  If you
want to help me rework thread_prologue_and_epilogue_insns so that it
works in cfglayout mode, that would be great.

Ian
Ian Lance Taylor Sept. 24, 2010, 6:08 p.m. UTC | #3 
Ian Lance Taylor <iant@google.com> writes:

> This is the third of the -fsplit-stack patches.  This adds a new feature
> to the CFG builder: if an edge is created because of a jump insn, and
> the jump insn has a REG_BR_PROB note, then we copy the probability from
> the note to the edge.  This is useful because the split stack prologue
> code has a branch, and the branch has a probability.  This permits the
> code to set a probability on the branch and have that probability be
> carried into the CFG.
>
> An alternative would be to have the split stack prologue code create
> basic blocks itself.  However at present there is other RTL level code
> which creates branches on edges, which the CFG code knows how to
> incorporate into the CFG, and I see no harm in continuing this existing
> facility.
>
> This code is in the middle-end and as such I do not require approval,
> but I would like to see any comments or objections to this approach.
>
> Ian
>
>
> 2010-09-21  Ian Lance Taylor  <iant@google.com>
>
> 	* cfgbuild.c (make_label_edge): Add insn parameter.  Change all
> 	callers.  If insn has a REG_BR_PROB note, copy it to the edge
> 	probability.

Honza questioned why this patch was needed, and he was right: it does
not appear to be required.  I do not know why I needed to write it when
I did the original work.

Please consider this patch withdrawn.  Sorry for taking up people's time
with it.

I think at this point I have received all the approvals and comments I
need for the split stack patch, and I plan to do some final testing and
go ahead and commit on Monday.

Ian
diff mbox

Patch

Index: gcc/cfgbuild.c
===================================================================
--- gcc/cfgbuild.c	(revision 164490)
+++ gcc/cfgbuild.c	(working copy)
@@ -40,7 +40,6 @@  along with GCC; see the file COPYING3.  
 #include "sbitmap.h"
 
 static void make_edges (basic_block, basic_block, int);
-static void make_label_edge (sbitmap, basic_block, rtx, int);
 static void find_bb_boundaries (basic_block);
 static void compute_outgoing_frequencies (basic_block);
 
@@ -131,14 +130,19 @@  control_flow_insn_p (const_rtx insn)
 }
 
 
-/* Create an edge between two basic blocks.  FLAGS are auxiliary information
-   about the edge that is accumulated between calls.  */
+/* Create an edge between two basic blocks.    */
 
-/* Create an edge from a basic block to a label.  */
+/* Create an edge from basic block SRC to LABEL.  Cache edges in
+   EDGE_CACHE.  If INSN is not NULL it is the jump insn which caused
+   this edge to be created.  FLAGS are auxiliary information about the
+   edge that is accumulated between calls.  */
 
 static void
-make_label_edge (sbitmap edge_cache, basic_block src, rtx label, int flags)
+make_label_edge (sbitmap edge_cache, basic_block src, rtx label, rtx insn,
+		 int flags)
 {
+  edge e;
+
   gcc_assert (LABEL_P (label));
 
   /* If the label was never emitted, this insn is junk, but avoid a
@@ -149,7 +153,14 @@  make_label_edge (sbitmap edge_cache, bas
   if (INSN_UID (label) == 0)
     return;
 
-  cached_make_edge (edge_cache, src, BLOCK_FOR_INSN (label), flags);
+  e = cached_make_edge (edge_cache, src, BLOCK_FOR_INSN (label), flags);
+
+  if (e != NULL && insn != NULL_RTX)
+    {
+      rtx note = find_reg_note (insn, REG_BR_PROB, 0);
+      if (note != NULL_RTX)
+	e->probability = INTVAL (XEXP (note, 0));
+    }
 }
 
 /* Create the edges generated by INSN in REGION.  */
@@ -170,7 +181,7 @@  rtl_make_eh_edge (sbitmap edge_cache, ba
 	  label = label_rtx (lp->post_landing_pad);
 	}
 
-      make_label_edge (edge_cache, src, label,
+      make_label_edge (edge_cache, src, label, NULL_RTX,
 		       EDGE_ABNORMAL | EDGE_EH
 		       | (CALL_P (insn) ? EDGE_ABNORMAL_CALL : 0));
     }
@@ -280,7 +291,7 @@  make_edges (basic_block min, basic_block
 
 	      for (j = GET_NUM_ELEM (vec) - 1; j >= 0; --j)
 		make_label_edge (edge_cache, bb,
-				 XEXP (RTVEC_ELT (vec, j), 0), 0);
+				 XEXP (RTVEC_ELT (vec, j), 0), NULL_RTX, 0);
 
 	      /* Some targets (eg, ARM) emit a conditional jump that also
 		 contains the out-of-range target.  Scan for these and
@@ -290,7 +301,8 @@  make_edges (basic_block min, basic_block
 		  && GET_CODE (SET_SRC (tmp)) == IF_THEN_ELSE
 		  && GET_CODE (XEXP (SET_SRC (tmp), 2)) == LABEL_REF)
 		make_label_edge (edge_cache, bb,
-				 XEXP (XEXP (SET_SRC (tmp), 2), 0), 0);
+				 XEXP (XEXP (SET_SRC (tmp), 2), 0),
+				 NULL_RTX, 0);
 	    }
 
 	  /* If this is a computed jump, then mark it as reaching
@@ -298,7 +310,8 @@  make_edges (basic_block min, basic_block
 	  else if (computed_jump_p (insn))
 	    {
 	      for (x = forced_labels; x; x = XEXP (x, 1))
-		make_label_edge (edge_cache, bb, XEXP (x, 0), EDGE_ABNORMAL);
+		make_label_edge (edge_cache, bb, XEXP (x, 0), insn,
+				 EDGE_ABNORMAL);
 	    }
 
 	  /* Returns create an exit out.  */
@@ -311,14 +324,15 @@  make_edges (basic_block min, basic_block
 	      int i, n = ASM_OPERANDS_LABEL_LENGTH (tmp);
 	      for (i = 0; i < n; ++i)
 		make_label_edge (edge_cache, bb,
-				 XEXP (ASM_OPERANDS_LABEL (tmp, i), 0), 0);
+				 XEXP (ASM_OPERANDS_LABEL (tmp, i), 0),
+				 NULL_RTX, 0);
 	    }
 
 	  /* Otherwise, we have a plain conditional or unconditional jump.  */
 	  else
 	    {
 	      gcc_assert (JUMP_LABEL (insn));
-	      make_label_edge (edge_cache, bb, JUMP_LABEL (insn), 0);
+	      make_label_edge (edge_cache, bb, JUMP_LABEL (insn), insn, 0);
 	    }
 	}
 
@@ -349,7 +363,7 @@  make_edges (basic_block min, basic_block
 		 could possibly do nonlocal gotos.  */
 	      if (can_nonlocal_goto (insn))
 		for (x = nonlocal_goto_handler_labels; x; x = XEXP (x, 1))
-		  make_label_edge (edge_cache, bb, XEXP (x, 0),
+		  make_label_edge (edge_cache, bb, XEXP (x, 0), NULL_RTX,
 				   EDGE_ABNORMAL | EDGE_ABNORMAL_CALL);
 	    }
 	}