[PATCH] D59322: [GlobalISel][AArch64] Gardening: Factor out vector inserts

[Jessica Paquette via Phabricator via llvm-commits]

paquette updated this revision to Diff 190512.
paquette added a comment.

Addressed review comments


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

https://reviews.llvm.org/D59322

Files:
  llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp


Index: llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp
===================================================================
--- llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp
+++ llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp
@@ -71,6 +71,10 @@
                                    const TargetRegisterClass *DstRC,
                                    unsigned Scalar,
                                    MachineIRBuilder &MIRBuilder) const;
+  MachineInstr *emitLaneInsert(unsigned LaneIdx, unsigned EltSize,
+                               unsigned DstVec, unsigned InsReg,
+                               const RegisterBank &RB,
+                               MachineIRBuilder &MIRBuilder) const;
   bool selectBuildVector(MachineInstr &I, MachineRegisterInfo &MRI) const;
   bool selectMergeValues(MachineInstr &I, MachineRegisterInfo &MRI) const;
   bool selectUnmergeValues(MachineInstr &I, MachineRegisterInfo &MRI) const;
@@ -2304,6 +2308,29 @@
   return true;
 }
 
+MachineInstr *AArch64InstructionSelector::emitLaneInsert(
+    unsigned LaneIdx, unsigned EltSize, unsigned DstVec, unsigned InsReg,
+    const RegisterBank &RB, MachineIRBuilder &MIRBuilder) const {
+  MachineInstr *InsElt = nullptr;
+  const TargetRegisterClass *DstRC = &AArch64::FPR128RegClass;
+  unsigned InsDef = MIRBuilder.getMRI()->createVirtualRegister(DstRC);
+  unsigned Opc = getInsertVecEltOpInfo(RB, EltSize).first;
+  if (RB.getID() == AArch64::FPRRegBankID) {
+    auto InsSub = emitScalarToVector(EltSize, DstRC, InsReg, MIRBuilder);
+    InsElt = MIRBuilder.buildInstr(Opc, {InsDef}, {DstVec})
+                 .addImm(LaneIdx)
+                 .addUse(InsSub->getOperand(0).getReg())
+                 .addImm(0);
+  } else {
+    InsElt = MIRBuilder.buildInstr(Opc, {InsDef}, {DstVec})
+                 .addImm(LaneIdx)
+                 .addUse(InsReg);
+  }
+
+  constrainSelectedInstRegOperands(*InsElt, TII, TRI, RBI);
+  return InsElt;
+}
+
 bool AArch64InstructionSelector::selectBuildVector(
     MachineInstr &I, MachineRegisterInfo &MRI) const {
   assert(I.getOpcode() == TargetOpcode::G_BUILD_VECTOR);
@@ -2315,11 +2342,6 @@
   if (EltSize < 16 || EltSize > 64)
     return false; // Don't support all element types yet.
   const RegisterBank &RB = *RBI.getRegBank(I.getOperand(1).getReg(), MRI, TRI);
-  unsigned Opc;
-  unsigned SubregIdx;
-
-  std::tie(Opc, SubregIdx) = getInsertVecEltOpInfo(RB, EltSize);
-
   MachineIRBuilder MIRBuilder(I);
 
   const TargetRegisterClass *DstRC = &AArch64::FPR128RegClass;
@@ -2336,34 +2358,11 @@
   // a copy using it.
   MachineInstr *PrevMI = nullptr;
   for (unsigned i = 2, e = DstSize / EltSize + 1; i < e; ++i) {
-    // Note that if we don't do a subregister copy, we end up making one more
-    // of these than we need.
-    unsigned InsDef = MRI.createVirtualRegister(DstRC);
-    unsigned LaneIdx = i - 1;
-    if (RB.getID() == AArch64::FPRRegBankID) {
-      auto ImpDef =
-          MIRBuilder.buildInstr(TargetOpcode::IMPLICIT_DEF, {DstRC}, {});
-      auto InsSub = MIRBuilder
-                        .buildInstr(TargetOpcode::INSERT_SUBREG, {DstRC},
-                                    {ImpDef, I.getOperand(i)})
-                        .addImm(SubregIdx);
-      auto InsElt = MIRBuilder.buildInstr(Opc, {InsDef}, {DstVec})
-                        .addImm(LaneIdx)
-                        .addUse(InsSub.getReg(0))
-                        .addImm(0);
-      constrainSelectedInstRegOperands(*ImpDef, TII, TRI, RBI);
-      constrainSelectedInstRegOperands(*InsSub, TII, TRI, RBI);
-      constrainSelectedInstRegOperands(*InsElt, TII, TRI, RBI);
-      DstVec = InsDef;
-      PrevMI = &*InsElt;
-    } else {
-      auto Ins = MIRBuilder.buildInstr(Opc, {InsDef}, {DstVec})
-                     .addImm(LaneIdx)
-                     .addUse(I.getOperand(i).getReg());
-      constrainSelectedInstRegOperands(*Ins, TII, TRI, RBI);
-      DstVec = InsDef;
-      PrevMI = &*Ins;
-    }
+    // Note that if we don't do a subregister copy, we can end up making an
+    // extra register.
+    PrevMI = &*emitLaneInsert(i - 1, EltSize, DstVec, I.getOperand(i).getReg(),
+                              RB, MIRBuilder);
+    DstVec = PrevMI->getOperand(0).getReg();
   }
 
   // If DstTy's size in bits is less than 128, then emit a subregister copy


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59322.190512.patch
Type: text/x-patch
Size: 4344 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190313/7b261f54/attachment-0001.bin>