[PATCH] D113209: [LV] Use VScaleForTuning to fine-tune the cost per lane.

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6015
   unsigned MaxTripCount = PSE.getSE()->getSmallConstantMaxTripCount(TheLoop);
-
   if (!A.Width.isScalable() && !B.Width.isScalable() && FoldTailByMasking &&
----------------
nit: Could you remove the unnecessary white space before merging? Thanks!


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6030
+  // Improve estimate for the vector width if it is scalable.
+  ElementCount EstimatedWidthA = A.Width;
+  ElementCount EstimatedWidthB = B.Width;
----------------
nit: This is just a suggestion, but it feels a little odd to be rescaling ElementCount by vscale, because this is already implicit for a scalable element count. For example, what we're doing here is taking a <vscale x 4> ElementCount and then multiplying by another vscale so we end up effectively with an ElementCount like <vscale x vscale x 4>. Is it worth just using unsigned values instead?

  unsigned EstimatedWidthA = A.Width.getKnownMinValue();
  unsigned EstimatedWidthB = B.Width.getKnownMinValue();
  if (Optional<unsigned> VScale = TTI.getVScaleForTuning()) {
    if (A.Width.isScalable())
      EstimatedWidthA *= VScale.getValue();
    if (B.Width.isScalable())
      EstimatedWidthB *= VScale.getValue();
  }
  ...




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113209