On Wed, 27 Oct 2021, Jan Hubicka wrote:
> >
> > gcc/ChangeLog:
> >
> > * tree-ssa-loop-split.c (split_loop): Fix incorrect probability.
> > (do_split_loop_on_cond): Likewise.
> > ---
> > gcc/tree-ssa-loop-split.c | 25 ++++++++++++++++---------
> > 1 file changed, 16 insertions(+), 9 deletions(-)
> >
> > diff --git a/gcc/tree-ssa-loop-split.c b/gcc/tree-ssa-loop-split.c
> > index 3f6ad046623..d30782888f3 100644
> > --- a/gcc/tree-ssa-loop-split.c
> > +++ b/gcc/tree-ssa-loop-split.c
> > @@ -575,7 +575,11 @@ split_loop (class loop *loop1)
> > stmts2);
> > tree cond = build2 (guard_code, boolean_type_node, guard_init, border);
> > if (!initial_true)
> > - cond = fold_build1 (TRUTH_NOT_EXPR, boolean_type_node, cond);
> > + cond = fold_build1 (TRUTH_NOT_EXPR, boolean_type_node, cond);
> > +
> > + edge true_edge = EDGE_SUCC (bbs[i], 0)->flags & EDGE_TRUE_VALUE
> > + ? EDGE_SUCC (bbs[i], 0)
> > + : EDGE_SUCC (bbs[i], 1);
> >
> > /* Now version the loop, placing loop2 after loop1 connecting
> > them, and fix up SSA form for that. */
> > @@ -583,10 +587,10 @@ split_loop (class loop *loop1)
> > basic_block cond_bb;
> >
> > class loop *loop2 = loop_version (loop1, cond, &cond_bb,
> > - profile_probability::always (),
> > - profile_probability::always (),
> > - profile_probability::always (),
> > - profile_probability::always (),
> > + true_edge->probability,
> > + true_edge->probability.invert (),
> > + true_edge->probability,
> > + true_edge->probability.invert (),
> > true);
>
> As discussed yesterday, for loop of form
>
> for (...)
> if (cond)
> cond = something();
> else
> something2
>
> Split as
Note that you are missing to conditionalize loop1 execution
on 'cond' (not sure if that makes a difference).
> loop1:
if (cond)
> for (...)
> if (true)
> cond = something();
> if (!cond)
> break
> else
> something2 ();
>
> loop2:
> for (...)
> if (false)
> cond = something();
> else
> something2 ();
>
> If "if (cond)" has probability p, you want to scale loop1 by p
> and loop2 by 1-p as your patch does, but you need to exclude the basic
> blocks guarded by the condition.
>
> One way is to break out loop_version and implement it inline, other
> option (perhaps leading to less code duplication) is to add argument listing
> basic blocks that should not be scaled, which would be set to both arms
> of the if.
>
> Are there other profile patches of your I should look at?
> Honza
> > gcc_assert (loop2);
> >
> > @@ -1486,10 +1490,10 @@ do_split_loop_on_cond (struct loop *loop1, edge
> > invar_branch)
> > initialize_original_copy_tables ();
> >
> > struct loop *loop2 = loop_version (loop1, boolean_true_node, NULL,
> > - profile_probability::always (),
> > - profile_probability::never (),
> > - profile_probability::always (),
> > - profile_probability::always (),
> > + invar_branch->probability.invert (),
> > + invar_branch->probability,
> > + invar_branch->probability.invert (),
> > + invar_branch->probability,
> > true);
> > if (!loop2)
> > {
> > @@ -1530,6 +1534,9 @@ do_split_loop_on_cond (struct loop *loop1, edge
> > invar_branch)
> > to_loop1->flags |= true_invar ? EDGE_FALSE_VALUE : EDGE_TRUE_VALUE;
> > to_loop2->flags |= true_invar ? EDGE_TRUE_VALUE : EDGE_FALSE_VALUE;
> >
> > + to_loop1->probability = invar_branch->probability.invert ();
> > + to_loop2->probability = invar_branch->probability;
> > +
> > /* Due to introduction of a control flow edge from loop1 latch to loop2
> > pre-header, we should update PHIs in loop2 to reflect this connection
> > between loop1 and loop2. */
> > --
> > 2.27.0.90.geebb51ba8c
> >
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
