[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/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.
----------------
ABataev wrote:
> 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.
Ah I see, would be good to update


================
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:
> > 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
Sounds good! Are you aware of any other potential correctness issues?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99750