[PATCH] D50480: [LV] Vectorizing loops of arbitrary trip count without remainder under opt for size

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:2663
+  // overflows: the vector induction variable will eventually wrap to zero given
+  // that it starts at zero and its Step is a power of two; the loop will then
+  // exit, with the last early-exit vector comparison also producing all-true.
----------------
hsaito wrote:
> I think there is a danger in assuming UF being a power of two. Granted that there may be other parts of LV already assuming it, I still wouldn't like to see any more of those being added. If new code is assuming power of two UF, it's best if we ensure that is really the case (e.g., when foldTailByMansking() is true, assert that UF is power of two). Better yet, check VF*UF is power of two here since that's the assumption this code has.
> 
OK, will add an assert that VF*UF is a power of 2 below under the `if (Legal->foldTailByMasking())`.



================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:2673
   // unroll factor (number of SIMD instructions).
-  Constant *Step = ConstantInt::get(TC->getType(), VF * UF);
   Value *R = Builder.CreateURem(TC, Step, "n.mod.vf");
 
----------------
hsaito wrote:
> This Urem creation should be skipped if we aren't generating remainder.
This Urem is also used to round N up to a multiple of Step, i.e., when we're not generating remainder.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4977
+    return MaxVF;
+
+  return None;
----------------
hsaito wrote:
> I think we need to add
> 
>      if (TC==0) { emit one kind of remark }
>      else { emit another kind of remark }
> 
> here ---- in order to match previous capability.
> 
> 
OK, will retain the previous MissedAnalysis remarks here, in addition to the new ones supplied by `canFoldTailByMasking()`.


Repository:
  rL LLVM

https://reviews.llvm.org/D50480