[PATCH] D18202: Enable non-power-of-2 pragma unroll counts

[Michael Zolotukhin via llvm-commits]

mzolotukhin added inline comments.

================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:736-739
@@ -740,2 +735,6 @@
 
+  // If loop has an unroll count pragma mark loop as unrolled to prevent
+  // unrolling beyond that requested by the pragma.
+  if (HasPragma && PragmaCount != 0)
+    SetLoopAlreadyUnrolled(L);
   return true;
----------------
Why do we need to move this?

================
Comment at: lib/Transforms/Utils/LoopUnrollRuntime.cpp:354-355
@@ +353,4 @@
+    //
+    // If (2) is true, we know that TripCount really is (1 << BEWidth) and so
+    // the number of iterations that remain to be run in the original loop is a
+    // multiple Count == (1 << Log2(Count)) because Log2(Count) <= BEWidth (we
----------------
This change is unnecessary.

================
Comment at: lib/Transforms/Utils/LoopUnrollRuntime.cpp:365-367
@@ +364,5 @@
+    // At that point ModValAdd could not overflow as ModValTmp < Count
+    ModVal = B.CreateURem(ModValAdd,
+                          ConstantInt::get(BECount->getType(), Count),
+                          "xtraiter");
+    // And finaly we get correct and overflow safe remainder counter
----------------
What is the final expression for `ModVal`? From the code it looks like
```
  ModVal = ((BECount % Count) + 1) % Count
```
Do we really need double urem here? If so, could you please add a comment explaining why we need exactly this?

================
Comment at: lib/Transforms/Utils/LoopUnrollRuntime.cpp:368
@@ +367,3 @@
+                          "xtraiter");
+    // And finaly we get correct and overflow safe remainder counter
+  }
----------------
Nitpick: dot missing at the end.


Repository:
  rL LLVM

http://reviews.llvm.org/D18202