On 2021/9/13 16:17, Richard Biener wrote:
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 <rguent...@suse.de>
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 <rguent...@suse.de>
* 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?
Oh, I see. I only noticed the functionality change of the patch on the case
and no failure check of it, misunderstood it was a regression instead of an
improvement to also hoisting invariants from infinite loop, sorry about that.
Finally, the function fill_always_executed_in_1 could mark all ALWAYS_EXECUTED
bb both including and after all subloops' bb but break after exiting from
infinite subloops with better performance, thanks. The only thing to be
worried is replacing get_loop_body_in_dom_order makes the code a bit more
complicated for later readers as the loop depth and DOM order is not a problem
here any more? ;)
Richard.
--
Thanks,
Xionghu
