>
> 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
