[PATCH] D153394: [AArch64][GlobalISel] Selection support for v2s16 G_ANYEXT

[Allen zhong via Phabricator via llvm-commits]

Allen marked 5 inline comments as done.
Allen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp:3212
+        !MRI.getType(DstReg).isVector()) {
       LLVM_DEBUG(dbgs() << "G_ANYEXT on bank: " << RBDst
                         << ", expected: GPR\n");
----------------
paquette wrote:
> Might be good to update this comment
> 
> Scalar G_ANYEXT on bank...
Done, thanks


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp:5634
+    return nullptr;
+  ExtI.eraseFromParent();
+  return CVec;
----------------
paquette wrote:
> emitConstantVector should return a nullptr on failure, right?
> 
> So then we can save one LOC:
> 
> ```
> // Try to replace ExtI with a constant vector.
> MachineInstr *MaybeCVec =
>       emitConstantVector(ExtI.getOperand(0).getReg(), CV, MIB, MRI);
> if (MaybeCVec)
>   ExtI.eraseFromParent();
> return MaybeCVec;
> ```
Thanks, apply your comment


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp:3230
     if (DstSize > 32) {
       Register ExtSrc = MRI.createVirtualRegister(&AArch64::GPR64allRegClass);
       BuildMI(MBB, I, I.getDebugLoc(), TII.get(AArch64::SUBREG_TO_REG))
----------------
dmgreen wrote:
> I don't think this will handle vector extends correctly.
Thanks, add a new function tryToFoldExtendOfVectorConstant to handle this case.


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

https://reviews.llvm.org/D153394