Re: [PATCH 1/3] loop-invariant: Don't move cold bb instructions to preheader in RTL

[Jan Hubicka <hubicka@kam.mff.cuni.cz>]

>  
> OK. Comments like?
> 
> /* Don't move insn of cold BB out of loop to preheader to reduce calculations
>    and register live range in hot loop with cold BB.  */

Looks good.
> 
> 
> And maybe some dump log will help tracking in xxx.c.271r.loop2_invariant.
> 
> --- a/gcc/loop-invariant.c
> +++ b/gcc/loop-invariant.c
> @@ -1190,7 +1190,12 @@ find_invariants_bb (class loop *loop, basic_block bb, bool always_reached,
>    basic_block preheader = loop_preheader_edge (loop)->src;
> 
>    if (preheader->count > bb->count)
> -    return;
> +    {
> +      if (dump_file)
> +       fprintf (dump_file, "Don't move invariant from bb: %d in loop %d\n",
> +                bb->index, loop->num);
> +      return;
> +    }

This is also a good idea - testcases are quite important for this typo
of stuff.
> > 
> > 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.
> 
> Sorry I don't understand why always_executed and always_reached matters here?
> the test is based on BB before the FOR_BB_INSNS loop...

always_executed is useful here to avoid the situation where bb->count or
preheader->count is wrong and it would disable wanted transformation.

always_executed is just a bug I noticed while looking at the code.  I
will produce testcase for bugzilla.

Honza