Re: Fix profile updating in cfgbuild

[Bernhard Reutner-Fischer <rep dot dot dot nop at gmail dot com>]

On 7 June 2017 08:44:13 CEST, Jan Hubicka <hubicka@ucw.cz> wrote:
>Hi
>the following patch makes cfgbuild to preserve profile when loops are
>introduced at RTL level (not very well, but at least do not throw it
>all
>away) and also avoids re-computing probabilities when there are no
>changes to CFG.
>
>Bootstrapped/regtested x86_64-linux. Comitted.
>
>Honza
>
>Index: cfgbuild.c
>===================================================================
>--- cfgbuild.c	(revision 248915)
>+++ cfgbuild.c	(working copy)
>@@ -475,6 +475,10 @@ find_bb_boundaries (basic_block bb)
> 
> 	  bb = fallthru->dest;
> 	  remove_edge (fallthru);
>+	  /* BB is unreachable at this point - we need to determine its
>profile
>+	     once edges are built.  */
>+	  bb->frequency = 0;
>+	  bb->count = profile_count::uninitialized ();
> 	  flow_transfer_insn = NULL;
> 	  if (code == CODE_LABEL && LABEL_ALT_ENTRY_P (insn))
> 	    make_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun), bb, 0);
>@@ -577,7 +581,7 @@ compute_outgoing_frequencies (basic_bloc
>         guess_outgoing_edge_probabilities (b);
>     }
> 
>-  if (b->count > profile_count::zero ())
>+  if (b->count.initialized_p ())
>     FOR_EACH_EDGE (e, ei, b->succs)
>       e->count = b->count.apply_probability (e->probability);
> }
>@@ -590,6 +594,9 @@ void
> find_many_sub_basic_blocks (sbitmap blocks)
> {
>   basic_block bb, min, max;
>+  bool found = false;
>+  auto_vec<unsigned int> n_succs;
>+  n_succs.safe_grow_cleared (last_basic_block_for_fn (cfun));
> 
>   FOR_EACH_BB_FN (bb, cfun)
>     SET_STATE (bb,
>@@ -597,11 +604,24 @@ find_many_sub_basic_blocks (sbitmap bloc
> 
>   FOR_EACH_BB_FN (bb, cfun)
>     if (STATE (bb) == BLOCK_TO_SPLIT)
>-      find_bb_boundaries (bb);
>+      {
>+	int n = last_basic_block_for_fn (cfun);
>+	unsigned int ns = EDGE_COUNT (bb->succs);
>+
>+        find_bb_boundaries (bb);
>+	if (n == last_basic_block_for_fn (cfun) && ns == EDGE_COUNT
>(bb->succs))
>+	  n_succs[bb->index] = EDGE_COUNT (bb->succs);
>+      }
> 
>   FOR_EACH_BB_FN (bb, cfun)
>     if (STATE (bb) != BLOCK_ORIGINAL)
>-      break;
>+      {
>+	found = true;
>+        break;
>+      }
>+
>+  if (!found)
>+    return;
> 
>   min = max = bb;
>   for (; bb != EXIT_BLOCK_PTR_FOR_FN (cfun); bb = bb->next_bb)
>@@ -624,14 +644,37 @@ find_many_sub_basic_blocks (sbitmap bloc
> 	  continue;
> 	if (STATE (bb) == BLOCK_NEW)
> 	  {
>+	    bool initialized_src = false, uninitialized_src = false;
> 	    bb->count = profile_count::zero ();
> 	    bb->frequency = 0;
> 	    FOR_EACH_EDGE (e, ei, bb->preds)
> 	      {
>-		bb->count += e->count;
>+		if (e->count.initialized_p ())
>+		  {
>+		    bb->count += e->count;
>+		    initialized_src = true;
>+		  }
>+		else
>+		  uninitialized_src = false;

false?
Please explain false respectively the initializer (false too) in the light of the condition in the hunk below?

thanks,

> 		bb->frequency += EDGE_FREQUENCY (e);
> 	      }
>+	    /* When some edges are missing with read profile, this is
>+	       most likely because RTL expansion introduced loop.
>+	       When profile is guessed we may have BB that is reachable
>+	       from unlikely path as well as from normal path.
>+
>+	       TODO: We should handle loops created during BB expansion
>+	       correctly here.  For now we assume all those loop to cycle
>+	       precisely once.  */
>+	    if (!initialized_src
>+		|| (uninitialized_src
>+		     && profile_status_for_fn (cfun) != PROFILE_READ))
>+	      bb->count = profile_count::uninitialized ();
> 	  }
>+	else
>+	  /* If nothing changed, there is no need to create new BBs.  */
>+	  if (EDGE_COUNT (bb->succs) == n_succs[bb->index])
>+	    continue;
> 
> 	compute_outgoing_frequencies (bb);
>       }