[PATCH] D98509: [LV] Calculate max feasible scalable VF.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5582-5584
+  MinBWs = computeMinimumValueSizes(TheLoop->getBlocks(), *DB, &TTI);
+  unsigned SmallestType, WidestType;
+  std::tie(SmallestType, WidestType) = getSmallestAndWidestTypes();
----------------
c-rhodes wrote:
> should this be moved closer to where it's used?
This is actually moved as close as possible, because the first use is WidestType (line 5591).


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5819
+      ComputeScalableMaxVF);
+  MaxVectorSize = MinVF(MaxVectorSize, MaxSafeVF);
   LLVM_DEBUG(dbgs() << "LV: The Widest register safe to use is: "
----------------
c-rhodes wrote:
> I know this came before your patch but I find it a bit confusing VF and VectorSize are use interchangeably 
I see how this is confusing, especially because the VectorSize is actually not a size, but rather an element count. The subtle distinction between the two is that the VF is more of a loop-vectorizer concept (the number of  lanes handled per vector iteration), whereas the MaxVectorSize (or better yet, MaxVectorElementCount) is the maximum number of elements in a target's vector register.

Given that I'm changing a lot of lines in this function, it probably doesn't really make the diff much worse if I at least remove part of the confusion by renaming VectorSize -> VectorElementCount.


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

https://reviews.llvm.org/D98509