[PATCH] D43058: [LV][near-NFC] move isLegalMasked* from LoopVectorizationLegality to LoopVectorizationCostModel

[Hideki Saito via Phabricator via llvm-commits]

hsaito added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:6199
 
+  if (!EnableCondStoresVectorization && NumPredStores) {
+    ORE->emit(createMissedAnalysis("ConditionalStore")
----------------
Does anyone want to keep this? This is the reason why I didn't bother making sure the same message will come out when we bail out here.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:6606
+        // TODO: Cost modeling for emulated masked load/store is completely
+        // broken at this moment. Do not apply discount logic.
+        if (!(isa<LoadInst>(&I) ||
----------------
If discount logic is applied, higher cost computed in getMemInstScalarizationCost() is thrown away.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:6608
+        if (!(isa<LoadInst>(&I) ||
+              (isa<StoreInst>(&I) && NumPredStores > NumberOfStoresToPredicate)) &&
+            computePredInstDiscount(&I, ScalarCosts, VF) >= 0)
----------------
NumPredStores here is needed for the following LIT tests. They expect computation of values to be stored also being scalarized, and the "discount" function does that work.

Transforms/LoopVectorize/AArch64/predication_costs.ll
Transforms/LoopVectorize/float-induction.ll
Transforms/LoopVectorize/if-pred-stores.ll




================
Comment at: test/Transforms/LoopVectorize/conditional-assignment.ll:4
 
-; CHECK: remark: source.c:2:8: loop not vectorized: store that is conditionally executed prevents vectorization
+; CHECK: remark: source.c:2:8: the cost-model indicates that vectorization is not beneficial
 
----------------
I think we need to fix the missed analysis message so that it start with "loop not vectorized: ".
That would be a separate patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D43058