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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
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;
----------------
david-arm wrote:
> 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();
>   }
>   ...
> 
> 
I was a bit worried it would get confusing with using both A.Width.isScalable() in combination EstimatedWidthA, but perhaps splitting it up is more clear as you suggest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113209