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

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added inline comments.


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:154
+  unsigned DesiredPeelCount = 0;
+  PHINode *IndVar = L->getCanonicalInductionVariable();
+  if (!IndVar)
----------------
efriedma wrote:
> Please don't use getCanonicalInductionVariable; indvars doesn't generate canonical induction variables anymore, so this will only handle limited cases.
> 
> Can you use SCEV here instead?
I would also advice using SCEV here. It can help to cover more cases than just comparison against constants for no extra price. See `ScalarEvolution::isKnownPredicate` or `ScalarEvolution::isLoopEntryGuardedByCond`.


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:175
+        Var == IndVar) {
+      unsigned Bound = UpperBound->getLimitedValue(MaxPeelCount + 1);
+      if (Bound <= MaxPeelCount)
----------------
Is it OK if you have negative UpperBound? If you interpret it as unsigned value, it is a very big value, and you are going to peel max possible amount of iterations in this case.


================
Comment at: test/Transforms/LoopUnroll/peel-loop-conditions.ll:10
+; CHECK-NEXT:  for.body.lr.ph:
+; CHECK-NEXT:    [[CMP1_PEEL:%.*]] = icmp ult i32 0, 2
+; CHECK-NEXT:    br i1 [[CMP1_PEEL]], label [[IF_THEN_PEEL:%.*]], label [[IF_ELSE_PEEL:%.*]]
----------------
I don't think it's profitable unless we have proved that `%k` is positive.


https://reviews.llvm.org/D43876