On Mon, 13 Sep 2021, Xionghu Luo wrote:
>
>
> On 2021/9/10 21:54, Xionghu Luo via Gcc-patches wrote:
> >
> >
> > On 2021/9/9 18:55, Richard Biener wrote:
> >> diff --git a/gcc/tree-ssa-loop-im.c b/gcc/tree-ssa-loop-im.c
> >> index 5d6845478e7..4b187c2cdaf 100644
> >> --- a/gcc/tree-ssa-loop-im.c
> >> +++ b/gcc/tree-ssa-loop-im.c
> >> @@ -3074,15 +3074,13 @@ fill_always_executed_in_1 (class loop *loop,
> >> sbitmap contains_call)
> >> break;
> >> if (bb->loop_father->header == bb)
> >> - {
> >> - if (!dominated_by_p (CDI_DOMINATORS, loop->latch, bb))
> >> - break;
> >> -
> >> - /* In a loop that is always entered we may proceed anyway.
> >> - But record that we entered it and stop once we leave it
> >> - since it might not be finite. */
> >> - inn_loop = bb->loop_father;
> >> - }
> >> + /* Record that we enter into a subloop since it might not
> >> + be finite. */
> >> + /* ??? Entering into a not always executed subloop makes
> >> + fill_always_executed_in quadratic in loop depth since
> >> + we walk those loops N times. This is not a problem
> >> + in practice though, see PR102253 for a worst-case testcase. */
> >> + inn_loop = bb->loop_father;
> >
> >
> > Yes your two patches extracted the get_loop_body_in_dom_order out and
> > removed
> > the inn_loop break logic when it doesn't dominate outer loop. Confirmed the
> > replacement
> > could improve for saving ~10% build time due to not full DOM walker and
> > marked the previously
> > ignored ALWAYS_EXECUTED bbs.
> > But if we don't break for inner loop again, why still keep the *inn_loop*
> > variable?
> > It seems unnecessary and confusing, could we just remove it and restore the
> > original
> > infinte loop check in bb->succs for better understanding?
>
>
> What's more, the refine of this fix is incorrect for PR78185.
>
>
> commit 483e400870601f650c80f867ec781cd5f83507d6
> Author: Richard Biener <[email protected]>
> Date: Thu Sep 2 10:47:35 2021 +0200
>
> Refine fix for PR78185, improve LIM for code after inner loops
>
> This refines the fix for PR78185 after understanding that the code
> regarding to the comment 'In a loop that is always entered we may
> proceed anyway. But record that we entered it and stop once we leave
> it.' was supposed to protect us from leaving possibly infinite inner
> loops. The simpler fix of moving the misplaced stopping code
> can then be refined to continue processing when the exited inner
> loop is finite, improving invariant motion for cases like in the
> added testcase.
>
> 2021-09-02 Richard Biener <[email protected]>
>
> * tree-ssa-loop-im.c (fill_always_executed_in_1): Refine
> fix for PR78185 and continue processing when leaving
> finite inner loops.
>
> * gcc.dg/tree-ssa/ssa-lim-16.c: New testcase.
>
>
> 3<-------
> | |
> 6<--- |
> | \ | |
> | \ | |
> 4 8 |
> |--- |
> | | |
> 5 7------
> |
> 1
>
> loop 2 is an infinite loop, it is only ALWAYS_EXECUTED for loop 2,
> but r12-3313-g483e40087 sets it ALWAYS_EXECUTED for loop 1.
> We need to restore it like this:
> https://gcc.gnu.org/pipermail/gcc-patches/2021-September/579195.html
I don't understand - BB6 is the header block of loop 2 which is
always entered and thus BB6 is always executed at least once.
The important part is that BB4 which follows the inner loop is
_not_ always executed because we don't know if we will exit the
inner loop.
What am I missing?
Richard.
