[PATCH] D19553: Unroll pass restructure.

[Gerolf Hoflehner via llvm-commits]

Gerolf added a subscriber: Gerolf.

================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:710
@@ +709,3 @@
+    if (UP.AllowRemainder &&
+        (LoopSize - BEInsns) * UP.Count + BEInsns < UP.Threshold)
+      return true;
----------------
Please move  (LoopSize - BEInsns) * UP.Count + BEInsns  to a function and comment properly. It will be invoked many times as the estimated size of the unrolled loop gets checked repeatedly.

================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:807
@@ +806,3 @@
+        UnrolledSize = (LoopSize - BEInsns) * UP.Count + BEInsns;
+        while (UP.Count != 0 && UnrolledSize > UP.PartialThreshold) {
+          UP.Count >>= 1;
----------------
Isn't that simply UP.Count = max(UP.PartialThreshold - BEInsns/ LoopSize -BEInsns -1, 0)? Probably it also needs a check that LoopSize > BEInsns, but that should probably be an assertion anyway. And for UP.PartialThreshold - BEInsns/LoopSize - BEInsns should  probably be fit into a function since there is a similar instance above. It makes sense to do all this now since the intent of this patch is a code cleanup. 

================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:859
@@ +858,3 @@
+  // the original count which satisfies the threshold limit.
+  while (UP.Count != 0 && UnrolledSize > UP.PartialThreshold) {
+    UP.Count >>= 1;
----------------
see above


http://reviews.llvm.org/D19553