This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Improve vector reverse lowering
ClosedPublic

Authored by dmgreen on Apr 20 2021, 12:57 PM.

Details

Summary

This improves the lowering of v8i16 and v16i8 vector reverse shuffles. Instead of going via a generic tbl it uses a rev64; ext pair, as already happens for v4i32.

Diff Detail

Event Timeline

dmgreen created this revision.Apr 20 2021, 12:57 PM
dmgreen requested review of this revision.Apr 20 2021, 12:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 20 2021, 12:57 PM
david-arm accepted this revision.Apr 21 2021, 1:03 AM
david-arm added a subscriber: david-arm.

LGTM!

This revision is now accepted and ready to land.Apr 21 2021, 1:03 AM
sdesmalen added inline comments.Apr 21 2021, 1:44 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
9060

nit: Is this condition necessary?

I know for LLVM IR nodes the result type doesn't necessarily have the same number of elements as the source vectors (but instead equals the number of elements in the mask), but is the same true for VECTOR_SHUFFLE?

The reason for asking is that I see in ISDOpcodes that it says:

/// VECTOR_SHUFFLE(VEC1, VEC2) - Returns a vector, of the same type as
/// VEC1/VEC2.

Thanks!

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
9060

Yeah, I added it as an additional safety check. It didn't alter any of the test cases I had, but I figured it was better safe than sorry.

I can remove it though, if it is guaranteed that they will already match in size.

sdesmalen added inline comments.Apr 21 2021, 4:36 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
9060

I think the sizes are supposed to be the same, looking at SelectionDAGBuilder::visitShuffleVector there is code that ensures the sizes match. To be sure, maybe you can add an assert instead?

Matt added a subscriber: Matt.Apr 21 2021, 6:19 AM
dmgreen updated this revision to Diff 339287.Apr 21 2021, 9:53 AM

Now with extra asserts.

This revision was landed with ongoing or failed builds.Apr 22 2021, 1:01 PM
This revision was automatically updated to reflect the committed changes.