PR85964

This makes tracer not explode with -fno-guess-branch-probabilities.
I've settled with find_best_successor/predecessor not returning
anything if _any_ edge in the interesting direction doesn't have
->count () initialized (rather than ignoring such edges).

Honza - I suppose it is on purpose that functions like
.to_frequency () do not ICE for uninitialized counters?
It at least looks like "previous" behavior was more sane
for tracer in the counts/frequencies that were exposed.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

Honza, does this look OK to you?

tracer going wild on this testcase exposes the CFG cleanup
scalability issue I've posted the following RFC for:
https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01644.html

Thanks,
Richard.

2018-05-30  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/85964
	* tracer.c (better_p): Drop initialized count check, we only
	call the function with initialized counts now.
	(find_best_successor): Do find a best edge if one
	has uninitialized count.
	(find_best_predecessor): Likewise.  Do BB frequency check only
	if count is initialized.

> 
> This makes tracer not explode with -fno-guess-branch-probabilities.
> I've settled with find_best_successor/predecessor not returning
> anything if _any_ edge in the interesting direction doesn't have
> ->count () initialized (rather than ignoring such edges).
> 
> Honza - I suppose it is on purpose that functions like
> .to_frequency () do not ICE for uninitialized counters?
> It at least looks like "previous" behavior was more sane
> for tracer in the counts/frequencies that were exposed.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
> 
> Honza, does this look OK to you?
> 
> tracer going wild on this testcase exposes the CFG cleanup
> scalability issue I've posted the following RFC for:
> https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01644.html
> 
> Thanks,
> Richard.
> 
> 2018-05-30  Richard Biener  <rguenther@suse.de>
> 
> 	PR tree-optimization/85964
> 	* tracer.c (better_p): Drop initialized count check, we only
> 	call the function with initialized counts now.
> 	(find_best_successor): Do find a best edge if one
> 	has uninitialized count.
> 	(find_best_predecessor): Likewise.  Do BB frequency check only
> 	if count is initialized.

> 
> Index: gcc/tracer.c
> ===================================================================
> --- gcc/tracer.c	(revision 260896)
> +++ gcc/tracer.c	(working copy)
> @@ -132,8 +132,7 @@ count_insns (basic_block bb)
>  static bool
>  better_p (const_edge e1, const_edge e2)
>  {
> -  if (e1->count ().initialized_p () && e2->count ().initialized_p ()
> -      && ((e1->count () > e2->count ()) || (e1->count () < e2->count  ())))
> +  if ((e1->count () > e2->count ()) || (e1->count () < e2->count ()))
>      return e1->count () > e2->count ();
>    /* This is needed to avoid changes in the decision after
>       CFG is modified.  */
> @@ -152,12 +151,15 @@ find_best_successor (basic_block bb)
>    edge_iterator ei;
>  
>    FOR_EACH_EDGE (e, ei, bb->succs)
> -    if (!best || better_p (e, best))
> -      best = e;
> +    {
> +      if (!e->count ().initialized_p ())
> +	return NULL;
> +      if (!best || better_p (e, best))
> +	best = e;
> +    }
>    if (!best || ignore_bb_p (best->dest))
>      return NULL;
> -  if (best->probability.initialized_p ()
> -      && best->probability.to_reg_br_prob_base () <= probability_cutoff)
> +  if (best->probability.to_reg_br_prob_base () <= probability_cutoff)

Technically we could accept when one edge has large known probability and other unknown,
but in practice it won't matter because w/o profile guessing tracer is useless anyway.

So OK.
Honza

>      return NULL;
>    return best;
>  }
> @@ -172,12 +174,17 @@ find_best_predecessor (basic_block bb)
>    edge_iterator ei;
>  
>    FOR_EACH_EDGE (e, ei, bb->preds)
> -    if (!best || better_p (e, best))
> -      best = e;
> +    {
> +      if (!e->count ().initialized_p ())
> +	return NULL;
> +      if (!best || better_p (e, best))
> +	best = e;
> +    }
>    if (!best || ignore_bb_p (best->src))
>      return NULL;
> -  if (EDGE_FREQUENCY (best) * REG_BR_PROB_BASE
> -      < bb->count.to_frequency (cfun) * branch_ratio_cutoff)
> +  if (bb->count.initialized_p ()
> +      && (best->count ().to_frequency (cfun) * REG_BR_PROB_BASE
> +	  < bb->count.to_frequency (cfun) * branch_ratio_cutoff))
>      return NULL;
>    return best;
>  }

