[PATCH] D125896: [LoopUnroll] Freeze tripcount rather than condition

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopUnrollRuntime.cpp:752
+    TripCount = B.CreateFreeze(TripCount);
+    BECount =
+        B.CreateAdd(TripCount, ConstantInt::get(TripCount->getType(), -1));
----------------
reames wrote:
> Really not a fan of this.  Your duplicating already subtle logic, and loosing the possibility of SCEV simplification on the BE count.
> 
> Doing it once makes sense, but maybe expand both and then freeze the results independently?
I gave this a try, and seeing one alive failure on Transforms/LoopUnroll/runtime-loop-multiple-exits.ll with this variant: https://gist.github.com/nikic/82c3a9ee77141e813f7cd5362890c017

I'm having a hard try trying to parse the output, but I think what it is telling us is that because the values are independently frozen, it's possible to pick the values such that //both// the unrolled loop and the epilogue are skipped and `@bar()` never gets called.

So I think it's important that we freeze one value and base the other one on it, rather than freezing both independently.


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

https://reviews.llvm.org/D125896