[PATCH] D47624: [PM/LoopUnswitch] Fix PR37651 by correctly invalidating SCEV when unswitching loops.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:359
+    if (OuterL)
+      SE->forgetLoop(OuterL);
+    else
----------------
mkazantsev wrote:
> chandlerc wrote:
> > mkazantsev wrote:
> > > Why it is enough? Imagine that you have
> > >   for (...) { // Loop 1
> > >     for (...) { // Loop 2
> > >       for (...) { // Loop 3
> > >         ...
> > >        }
> > >     }
> > >   }
> > > 
> > > Let `L` be Loop 3 and all exits from `L` lead to Loop 2. Then, if I understand the code correctly, OuterL will be found as Loop 2 and we invalidate it (and its internals). But what if what we are going to do with Loop 3 also affects trip count in Loop 1?
> > Do you know how to build such a test case? I wasn't able to craft anything, but I'm not a SCEV expert.
> Not sure how to construct a test that will fail an assert, however the general idea is that, for example, a block that goes to method exit from the innermost look is also an exiting block for all its containing loops, up to the topmost parent. It means that this exiting block may be considered for iter count calculation for all those loops. From this point of view, there is no practical difference between the immediate parent and any other top loop.
> 
> 
Yes, but this code finds the outermost exit block and uses that as the root to invalidate. So the case you're describing should be correctly handled.

The question is can a loop outside of this outermost exit block's loop have its iteration count change. I keep trying to build a test case for this and failing.

Notably, the best idea I had doesn't actually work: if you have a loop-invariant full exit from the loop nest which will make the number of loop exits go from 2 to 1 after unswitching, it might seem like this will impact every loop in the nest. But in practice, we will unswitch this one loop at a time. So if we have loops A, B, and C (C is the inner most), we will move the multi-exit from C to B in the first one. Both A and B will still be multi-exit after this, so invalidating A can't help anything. 


Repository:
  rL LLVM

https://reviews.llvm.org/D47624