On Wed, 30 May 2018, Jan Hubicka wrote:

> > 
> > This makes tracer not explode with -fno-guess-branch-probabilities.
> > I've settled with find_best_successor/predecessor not returning
> > anything if _any_ edge in the interesting direction doesn't have
> > ->count () initialized (rather than ignoring such edges).
> > 
> > Honza - I suppose it is on purpose that functions like
> > .to_frequency () do not ICE for uninitialized counters?
> > It at least looks like "previous" behavior was more sane
> > for tracer in the counts/frequencies that were exposed.
> > 
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> > 
> > Honza, does this look OK to you?
> > 
> > tracer going wild on this testcase exposes the CFG cleanup
> > scalability issue I've posted the following RFC for:
> > https://gcc.gnu.org/ml/gcc-patches/2018-05/msg01644.html
> > 
> > Thanks,
> > Richard.
> > 
> > 2018-05-30  Richard Biener  <rguenther@suse.de>
> > 
> > 	PR tree-optimization/85964
> > 	* tracer.c (better_p): Drop initialized count check, we only
> > 	call the function with initialized counts now.
> > 	(find_best_successor): Do find a best edge if one
> > 	has uninitialized count.
> > 	(find_best_predecessor): Likewise.  Do BB frequency check only
> > 	if count is initialized.
> 
> > 
> > Index: gcc/tracer.c
> > ===================================================================
> > --- gcc/tracer.c	(revision 260896)
> > +++ gcc/tracer.c	(working copy)
> > @@ -132,8 +132,7 @@ count_insns (basic_block bb)
> >  static bool
> >  better_p (const_edge e1, const_edge e2)
> >  {
> > -  if (e1->count ().initialized_p () && e2->count ().initialized_p ()
> > -      && ((e1->count () > e2->count ()) || (e1->count () < e2->count  ())))
> > +  if ((e1->count () > e2->count ()) || (e1->count () < e2->count ()))
> >      return e1->count () > e2->count ();
> >    /* This is needed to avoid changes in the decision after
> >       CFG is modified.  */
> > @@ -152,12 +151,15 @@ find_best_successor (basic_block bb)
> >    edge_iterator ei;
> >  
> >    FOR_EACH_EDGE (e, ei, bb->succs)
> > -    if (!best || better_p (e, best))
> > -      best = e;
> > +    {
> > +      if (!e->count ().initialized_p ())
> > +	return NULL;
> > +      if (!best || better_p (e, best))
> > +	best = e;
> > +    }
> >    if (!best || ignore_bb_p (best->dest))
> >      return NULL;
> > -  if (best->probability.initialized_p ()
> > -      && best->probability.to_reg_br_prob_base () <= probability_cutoff)
> > +  if (best->probability.to_reg_br_prob_base () <= probability_cutoff)
> 
> Technically we could accept when one edge has large known probability and other unknown,
> but in practice it won't matter because w/o profile guessing tracer is useless anyway.

So the above hunk requires the fix below.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.

Richard.

2018-06-04  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/86038
	* tracer.c (find_best_successor): Check probability for
	being initialized, bail out if not.

	* gcc.dg/pr86038.c: New testcase.

Index: gcc/tracer.c
===================================================================
--- gcc/tracer.c	(revision 261136)
+++ gcc/tracer.c	(working copy)
@@ -159,7 +159,8 @@ find_best_successor (basic_block bb)
     }
   if (!best || ignore_bb_p (best->dest))
     return NULL;
-  if (best->probability.to_reg_br_prob_base () <= probability_cutoff)
+  if (!best->probability.initialized_p ()
+      || best->probability.to_reg_br_prob_base () <= probability_cutoff)
     return NULL;
   return best;
 }
Index: gcc/testsuite/gcc.dg/pr86038.c
===================================================================
--- gcc/testsuite/gcc.dg/pr86038.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/pr86038.c	(working copy)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target pthread } */
+/* { dg-options "-O2 -ftracer -ftree-parallelize-loops=2 -fno-tree-scev-cprop --param parloops-schedule=dynamic" } */
+
+int
+sd (int lw)
+{
+  while (lw < 1)
+    ++lw;
+
+  return lw;
+}