Page MenuHomePhabricator

[VPlan] Support to widen call intructions in VPlan native path
ClosedPublic

Authored by Kazhuu on Feb 23 2021, 6:02 AM.

Details

Summary

Add support to widen call instructions in VPlan native path by using a correct recipe when such instructions are encountered. This is already used by inner loop vectorizer.

Previously call instructions got handled by wrong recipes and resulted in unreachable instruction errors like this one: https://bugs.llvm.org/show_bug.cgi?id=48139.

Diff Detail

Event Timeline

Kazhuu created this revision.Feb 23 2021, 6:02 AM
Kazhuu requested review of this revision.Feb 23 2021, 6:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 23 2021, 6:02 AM
a.elovikov added inline comments.
llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
75

For my education, how do we know that such wide version is available? What if the vectorization was forced by OMP pragma (so bypass any dependence analysis legality checks), but the function being called is some arbitrary function that doesn't have a wide variant (i.e. not an intrinsic and no OMP's vector variant available)?

Also, what was the previous behavior? Did we serialize the call?

Kazhuu added inline comments.Mar 3 2021, 8:29 PM
llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
75

This is actually a good point. What I've checked the code the inner loop vectorizer use this function VPRecipeBuilder::tryToWidenCall() to try to widen the call. I guess something similar should be implemented here instead of trying to widen all calls. But of course avoid code duplication. Now this works because I've been only testing with simple intrinsic calls.

fhahn accepted this revision.Mar 4 2021, 9:46 AM

LGTM, thanks. As mentioned in the inline comment, this should only restore the behavior we had before splitting of VPWidenRecipe into VPWidenCall & co.

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
75

For my education, how do we know that such wide version is available?

The short answer is we don't and we never have. This patch should restore the previous behavior, which regressed after splitting up VPWidenRecipe.

In its current state, the VPlan native path is missing a lot of legality checks. Currently it's the responsibility of the user to make sure it is only used for supported loops. Most of the recent work has been spent working on moving more bits of the inner loop vectorizer to VPlan, which in turn should eventually be useable for the VPlan native path as well.

This revision is now accepted and ready to land.Mar 4 2021, 9:46 AM
a.elovikov added inline comments.Mar 4 2021, 9:55 AM
llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
75

Thank you for the detailed response!

This patch should restore the previous behavior

That makes sense.

Thanks for the review and comments @fhahn. I need someone to submit this patch for me because I don't have commit rights. Commit can be made with name Mauri Mustonen and email of mauri.mustonen@tuni.fi, thanks!

This revision was automatically updated to reflect the committed changes.