Page MenuHomePhabricator

[GlobalISel][AArch64] Add selection support for G_EXTRACT_VECTOR_ELT with FPR dest
ClosedPublic

Authored by paquette on Feb 20 2019, 11:49 AM.

Details

Summary

This adds support for G_EXTRACT_ELT in AArch64 when we have something like

%1:fpr(s64) = G_EXTRACT_VECTOR_ELT %0(<2 x s64>), %3(s64)

This also only handles cases where the index is generated by a G_CONSTANT.

It also factors out the lane copy opcode selection part into its own function, getLaneCopyOpcode which is used by both AArch64InstructionSelector::selectUnmergeValues and AArch64InstructionSelector::selectExtractElt.

There's some more refactoring that can be done between those two functions, but for the purposes of keeping the patch somewhat small, I just stuck with that one part. (For example, the IMPLICIT_DEF/INSERT_SUBREG part is basically the same.)

Diff Detail

Repository
rL LLVM

Event Timeline

paquette created this revision.Feb 20 2019, 11:49 AM

Apart from the aforementioned refactorings which can be done later, using MachineIRBuilder would also simplify some the code.

lib/Target/AArch64/AArch64InstructionSelector.cpp
1876 ↗(On Diff #187639)

Might need to check here for WideTy == 64 bits. Off the top of my head I'm not sure if we can have extracts from smaller vectors like <4 x s8>. If so, adding support for that would also mean we need to check the insert_subreg uses the right subreg index.

paquette marked an inline comment as done.Feb 22 2019, 12:56 PM
paquette added inline comments.
lib/Target/AArch64/AArch64InstructionSelector.cpp
1876 ↗(On Diff #187639)

I guess we should use getSubRegForClass to determine the subregister no matter what.

And, I'm not sure about <4 x s8>, but I found a couple tests earlier that used <2 x s16>, so I guess we'll have to handle smaller things regardless.

paquette updated this revision to Diff 187999.Feb 22 2019, 3:07 PM

Fix subregister insert bug.

Also enforce that all registers are always FPRs for G_EXTRACT_VECTOR_ELT in AArch64RegisterBankInfo.

Please add a test for regbank select.

paquette updated this revision to Diff 188466.Feb 26 2019, 2:56 PM
  • Add test for regbank select
  • Modify regbank selection to put the last operand (the index) in a GPR
  • Fix bug in subregister choice, which was looking at the destination (element) type rather than the source vector type
aemerson accepted this revision.Mar 4 2019, 11:19 AM

LGTM with small change.

lib/Target/AArch64/AArch64InstructionSelector.cpp
1890 ↗(On Diff #188466)

We should be able to replace this with emitScalarToVector().

This revision is now accepted and ready to land.Mar 4 2019, 11:19 AM
paquette updated this revision to Diff 189208.Mar 4 2019, 2:30 PM

Updating diff to address comment before committing

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 4 2019, 2:37 PM