Fix profile updating in cfgbuild

Fix profile updating in cfgbuild

[Jan Hubicka]

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;
 		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);
       }

Re: Fix profile updating in cfgbuild

[Bernhard Reutner-Fischer]

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);
>       }

Re: Fix profile updating in cfgbuild

[Jan Hubicka]

> > 	      {
> >-		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?

This is a thinko. It was meant to be true.  I am testing the fix.

Thanks!
Honza
> 
> 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);
> >       }

Re: Fix profile updating in cfgbuild

[Jan Hubicka]

Hi,
this is what I comitted

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 249007)
+++ ChangeLog	(working copy)
@@ -1,3 +1,7 @@
+2017-06-08  Jan Hubicka  <hubicka@ucw.cz>
+
+	* cfgbuild.c (find_many_sub_basic_blocks): Fix thinko.
+
 2017-06-08  Martin Liska  <mliska@suse.cz>
 
 	PR gcov-profile/80911
Index: cfgbuild.c
===================================================================
--- cfgbuild.c	(revision 249007)
+++ cfgbuild.c	(working copy)
@@ -655,7 +655,7 @@ find_many_sub_basic_blocks (sbitmap bloc
 		    initialized_src = true;
 		  }
 		else
-		  uninitialized_src = false;
+		  uninitialized_src = true;
 		bb->frequency += EDGE_FREQUENCY (e);
 	      }
 	    /* When some edges are missing with read profile, this is