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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9071
+        assert(isa<TruncInst>(Instr) ||
+               isa<VPEVLRecipe>(HeaderVPBB->getFirstNonPhi()));
         Recipe->insertBefore(*HeaderVPBB, HeaderVPBB->getFirstNonPhi());
----------------
Is this still needed? The latest version only introduces `VPEVLRecipe` in `addCanonicalIVRecipes`.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:2065
+/// for current tail-folding implementation.
+/// 2. The second way is to insert instructions to compute min(VF, trip_count -
+/// index) for each vector iteration.
----------------
IIUC this is what the recipe actually computes at the moment, right?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp:1415
+
+    // TODO: Add support for MaxSafeDist for correct loop emission.
+    Constant *WidthArg = State.Builder.getInt32(ElementWidth);
----------------
ABataev wrote:
> fhahn wrote:
> > Does this mean the code we generate is not correct cases where `MaxSafeDist` needs to be handled?
> Yes, as I mentioned above, this is just a first patch, some extra work is required to handle correctly all corner cases. Plus, MaxSafeDist analysis needs to be adjusted for EVL, since we can support non-power-of-2 value here
Thanks for clarifying! In that case, I think it would be better to not use EVL lowering for such cases so LV will only generate correct code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99750