[PATCH] D24790: [LoopUnroll] Use the upper bound of the loop trip count to completely unroll loops

[Michael Zolotukhin via llvm-commits]

mzolotukhin added a comment.

Hi Haicheng,

Please find a couple of minor comments inline.

Thanks,
Michael



================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:779
                             UnrolledSize, UnrolledSize)) {
+      UP.UpperBound = (MaxTripCount == FullUnrollTripCount);
+      TripCount = FullUnrollTripCount;
----------------
haicheng wrote:
> haicheng wrote:
> > mzolotukhin wrote:
> > > What if `TripCount` matches `MaxTripCount`? Will we think that we're using upper bound instead of the exact trip count in this case?
> > `TripCount `and `MaxTripCount `cannot both be non zero because `MaxTripCount `is computed only if `TripCount `is Zero.  If one is non zero, the other one must be zero.  If they are both zero, `FullUnrollTripCount `is zero and then we cannot enter here.
> I also added a sentence in the comment to describe this.
Could you add an assert for this please?


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:95-102
+static cl::opt<bool> UnrollUpperBound(
+    "unroll-upperbound", cl::ZeroOrMore, cl::Hidden,
+    cl::desc("Unroll loops with the upper bounds of the trip counts"));
+
+static cl::opt<unsigned> UnrollMaxUpperBound(
+    "unroll-max-upperbound", cl::init(8), cl::Hidden,
+    cl::desc(
----------------
Do we really need two options here? Can the first one be replaced with `(UnrollMaxUpperBound == 0)`?


Repository:
  rL LLVM

https://reviews.llvm.org/D24790