[PATCH] D91718: [LV] Legalize scalable VF hints

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5419
+             "unfeasible. Using fixed-width vectorization instead.\n");
+      MaxSafeVF = ElementCount::getFixed(MaxSafeElements);
+    }
----------------
This does something different than what I suggested though.

For the following example, your code would set the MaxSafeVF to 32 elements instead of using a fixed VF of 4 elements (fixed):

  #pragma clang loop vectorize_width(4, scalable)
  for (int i=0; i<N; ++i)
    a[i + 32] = a[i] + 1;   // for some int* a

I think a VF of 4 would be preferred, because otherwise selectiondag will need to expand `<32 x i32>` to 8 x `<4 x i32>` fixed-width vectors (assuming 128bit vectors), which would be somewhat similar to requesting an interleave-count of 8.


================
Comment at: llvm/test/Transforms/LoopVectorize/scalable-vf-hint.ll:20
+; For scalable vectorization 'vscale' has to be considered, in this example
+; vectorization is only safe when vscale=1. The default max vscale is 1 unless
+; a target specifies otherwise, therefore it is safe to vectorize this loop.
----------------
At least until we add lowering of scalable vectors for targets that don't support it, I would rather the code explicitly checks if the target can handle them and ignore the hint if it doesn't (a similar suggestion was made on D88962).


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

https://reviews.llvm.org/D91718