This is an archive of the discontinued LLVM Phabricator instance.

[LV] Fix crash for reverse interleaved loads with gap under fold-tail
ClosedPublic

Authored by Ayal on Aug 29 2021, 9:13 AM.

Details

Summary

This patch fixes the crash found by PR51614:
whenever doing tail folding, interleave groups must be considered under mask.

Another fix D108900 will follow for targets that support masked vector loads and stores:
when *deciding* to vectorize with masked interleave groups, check if the access
is reverse - which is currently not supported; rather than (only) asserting when
computing cost and generating code.

Diff Detail

Event Timeline

Ayal created this revision.Aug 29 2021, 9:13 AM
Ayal requested review of this revision.Aug 29 2021, 9:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 29 2021, 9:13 AM
bjope added a subscriber: bjope.Aug 29 2021, 12:56 PM

I can confirm that this patch solves the problem we saw. Both the reduced example provided in pr51614 and the full reproducer we have for our out-of-tree target.

For the actual fix, I really have no idea if this is the way to solve the problem so please wait for @fhahn or someone else knowing this code.

Thanks!

uabelho edited reviewers, added: nikic, sdesmalen, dorit; removed: uabelho.Sep 13 2021, 3:32 AM
fhahn accepted this revision.Sep 13 2021, 3:38 AM

LGTM, thanks!

This revision is now accepted and ready to land.Sep 13 2021, 3:38 AM
fhahn added a comment.Sep 13 2021, 3:52 AM

Another fix will follow for targets that support masked vector loads and stores:
when *deciding* to vectorize with masked interleave groups, check if the access
is reverse - which is currently not supported; rather than (only) asserting when
computing cost and generating code.

Could you update the description/commit message with a reference to the patch?

Ayal edited the summary of this revision. (Show Details)Sep 21 2021, 10:44 AM
Ayal added a comment.Sep 21 2021, 11:00 AM

Another fix will follow for targets that support masked vector loads and stores:
when *deciding* to vectorize with masked interleave groups, check if the access
is reverse - which is currently not supported; rather than (only) asserting when
computing cost and generating code.

Could you update the description/commit message with a reference to the patch?

Sure, done.