[PATCH] D100004: [SCEV] Correct handling of recurrences matched in partially unreachable code (try 2)

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:5675
   auto *L = LI.getLoopFor(P->getParent());
-  assert(L && L->getHeader() == P->getParent());
-  if (!L->contains(BO->getParent()))
+  if (!L || L->getHeader() != P->getParent())
+    // If we have unreachable blocks involved, then dominance collapses and
----------------
mkazantsev wrote:
> reames wrote:
> > mkazantsev wrote:
> > > Do we have a guarantee of having no loops in unreachable code?
> > See LoopInfoBase<>::analyze specififically the line:
> > DomTree.isReachableFromEntry(Backedge))
> > 
> > Note that the verify routine also checks against a newly constructed LI, so any failure to update should be caught in a expensive asserts run.
> But expensive verification runs between the passes. It is still possible that some pass makes a loop unreachable in the middle of the transform and does not immediately destroy the loop while continuing to use SCEV.
> 
> I am still strongly leaning towards not relying on such things, even if they might be correct. It's far from obvious and far from simple.
Passes that rely on running with partially corrupt analyzes are buggy.  We should find and fix them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100004/new/

https://reviews.llvm.org/D100004