[PATCH] D115329: [LoopVectorize] Pass a vector type to isLegalMaskedGather/Scatter

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1590
+      Ty = VectorType::get(Ty, VF);
     return (LI && TTI.isLegalMaskedGather(Ty, Align)) ||
            (SI && TTI.isLegalMaskedScatter(Ty, Align));
----------------
According to the default `VF = ElementCount::getFixed(1)` it looks like we can still have an inconsistency during vectorisation where we get different answers? I think it's also called from `collectElementTypesForWidening` so we could potentially say a gather is legal when the type is scalar, but then say it's illegal in setCostBasedWideningDecision. I wonder if it's worth adding a comment there saying something like:

  // We're simply querying at this point if the target even supports any vector
  // gathers and scatters for the given element type? Certain vector forms may
  // still be illegal, but setCostBasedWideningDecision will distinguish between
  // the legal behaviour for different VFs at that point and generate costs accordingly.

Also, does it matter that `LoopVectorizationCostModel::isScalarWithPredication` is still passing in an element type rather than a vector? For example, `collectLoopUniforms` calls `isScalarWithPredication` and at that point knows the VF. It may be fine, but I'm just a bit worried we might be swapping one inconsistency for another that's all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115329