* [PATCH] PR85964
@ 2018-05-30 10:42 Richard Biener
2018-05-30 14:53 ` Jan Hubicka
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2018-05-30 10:42 UTC (permalink / raw)
To: gcc-patches; +Cc: Jan Hubicka
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)
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;
}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] PR85964
2018-05-30 10:42 [PATCH] PR85964 Richard Biener
@ 2018-05-30 14:53 ` Jan Hubicka
2018-06-04 9:26 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Jan Hubicka @ 2018-05-30 14:53 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
>
> 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;
> }
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] PR85964
2018-05-30 14:53 ` Jan Hubicka
@ 2018-06-04 9:26 ` Richard Biener
0 siblings, 0 replies; 3+ messages in thread
From: Richard Biener @ 2018-06-04 9:26 UTC (permalink / raw)
To: Jan Hubicka; +Cc: gcc-patches
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;
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-06-04 9:26 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-30 10:42 [PATCH] PR85964 Richard Biener
2018-05-30 14:53 ` Jan Hubicka
2018-06-04 9:26 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).