[PATCH] D62989: [Unroll] Do NOT unroll a loop with small runtime upperbound

[Z. Zheng via Phabricator via llvm-commits]

zzheng marked 2 inline comments as done.
zzheng added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:795
+  if (!(UP.UpperBound || MaxOrZero) || UnrollByMaxCount > UnrollMaxUpperBound)
+    UnrollByMaxCount = 0;
+
----------------
hfinkel wrote:
> Did you make this new variable so that you could apply this `!(UP.UpperBound || MaxOrZero) || UnrollByMaxCount > UnrollMaxUpperBound` condition only to full-unrolling case and not to the partial unrolling case?
No, new variable UnrollByMaxCount replaces use of MaxTripCount in full-unrolling case.

We only full-unroll a runtime loop by its upper bound if MaxOrZero is true; otherwise runtime unrolling will handle it, sometimes unroll more than MaxTripCount. Runtime unrolling needs to know MaxTripCount, so we cannot set it to zero here.

https://reviews.llvm.org/D63064 is trying to solve the same problem for ARM target.

I'm confuse why this relevant to partial unrolling, which only applies to known TripCount. We only calculate MaxTripCount and MaxOrZero if TripCount is 0.


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:1097
   // count.
+  unsigned MaxTripCount = 0;
   bool MaxOrZero = false;
----------------
hfinkel wrote:
> Why did this move down here?
To move it close to use, like MaxOrZero.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62989