[PATCH] D96522: [LV] Try larger VFs if VF is unprofitable for small types.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

In D96522#2557790 <https://reviews.llvm.org/D96522#2557790>, @fhahn wrote:

> In D96522#2557608 <https://reviews.llvm.org/D96522#2557608>, @dmgreen wrote:
>
>> I like the idea of this. I think it often makes sense to at least try a larger vector size, even if it's over the vector width.
>
> Agreed! ideally we would be confident enough in the cost-model to more liberally explore larger VFs and trust the cost-model to pick the best one, even in the presence of larger ones. I think this patch is a small first step in this direction, with potential to extend this to more cases in the future.

The approach seems sensible to me. Is the reason for not exploring larger VFs more liberally because the cost-model doesn't always accurately represent/consider the legalization costs?



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5786
 
+  LLVMContext &Context = TheLoop->getHeader()->getContext();
+  // The largest type limits the vectorization factor, but this can be too
----------------
nit: Are you planning to handle more special cases like this in the future? If so, then it may be worth moving this to its own `shouldMaximizeBandwidth` function.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5795
+  if (SmallestType <= 32 && SmallestType < WidestType &&
+      !MaxVectorSize.isScalable()) {
+    Type *SmallVT = FixedVectorType::get(
----------------
nit: this condition is currently always true. Can you change this into an assert?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96522