> > gcc/ChangeLog:
> > 
> >     * loop-invariant.c (find_invariants_bb): Check profile count
> >     before motion.
> >     (find_invariants_body): Add argument.
> > ---
> >  gcc/loop-invariant.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/gcc/loop-invariant.c b/gcc/loop-invariant.c
> > index 5eee2e5c9f8..c61c8612fae 100644
> > --- a/gcc/loop-invariant.c
> > +++ b/gcc/loop-invariant.c
> > @@ -1183,9 +1183,14 @@ find_invariants_insn (rtx_insn *insn, bool 
> > always_reached, bool always_executed)
> >     call.  */
> >  
> >  static void
> > -find_invariants_bb (basic_block bb, bool always_reached, bool 
> > always_executed)
> > +find_invariants_bb (class loop *loop, basic_block bb, bool always_reached,
> > +               bool always_executed)
> >  {
> >    rtx_insn *insn;
> > +  basic_block preheader = loop_preheader_edge (loop)->src;
> > +
> > +  if (preheader->count > bb->count)
> > +    return;
> 
> Please add a comment explaining the conditional and if possible also a
> testcase.  Since profile updating and use is sensitive topic and it may
> trigger regressions later, it is important to keep track of info why
> given tests was added.
> 
> I wonder why the cost model chose to move any invariatns to preheader
> why preheader->count > bb->count?

Thinking about this more, you want to test:
  if (!always_executed && preheader->count > bb->count)
    return;
This will rule out some profile misupates.

Also the code updating always_reached is worng:
      if (always_reached
          && CALL_P (insn)
          && (RTL_LOOPING_CONST_OR_PURE_CALL_P (insn)
              || ! RTL_CONST_OR_PURE_CALL_P (insn)))
  always_reached = false;
It misses the case where statement can trhow external exception or
volatile asms that can also terminate execution.

I bit worry on how often the test will have false positives with guessed
profile where earlier loop optimizations reduced trip count below
realistic estimate.

Honza
> 
> Honza
> >  
> >    FOR_BB_INSNS (bb, insn)
> >      {
> > @@ -1214,8 +1219,7 @@ find_invariants_body (class loop *loop, basic_block 
> > *body,
> >    unsigned i;
> >  
> >    for (i = 0; i < loop->num_nodes; i++)
> > -    find_invariants_bb (body[i],
> > -                   bitmap_bit_p (always_reached, i),
> > +    find_invariants_bb (loop, body[i], bitmap_bit_p (always_reached, i),
> >                     bitmap_bit_p (always_executed, i));
> >  }
> >  
> > -- 
> > 2.25.1
> > 

Reply via email to