This is an archive of the discontinued LLVM Phabricator instance.

[GISel][AArch64] Combine G_BUILD_VECTOR(G_UNMERGE) with undef elements
ClosedPublic

Authored by dmgreen on Aug 16 2023, 2:57 AM.

Details

Summary

This extends the existing legalization combine to fold G_BUILD_VECTOR where the sources are all from the same G_UNMERGE, to handle cases where some of the lanes are undef. This comes up in the legalization of <3 x ..> vectors in AArch64, where they are padded with undef.

There are two choices for what to create. This patch just removes the G_BUILD_VECTOR/G_UNMERGE, losing the information about which lanes are undef. The alternative would be to generate an identity G_SHUFFLE_VECTOR with undef lanes marked as undef. I think both have advantages and disadvantages.

Diff Detail

Event Timeline

dmgreen created this revision.Aug 16 2023, 2:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 16 2023, 2:57 AM
dmgreen requested review of this revision.Aug 16 2023, 2:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 16 2023, 2:57 AM
Herald added a subscriber: wdng. · View Herald Transcript
arsenm added inline comments.Aug 16 2023, 6:49 AM
llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h
969

shuffle vector isn't defined as a legalization artifact. Probably best if the artifact combiner avoids introducing something that isn't an artifact

dmgreen updated this revision to Diff 551937.Aug 21 2023, 2:15 AM
dmgreen edited the summary of this revision. (Show Details)

OK - this now just removes the nodes. It does mean we lose information about which lanes are undef, but there are less instructions to deal with and we don't need to do it just when the shuffle is legal.

arsenm accepted this revision.Aug 21 2023, 4:13 AM

OK - this now just removes the nodes. It does mean we lose information about which lanes are undef, but there are less instructions to deal with and we don't need to do it just when the shuffle is legal.

If you think that's useful, it might make sense to add a pre-legalizer combine for that

This revision is now accepted and ready to land.Aug 21 2023, 4:13 AM
This revision was landed with ongoing or failed builds.Aug 22 2023, 6:25 AM
This revision was automatically updated to reflect the committed changes.