This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SVE] Add support for DestructiveBinary and DestructiveBinaryComm DestructiveInstTypes
ClosedPublic

Authored by cameron.mcinally on Jan 30 2020, 7:44 AM.

Details

Summary

This is the third patch in a collection to support prefixing destructive operations, with the MOVPRFX instruction, to build constructive operations. The previous patch is D73212.

Here we add support for DestructiveBinary and DestructiveBinaryComm DestructiveInstTypes, as well as the lowering code to expand the new Pseudos into the final movprfx+instruction pairs.

This patch comes mostly from D71712, with some modifications to SelectDupZero(...) in AArch64ISelDAGToDAG.cpp. IINM, the SelectDupZero(...) difference works around a separate feature (not upstreamed yet) to make use of SPLAT_VECTOR for zero vectors.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptJan 30 2020, 7:44 AM
cameron.mcinally marked an inline comment as done.Jan 30 2020, 7:49 AM
cameron.mcinally added inline comments.
llvm/lib/Target/AArch64/AArch64InstrFormats.td
39

Note that I took some liberties adding all the enumerators, even though most aren't used yet. I think it makes sense to put them in place now to avoid unnecessary churn later. There should be little risk since they are not currently used.

If anyone feels strongly about this, I will only add the tested enumerators...

Fix whitespace issue.

sdesmalen added inline comments.Feb 12 2020, 8:57 AM
llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
167

BUILD_VECTOR only works for fixed-width vectors, so you'll need to check for SPLAT_VECTOR instead.

This also puzzles me a bit on how the tests could pass; I'd either expect the pattern not to match because the incoming value is not a BUILD_VECTOR, or I'd expect an assertion in BUILD_VECTOR to fail because it doesn't work with scalable vectors.

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
167

Hmm, that's strange. I assumed there was a separate missing lowering somewhere for zeroinitializer.

I've definitely seen other DestructiveInstr types matching DUP/SPLAT there. Maybe I missed something subtle in this patch.

Will check it out...

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
167

Ah, got it. ISel is missing support for FP splat_vectors.

I'll do that under a separate Diff, assuming I can find a good way to add standalone tests for the splat_vectors...

Update BUILD_VECTOR->SPLAT_VECTOR in SelectDupZero(...). The BUILD_VECTOR problem was corrected in D74632.

cameron.mcinally marked 3 inline comments as done.Feb 19 2020, 2:13 PM
sdesmalen accepted this revision.Feb 21 2020, 10:43 AM

LGTM!

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
167

Thanks for fixing this and adding the support for splat_vectors in the other patch!

176

nit: missing either break; or otherwise LLVM_FALLTHROUGH statement here.

This revision is now accepted and ready to land.Feb 21 2020, 10:43 AM
This revision was automatically updated to reflect the committed changes.