Page MenuHomePhabricator

[AArch64] Improve vector reverse lowering

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



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.Tue, Apr 20, 12:57 PM
dmgreen requested review of this revision.Tue, Apr 20, 12:57 PM
Herald added a project: Restricted Project. · View Herald TranscriptTue, Apr 20, 12:57 PM
david-arm accepted this revision.Wed, Apr 21, 1:03 AM
david-arm added a subscriber: david-arm.


This revision is now accepted and ready to land.Wed, Apr 21, 1:03 AM
sdesmalen added inline comments.Wed, Apr 21, 1:44 AM

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.



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.Wed, Apr 21, 4:36 AM

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.Wed, Apr 21, 6:19 AM
dmgreen updated this revision to Diff 339287.Wed, Apr 21, 9:53 AM

Now with extra asserts.

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