[PATCH] D106388: [AArch64][GlobalISel] Legalize ctpop for v2s64, v2s32, v4s32, v4s16, v8s16

[Jon Roelofs via Phabricator via llvm-commits]

jroelofs added inline comments.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp:1184
+    UADD = MIRBuilder.buildInstr(Opc, {HTy}, {HSum});
+    constrainSelectedInstRegOperands(*UADD, *ST->getInstrInfo(),
+                                     *MRI.getTargetRegisterInfo(),
----------------
paquette wrote:
> jroelofs wrote:
> > paquette wrote:
> > > I don't think you should have to do this here, since we're before register bank selection?
> > MachineVerifier seems to expect them just after legalization:
> > 
> > ```
> > *** Bad machine code: Virtual register does not match instruction constraint ***
> > - function:    s32_lower
> > - basic block: %bb.0  (0x7fba998570c8)
> > - instruction: %ctpop:_(s32) = UADDLVv8i8v %4:_(<8 x s8>)
> > - operand 0:   %ctpop:_
> > Expect register class FPR16 but got nothing
> > ```
> ah, yeah I guess it's cause it's not a generic instr. :/
> 
> Maybe it would make sense to add a G_UADDLV generic instruction to avoid emitting target instrs.
> 
> IIRC we import tablegen patterns for the uaddlv //intrinsic// allowing us to select it easily. Maybe if we had a generic instruction, we could do something similar.
maybe I ought to just use the intrinsics then


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106388/new/

https://reviews.llvm.org/D106388