[PATCH] D24451: [LoopUnroller] Replace UnrollingPreferences::Force with ForceMaxCount + SystemZ getUnrollingPreferences().

[Jonas Paulsson via llvm-commits]

jonpa marked 2 inline comments as done.
jonpa added a comment.

Evegeny, fixed the braces, but not sure about your other comment.

I would like to mention that small loops is bad on SystemZ for the reason that they make compiler development generally difficult, as they are sensitive to code changes. In other words, if working on any arbitrary optimization, a small code change could potentially cause a regression. This is the reason I have worked on loop unrolling, and why my scheduler patch for SystemZ is yet to be commited (https://reviews.llvm.org/D17260).

So I would like to commit this patch with the reasoning that

1. Elimination of small loops is important on SystemZ, and only forced unrolling can (at least currently) handle them all (for numbers, see earlier reply to Chandler).
2. Forced unrolling produces different results but is yet needed for SystemZ. It should however only be used sparingly as a last resort. It has been shown with benchmarks that it is best to do max 2-3 iterations, to get the loop above the "tiny" threshold. This means it does in fact deserve its own max count variable.
3. The patch has been proven beneficial on benchmarks.

Will first await Evegenys reply, of course.


================
Comment at: lib/Transforms/Utils/LoopUnroll.cpp:308
@@ -306,1 +307,3 @@
       RuntimeTripCount = false;
+      Count = std::min(Count, ForceMaxCount);
+    }
----------------
evstupac wrote:
> Is this possible to move "Count" change to /Scalar/ part?
> This will also rewrite unroll count set by user (pragma unroll or -unroll-count option).
I am not sure what you mean. The patch does not change any previous behaviour for other targets than SystemZ. Please illustrate.


https://reviews.llvm.org/D24451