[PATCH] D50075: [UnJ] Improve explicit loop count checks

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Thanks for taking a look. The problem I was trying to solve here was the runtime thresholds preventing UnJ even if an explicit count was set.

There are other pragma tests in pragma.ll, which check combinations of unroll and unroll_and_jam pragmas. The current behaviour if there is both unroll metadata and unroll_and_jam metadata isn't currently very refined. I would expect, at least in the default pipeline, for the unroll metadata to be handled first in one of the early unroll passes.

Let me know if you think I should change how it works here.



================
Comment at: lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp:160
+  bool ExplicitUnroll = computeUnrollCount(
+      L, TTI, DT, LI, SE, EphValues, ORE, OuterTripCount, MaxTripCount,
+      OuterTripMultiple, OuterLoopSize, UP, UseUpperBound);
----------------
Meinersbur wrote:
> `OuterTripCount` is passed by-reference to `computeUnrollCount` but passed by-value to `computeUnrollAndJamCount`. It is used here and by the caller of `computeUnrollAndJamCount` (but which will use the non-updated value because of passing by-value). Is this intentional?
I believe computeUnrollCount will only update TripCount if it is fully unrolling, and only update it to either TripCount (itself) or MaxTripCount. The second shouldn't be used as we are setting it to 0 here, so OuterTripCount shouldn't be updated. 

I've removed the OuterTripCount use below in this function, I think the increased threshold should apply for partial loops too.


================
Comment at: lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp:213-216
+  // If we have an explicitly trip count, we are done. Else we may need to
+  // refine the count.
+  if (ExplicitUnroll)
+    return true;
----------------
Meinersbur wrote:
> What is supposed to happen with `llvm.loop.unroll_and_jam.enable` set, but not `llvm.loop.unroll_and_jam.count`? My expectation would be that the count is determined automatically, including considering `UP.UnrollAndJamInnerLoopThreshold` which would be skipped here.
Yep, Good point. The pragma threshold is very high, but should still be checked. I've tried to split ExplicitCount's vs other Explicits.


https://reviews.llvm.org/D50075