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

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

I find the summary confusing. Can we say that simple unrolling will be prioritized over `llvm.loop.unroll_and_jam.count` metadata and `-unroll-and-jam-count=` cmdline options?

The test case does not seem to test this. The comment mentions it expects unroll-and-jam to happen, instead of prioritizing `llvm.loop.unroll.enable`.



================
Comment at: lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp:160
+  bool ExplicitUnroll = computeUnrollCount(
+      L, TTI, DT, LI, SE, EphValues, ORE, OuterTripCount, MaxTripCount,
+      OuterTripMultiple, OuterLoopSize, UP, UseUpperBound);
----------------
`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?


================
Comment at: lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp:198
   bool PragmaEnableUnroll = HasUnrollAndJamEnablePragma(L);
   ExplicitUnroll = PragmaCount > 0 || PragmaEnableUnroll || UserUnrollCount;
 
----------------
`ExplicitUnroll` seem to have two meanings: Once to determine whether simple unroll is enabled, second to determine whether Unroll-And-Jam is enabled. Could we rename this to `ExplicitUnrollAndJam`? 


================
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;
----------------
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.


https://reviews.llvm.org/D50075