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

[Nuno Lopes via Phabricator via llvm-commits]

nlopes 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));
----------------
nikic wrote:
> 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.
Yeah, the counterexample produced by Alive2 isn't great. I'll look into that.

What it's saying is that in source we call `@bar()` and that function doesn't return.
In target, we don't do the function call and return instead. This is because `%trip` is frozen twice independently:
```
  %0 = add i64 %trip, -1
  %1 = freeze i64 %trip
  %2 = freeze i64 %0
```

When `%trip=undef`, we may have `%2 != %1 - 1`.



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

https://reviews.llvm.org/D125896