[PATCH] D102253: [LV] Prevent vectorization with unsupported element types.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

I left a few more comments, but other than that, I'm mostly happy with the approach taken in this patch.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1305
 
+  /// Collect all element types in the loop for which vectors are needed.
+  void collectAllElementTypesInLoop();
----------------
nit: widening is


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5663
 
+bool LoopVectorizationCostModel::canWidenLoopWithScalableVectors(
+    ElementCount MaxVF) const {
----------------
Given the recent change in direction to use invalid costs to avoid vectorization with scalable vectors, this function should be quite limited in size/scope, so maybe it's better not moving this out into a separate function like you've done here, but just add the extra condition to `getMaxLegalScalableVF` as in:

i.e.
  if (any_of(ElementTypesInLoop, [] (const Type *Ty) { return !Ty->isVoidTy() && !TTI.isElementTypeLegalForScalableVector(Ty); })) {
    reportVectorizationInfo(...);
    return ElementCount::getScalable(0);
  }


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9877
 
+  CM.collectAllElementTypesInLoop();
+
----------------
Could you split out the non-functional change which adds `collectAllElementTypesInLoop` and changes `getSmallestAndWidestTypes` accordingly, and commit this as a separate (NFC) patch?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102253/new/

https://reviews.llvm.org/D102253