[PATCH] D30653: [LV] Refactor Cost Model's selectVectorizationFactor(), driven by a LoopVectorizationPlanner

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:6163
 
-LoopVectorizationCostModel::VectorizationFactor
-LoopVectorizationCostModel::selectVectorizationFactor(bool OptForSize) {
-  // Width 1 means no vectorize
-  VectorizationFactor Factor = {1U, 0U};
-  if (OptForSize && Legal->getRuntimePointerChecking()->Need) {
+unsigned LoopVectorizationCostModel::computeMaxVFOrFail(bool OptForSize) {
+  const unsigned Fail = 0;
----------------
mkuper wrote:
> Any reason not to use Optional<> instead?
Ahh, sure, done.

Any suggestion for a better method name, avoiding having two computeMaxVF()'s?


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7385
+    DEBUG(dbgs() << "LV: Using user VF " << UserVF << ".\n");
+    if (UserVF == 1)
+      return {UserVF, 0};
----------------
mkuper wrote:
> Maybe:
> ```
> DEBUG(dbgs() << "LV: Using user VF " << UserVF << ".\n");
> assert(isPowerOf2_32(UserVF) && "VF needs to be a power of two");
> if (UserVF != 1)
>   CM.selectUserVectorizationFactor(UserVF);
> return {UserVF, 0}
> ```
> ?
> 
> (I think isPowerOf2_32(1) is true.)
Right, isPowerOf2_32(1) is true.

Will actually simplify this further by dropping the "if (UserVF != 1)", since selectUserVectorizationFactor() knows how to handle a UserVF of 1 (namely, by doing nothing).


https://reviews.llvm.org/D30653