[PATCH] D137285: [LV] Account for minimum vscale when rejecting scalable vectorization of short loops

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5207
+  unsigned WidestRegisterMinEC = MaxVectorElementCount.getKnownMinValue();
+  if (MaxVectorElementCount.isScalable()) {
+    if (TheFunction->hasFnAttribute(Attribute::VScaleRange)) {
----------------
nit: I think you can combine these two if statements into one, i.e.

  if (MaxVectorElementCount.isScalable() && TheFunction->hasFnAttribute(Attribute::VScaleRange)) {
    auto Attr = TheFunction->getFnAttribute(Attribute::VScaleRange);
    auto Min = Attr.getVScaleRangeMin();
    WidestRegisterMinEC *= Min;
  }


================
Comment at: llvm/test/Transforms/LoopVectorize/RISCV/short-trip-count.ll:4
 
 ; FIXME: Using a <4 x i32> would be strictly better than tail folded
 ; scalable vectorization in this case.
----------------
I think this comment is no longer valid.


================
Comment at: llvm/test/Transforms/LoopVectorize/RISCV/short-trip-count.ll:6
 ; scalable vectorization in this case.
 define void @small_trip_count(i32* nocapture %a) nounwind vscale_range(4,1024) {
 ; CHECK-LABEL: @small_trip_count(
----------------
Perhaps it's better to have two tests now - one with min vscale = 4, and one with min vscale = 1, so that you can demonstrate the expected behaviour of using tail-folded loops for the latter as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137285