[PATCH] D99750: [LV, VP] RFC: VP intrinsics support for the Loop Vectorizer (Proof-of-Concept)

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a subscriber: loralb.
fhahn added a comment.

It looks like the description is out-of date, it would be great if it could be updated to be in sync with the patch. @rogfer01  and @loralb had a poster at EuroLLVM about a different approach and I am curious if you by any chance already took a look at their approach?

Is it possible to have multiple EVLs per vector region or will it always be a single one? Does the EVL argument need to be passed to each vector op for correctness? On RISCV, would it be sufficient/desired to set the vector length at the beginning of the vector loop?

The patch seems to add code for using VP for both masking and EVL, but the masking part seems to be mostly unused in the current version. Would it be possible to disentangle them?



================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:908
+    OS << "\\n";
+    Plan.TripCount->print(OS, SlotTracker);
+    OS << " := TripCount";
----------------
This seems like an unrelated fix?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:247
+  /// vectorization with the runtime vector length.
+  std::optional<unsigned> ElementWidth;
+
----------------
This is only used by the EVL recipe unless I missed something. Could it just be a member of the recipe instead?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:2042
+/// A recipe to generate Explicit Vector Length (EVL) value to be used with
+/// VPred intrinsics.
+class VPEVLRecipe : public VPRecipeBase, public VPValue {
----------------
It would probably be helpful to explain here how the explicit vector length is computed 


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp:1386
+  assert(State.UF < 2 &&
+         "Neither unrolling, nor interleaving is supported by RVV VLA");
+  State.Builder.SetInsertPoint(State.CFG.PrevBB->getFirstNonPHI());
----------------
What’s RVV VLA?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp:1387
+         "Neither unrolling, nor interleaving is supported by RVV VLA");
+  State.Builder.SetInsertPoint(State.CFG.PrevBB->getFirstNonPHI());
+  for (unsigned Part = 0, UF = State.UF; Part < UF; ++Part) {
----------------
Why does the insert point need resetting?


================
Comment at: llvm/test/Transforms/LoopVectorize/X86/vectorize-vp-intrinsics.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -passes=loop-vectorize \
----------------
Do the current changes have any impact on the test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99750