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

[Sanjoy Das via llvm-commits]

sanjoy added a comment.

Minor drive-by comment.


================
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
----------------
evstupac wrote:
> mzolotukhin wrote:
> > 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?
> Potentially BECount + 1 can unsigned overflow, while (BECount % Count) +1 can not. That is why double URem is used. I can try to simplify this to something like this
> ```
> ModVal = ((Count - ModValAdd) >> (ModVal->getSclarType()->getPrimitiveSizeInBits() - 1) & ModValAdd
> ``` or to select here or somewhere in further combiners. However non-power-of-2 is not frequent case as for runtime unrolling it comes only from user specified pragma.
Alternatively, you could have `ModVal` as `BECount == -1 ? <constant expr> : ((BECount +nuw 1) % Count`.


Repository:
  rL LLVM

http://reviews.llvm.org/D18202