[PATCH] D43876: [LoopUnroll] Peel off iterations if it makes conditions true/false.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:173
     }
+
+    // Pay respect to limitations implied by loop size and the max peel count.
+    unsigned MaxPeelCount = UnrollPeelMaxCount;
+    MaxPeelCount = std::min(MaxPeelCount, UP.Threshold / LoopSize - 1);
+
+    DesiredPeelCount = std::max(DesiredPeelCount,
+                                countToEliminateCompares(*L, MaxPeelCount, SE));
+
     if (DesiredPeelCount > 0) {
-      // Pay respect to limitations implied by loop size and the max peel count.
----------------
mkazantsev wrote:
> What was the point of moving this `if`? Could we not just update DesiredPeelCount before this line?
> We only need MaxPeelCount under this condition, there is no point in calculating it before it.
MaxPeelCount is passed to `countToEliminateCompares`, to limit the maximum iterations we do.


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:194
+           SE.isKnownPredicate(Pred, IterVal, RightSCEV)) {
+      IterVal = SE.getAddExpr(
+          IterVal, cast<SCEVAddRecExpr>(LeftSCEV)->getStepRecurrence(SE));
----------------
mkazantsev wrote:
> Step calculation can be hoisted out of this loop. I would also suggest bailing early if AR is not affine because adding a step of non-affine AddRec many times can produce really big and ugly SCEVs.
I've added a comment that makes it clearer I hope. The idea is to handle cases like below, where the condition is known to be false initially. Initially `i > 2` is not known, but the inverse `i  <= 2` is known.

```
if (i > 2) {
  // do something
} else {
  // do something else
}
```


https://reviews.llvm.org/D43876