[PATCH] D96021: [LoopVectorize] NFC: Move UserVF feasibility checks to separate function.

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1636-1638
+  /// \return UserVF if it is non-zero and there are no dependences, otherwise
+  /// a clamped value. For scalable UserVF, the resulting feasible VF may be a
+  /// fixed-width VF.
----------------
```
  /// \return UserVF if it is non-zero and there are no dependences, otherwise
  /// return a clamped value. For a scalable UserVF, the resulting feasible VF
  /// may be fixed-width.```


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5555-5557
+  MinBWs = computeMinimumValueSizes(TheLoop->getBlocks(), *DB, &TTI);
+  unsigned SmallestType, WidestType;
+  std::tie(SmallestType, WidestType) = getSmallestAndWidestTypes();
----------------
should this be part of the lambda since it's not used elsewhere? Not sure if it's an issue but `MinBWs` could now be computed where it previously wasn't.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5559
+
+  auto GetFeasibleMaxVF = [&]() -> ElementCount {
+    // First analyze the UserVF, fall back if the UserVF should be ignored.
----------------
should functions start with lower case?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5691
+                                                  unsigned WidestType) {
+  if (!UserVF.isNonZero())
+    return None;
----------------
`if (UserVF.isZero())`?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5714-5715
+  // If UserVF is specified and there are dependencies, check if it's legal.
+  if (Legal->isSafeForAnyVectorWidth())
+    return UserVF;
 
----------------
can this be moved above `MaxSafeVectorWidthInBits`?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5749-5759
+  LLVM_DEBUG(dbgs() << "LV: User VF=" << UserVF
+                    << " is unsafe, clamping to max safe VF=" << MaxSafeVF
+                    << ".\n");
+  ORE->emit([&]() {
+    return OptimizationRemarkAnalysis(DEBUG_TYPE, "VectorizationFactor",
+                                      TheLoop->getStartLoc(),
+                                      TheLoop->getHeader())
----------------
it's nice how you've reused the remark for the debug message above, not really important for this patch but it would be good to do the same thing here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96021