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

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

In D99750#4350780 <https://reviews.llvm.org/D99750#4350780>, @fhahn wrote:

> 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?

Didd some cleanup.

> 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?

Craig is right, only single EVL is expected.

> 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?

I rather doubt it is possible, otherwise the code won't work correctly, at all. Masking is important part of predicated intrinsics.



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


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


================
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());
----------------
fhahn wrote:
> What’s RVV VLA?
RISCV Vector 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) {
----------------
fhahn wrote:
> Why does the insert point need resetting?
Must be the very first instructions in the loop, all other instructions depend on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99750