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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

> Peeling off iterations at the end might hit the case I'm trying to optimize. I will be happy to support it.

Great! Is there any code/examples you could share that would benefit?



================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:175
+        Var == IndVar) {
+      unsigned Bound = UpperBound->getLimitedValue(MaxPeelCount + 1);
+      if (Bound <= MaxPeelCount)
----------------
mkazantsev wrote:
> 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.
With using SCEV, this problem went away I think.


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:177
+      if (Bound <= MaxPeelCount)
+        DesiredPeelCount = std::max(DesiredPeelCount, Bound);
+    }
----------------
efriedma wrote:
> This doesn't check for the possibility that the induction variable could wrap... not a correctness problem, of course, but it's not clear the transform is profitable in that case.
I think this still needs wrapping checks. I'll have to look into how to best do that what SCEV.


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:169
+    CmpInst::Predicate Pred;
+    if (!Condition ||
+        !match(Condition, m_ICmp(Pred, m_Value(LeftVal), m_Value(RightVal))))
----------------
junbuml wrote:
> Isn't it okay to remove this nullcheck for  Condition ? 
Ah yes, we are not dyn_cast'ing any more.


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:192
+      Pred = ICmpInst::getSwappedPredicate(Pred);
+    while (DesiredPeelCount < MaxPeelCount &&
+           SE.isKnownPredicate(Pred, IterVal, RightSCEV)) {
----------------
junbuml wrote:
> Do we really need this loop. Isn't it possible to find the count using getMinusSCEV(RightSCEV, LeftSCEV->getStart()) as long as getMinusSCEV(RightSCEV, LeftSCEV->getStart())  is constant ?  
Yes we could, but we would need some logic dealing with different predicates I think. That should not be too much work and I am happy to do it, unless there is existing infrastructure I might have missed.


https://reviews.llvm.org/D43876