[PATCH] D18670: LoopUnroll: some small fixes/tweaks to make it more useful for partial unrolling

[Z. Zheng via llvm-commits]

zzheng added a comment.

In http://reviews.llvm.org/D18670#389043, @escha wrote:

> > That's what UP.Count is intended to do: let the target suggest an unroll count it deems reasonable.
>
>
> I thought UP.Count overrides Threshold, causing it to unroll loops even if their LoopSize is greater than the Threshold? If that's not true, then I guess that part of this patch is unnecessary, but it didn't seem to work that way when I tried.
>
> > It seems partial unrolling is preferred over full unrolling for your target. I would set it up in getUnrollingPerferences()... unless it's too early for the TTI to estimate how much unrolling is suitable during LoopUnrollPass.
>
>
> No, full unrolling is strongly preferred. However, in some cases, full unrolling is too costly, either due to register usage (which our TTI calculates) or due to Threshold (which LoopUnrollPass calculates), and we'd rather partial unroll than do nothing at all.


Count is still bound by UP.Threshold in canUnrollCompletely(), which decides full or partial unrolling. Setting UP.Threshold and UP.PartialThreshold but not UP.Count in TTI can limit full unroll as well.

In http://reviews.llvm.org/D18670#389046, @escha wrote:

> Going by the code, Count is emphatically not what I want:
>
>   if (!AllowPartial && !CountSetExplicitly) {
>     DEBUG(dbgs() << "  will not try to unroll partially because "
>                  << "-unroll-allow-partial not given\n");
>     return false;
>   }
>   if (!AllowRuntime && !CountSetExplicitly) {
>     DEBUG(dbgs() << "  will not try to unroll loop with runtime trip count "
>                  << "-unroll-runtime not given\n");
>     return false;
>   }
>   
>
> It forces an unroll of Count, even if you haven't enabled Runtime unrolling and don't want it.


You are right with runtime unrolling part, :-)

Can you also add a test case?


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:570
@@ -568,2 +569,3 @@
     Count = TripCount;
+  Count = std::min(Count, UP.FullUnrollMaxCount);
 
----------------
Now I understand what you're trying to do...


Repository:
  rL LLVM

http://reviews.llvm.org/D18670