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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:5559
+
+  auto GetFeasibleMaxVF = [&]() -> ElementCount {
+    // First analyze the UserVF, fall back if the UserVF should be ignored.
----------------
c-rhodes wrote:
> should functions start with lower case?
GetFeasibleMaxVF is also a local variable (with a function as value) so I thought it should start with upper case. But it seems the code-base is a bit undecided; in a quick grep I counted slightly (15%) more cases starting with an upper-case, than starting with a lower-case.


================
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())
----------------
c-rhodes wrote:
> 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
Thanks. I'm happy to see if I can do that in a separate patch (it requires some changes to the tests).


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

https://reviews.llvm.org/D96021