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

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev 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
----------------
reames wrote:
> 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.
It is not a corrupt analysis. It is clearly stated that LI for unreachable blocks is undefined. It may have loops anywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100004