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

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h:126
 
-  bool isScalable() const { return Scalable.Value; }
+  /// \return true if scalable vectorization has been disabled explicitly.
+  bool disableScalableVectorization() const {
----------------
nit: explicitly disabled (?)


================
Comment at: llvm/include/llvm/Transforms/Vectorize/LoopVectorize.h:177
 
+/// Reports a vectorization informative message: print \p Msg for debugging
+/// purposes as well as an optimization remark. Uses either \p I as location of
----------------
nit: `Reports an informative vectorization message:`


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1061
 
 /// Write a record \p DebugMsg about vectorization failure to the debug
 /// output stream. If \p I is passed, it is an instruction that prevents
----------------
drop failure


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5571
+
+  // Disable scalable vectorization, if the loop contains unsupported
+  // reductions.
----------------
nit: comma can be dropped


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5668
+  ElementCount MaxFixedVF = ElementCount::getFixed(1);
+  if (auto Max = getMaxVectorSize(ConstTripCount, SmallestType, WidestType,
+                                  MaxSafeFixedVF))
----------------
nit: MaxVF?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5840
   // Note that both WidestRegister and WidestType may not be a powers of 2.
-  auto MaxVectorSize =
-      ElementCount::getFixed(PowerOf2Floor(WidestRegister / WidestType));
-
-  LLVM_DEBUG(dbgs() << "LV: The Smallest and Widest types: " << SmallestType
-                    << " / " << WidestType << " bits.\n");
+  auto MaxVectorSize = ElementCount::get(
+      PowerOf2Floor(WidestRegister.getKnownMinSize() / WidestType),
----------------
`MaxVF`?


================
Comment at: llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll:185-188
+; CHECK-DBG: Found feasible scalable VF = vscale x 2
+; CHECK-DBG: LV: Selecting VF: 4.
 ; CHECK-LABEL: @test4
+; CHECK: <4 x i32>
----------------
Should this select the feasible scalable VF? I suppose this is related to the issue @david-arm pointed out in `LoopVectorizationCostModel::computeFeasibleMaxVF`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98509