[PATCH] D44419: [SCEV][NFC] Remove TBB, FBB parameters from exit limit computations

[Florian Hahn via Phabricator via llvm-commits]

fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM, it seems like TBB and FBB were only used to determine whether the br instruction using ExitCond leaves the loop and determining this up front should be fine. But I am not extremely familiar with this code, so it might be good to wait a bit with committing to leave the other reviewers some time to raise additional concerns.



================
Comment at: include/llvm/Analysis/ScalarEvolution.h:1436
   /// execute if its exit condition were a conditional branch of ExitCond,
-  /// TBB, and FBB.
+  /// and ExitByCond.
   ///
----------------
Maybe it is worth stating that ExitByCond will be true, if the conditional branch exits the loop?


================
Comment at: lib/Analysis/ScalarEvolution.cpp:6946
+    bool ExitByCond = !L->contains(BI->getSuccessor(0));
+    assert(ExitByCond == L->contains(BI->getSuccessor(1)) &&
+           "It should have one successor in loop and one exit block!");
----------------
 I think in some cases, it could be triggered with this change whereas no assertion was triggered before, e.g. ExitByCond being something not understood by computeExitLimitFromCondImpl. But I think it makes sense to only use it for branches with one successor in the loop and one outside.


https://reviews.llvm.org/D44419