[PATCH] D15559: [SCEVExpander] Make findExistingExpansion smarter

[Junmo Park via llvm-commits]

flyingforyou added a subscriber: sunfish.
flyingforyou marked an inline comment as done.

================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:1901-1902
@@ +1900,4 @@
+            SE.DT.dominates(EntInst, At) &&
+            (SE.LI.getLoopFor(EntInst->getParent()) == nullptr ||
+             SE.LI.getLoopFor(EntInst->getParent())->contains(At))) {
+          return Ent;
----------------
mzolotukhin wrote:
> Why do we need these conditions? Isn't dominance sufficient?
> 
> Also, would it make sense to factor out this code block to a separate function (and use it both here and in `expand`)?
Attached Wei's comment in r259815

>     Fix a regression for r259736.
>     
>     When SCEV expansion tries to reuse an existing value, it is needed to ensure
>     that using the Value at the InsertPt will not break LCSSA. The fix adds a
>     check that InsertPt is either inside the candidate Value's parent loop, or
>     the candidate Value's parent loop is nullptr.



> would it make sense to factor out this code block to a separate function (and use it both here and in expand)?
> 

Sure.



http://reviews.llvm.org/D15559