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

[Alexey Bataev via Phabricator via llvm-commits]

ABataev 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());
----------------
fhahn wrote:
> Is this still needed? The latest version only introduces `VPEVLRecipe` in `addCanonicalIVRecipes`.
Still needed. VPWidenIntOrFpInductionRecipe is emitted after addCanonicalIVRecipes and needs to be inserted as a PHI recipe. I'm not sure about TODO, I think it should be removed. But we still need to insert VPWidenIntOrFpInductionRecipe before VPEVLRecipe, which is emitted immediately after canonical IV recipe.


================
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.
----------------
fhahn wrote:
> IIUC this is what the recipe actually computes at the moment, right?
No, currently it emits number 3. Other are not fully supported, they will require some extra work.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp:1415
+
+    // TODO: Add support for MaxSafeDist for correct loop emission.
+    Constant *WidthArg = State.Builder.getInt32(ElementWidth);
----------------
fhahn wrote:
> 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.
Will add a check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99750