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

[Kerry McLaughlin via Phabricator via llvm-commits]

kmclaughlin added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6284
   const DataLayout &DL = TheFunction->getParent()->getDataLayout();
+  for (Type *T : ElementTypesInLoop) {
+    MinWidth = std::min<unsigned>(
----------------
david-arm wrote:
> If we're relying upon the element types being collected for correctness here is it worth adding an assert:
> 
>   assert(!ElementTypesInLoop.empty() && "Unable to calculate smallest and widest types");
Since we only add the element types of loads, stores and (reduction) phi nodes in collectAllElementTypesInLoop(), I think it is possible for ElementTypesInLoop to be empty here for a given loop. I tried adding the assert and did run into quite a few test failures with the change.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6293
 
+void LoopVectorizationCostModel::collectAllElementTypesInLoop() {
   // For each block.
----------------
david-arm wrote:
> Do we ever call this function more than once for a given loop? I wonder if it's worth clearing the list at the start just in case, i.e.
> 
>   ElementTypesInLoop.clear();
> 
I don't think we ever call this function more than once for each loop, though I think it's still worth clearing the list as you suggest.


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

https://reviews.llvm.org/D102253