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

[Jessica Paquette via Phabricator via llvm-commits]

paquette added a comment.

Can you add a MIR testcase?



================
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");
----------------
Might be good to update this comment

Scalar G_ANYEXT on bank...


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp:5608
+  assert(DstSize <= 128 && "Unexpected build_vec type!");
+  if (DstSize < 32)
+    return nullptr;
----------------
Comment?


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp:5615
+  unsigned OpTySize = DstTy.getScalarSizeInBits();
+  for (unsigned Idx = 1; Idx < SrcMI->getNumOperands(); ++Idx) {
+    // Try to find G_CONSTANT
----------------
Could use a comment explaining that you're looking for G_BUILD_VECTORs with all constant source operands?


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp:5634
+    return nullptr;
+  ExtI.eraseFromParent();
+  return CVec;
----------------
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;
```


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

https://reviews.llvm.org/D153394