[PATCH] D147522: [LoopVectorize] Take vscale into account when deciding to create epilogues

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added a comment.

I'm worried the test changes don't look relevant to the code change.  I mean, sure the changes are are the effect of the code change, but the tests themselves are not related to epilogue vectorisation? nor do they specify what to tune for and thus they've only changed because of the current default for this tuning option.  This means the tests will change if/when the default changes.  I think it would be better to have a dedicated test file that shows the result of a simple loop when RUN using several cpu tuning parameters.  And for the other/existing loop vectorisation tests to be independent of the effect of tuning when the effect is not relevant to what the test is protecting (which is how you're already handling runtime-check-size-based-threshold.ll).



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5580-5581
+  if (VF.isScalable())
+    if (std::optional<unsigned> VScale = getVScaleForTuning())
+      Multiplier = *VScale;
+  if ((Multiplier * VF.getKnownMinValue()) >= EpilogueVectorizationMinVF)
----------------
You could use `Multiplier = getVScaleForTuning().value_or(1)` here if you wanted.  At some point in the future we might want to change this to `value_or(MinVScale)` anyway.


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

https://reviews.llvm.org/D147522