Page MenuHomePhabricator

[AArch64][GlobalISel] Regbankselect + select @llvm.aarch64.neon.uaddlv
ClosedPublic

Authored by paquette on Apr 13 2021, 10:43 AM.

Details

Summary

It turns out we actually import a bunch of selection code for intrinsics. The imported code checks that the register banks on the G_INTRINSIC instruction are correct. If so, it goes ahead and selects it.

This adds code to AArch64RegisterBankInfo to allow us to correctly determine register banks on intrinsics which have known register bank constraints.

For now, this only handles @llvm.aarch64.neon.uaddlv. This is necessary for porting AArch64TargetLowering::LowerCTPOP.

Also add a utility for getting the intrinsic ID from a G_INTRINSIC instruction. This seems a little nicer than having to know about how intrinsic instructions are structured.

Diff Detail

Event Timeline

paquette created this revision.Apr 13 2021, 10:43 AM
paquette requested review of this revision.Apr 13 2021, 10:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 13 2021, 10:43 AM
aemerson added inline comments.Apr 14 2021, 2:12 PM
llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
477

Can we look up this information using the type signature of the intrinsic instead of hard coding it?

paquette added inline comments.Apr 15 2021, 2:02 PM
llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
477

Maybe I'm looking in the wrong place, but it looks like probably not:

class AdvSIMD_1VectorArg_Int_Across_Intrinsic
  : DefaultAttrsIntrinsic<[llvm_anyint_ty], [llvm_anyvector_ty], [IntrNoMem]>;
...
def int_aarch64_neon_uaddlv : AdvSIMD_1VectorArg_Int_Across_Intrinsic;
aemerson added inline comments.Apr 15 2021, 4:43 PM
llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
477

In that case, this intrinsic could define a GPR right? The comment for this function says it's true if the ID only uses and defines FPRs.

paquette added inline comments.Apr 15 2021, 4:54 PM
llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
477

I don't think it's supposed to.

There are 0 testcases which have a W or X register as the definition for a uaddlv.

AFAIK from the documentation for uaddlv, it always ends up on a FPR: https://developer.arm.com/documentation/ddi0596/2020-12/SIMD-FP-Instructions/UADDLV--Unsigned-sum-Long-across-Vector-

It defines an integer value though.

aemerson accepted this revision.Apr 15 2021, 11:09 PM

LGTM.

llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
477

Right, so this function is used to determine which bank to put them on, so we can hard code in the fact that it should be assigned to FPR despite the value type.

This revision is now accepted and ready to land.Apr 15 2021, 11:09 PM