[PATCH] D88210: [IndVars] Use knowledge about execution on last iteration when removing checks

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2433
       auto *BI = cast<BranchInst>(ExitingBB->getTerminator());
-      auto OptimizeCond = [&](bool Inverted) {
-        if (isTrivialCond(L, BI, SE, Inverted, MaxExitCount)) {
+      auto OptimizeCond = [&](bool Inverted, bool SkipLastIter) {
+        const SCEV *MaxIter = MaxExitCount;
----------------
ebrevnov wrote:
> NIT1: please list all captured values explicitly. Maybe capture SkipLastIter by value?
> NIT2: please think of moving handling of SkipLastIter case to this lambda. In this case we will "hide" complexity from the main loop.
As for nit 1: will do;
As for nit 2: this logic is going to grow more complex in further patch (see dependency), so, to avoid copy-paste, I'd rather keep it as is.


================
Comment at: llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2437
+          const SCEV *One = SE->getOne(MaxIter->getType());
+          MaxIter = SE->getMinusSCEV(MaxIter, One);
+        }
----------------
ebrevnov wrote:
> This lambda may be called twice with SkipLastIter equal to true for each exit. As a result MaxIter may be decremented by 2 for particular exit.
This cannot happen because `MaxIter` is local.


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

https://reviews.llvm.org/D88210