Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

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

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



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:

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.

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

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.


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

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, thanks!

This revision was automatically updated to reflect the committed changes.