[GlobalISel] Allow MachineIRBuilder to build subregister copies

[aemerson]

Summary

This relaxes some asserts about sizes, and adds an optional subreg parameter to buildCopy().

Also update AArch64 instruction selector to use this in places where we previously used MachineInstrBuilder manually.

[paquette]

LGTM

llvm/trunk/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h

public:
 
   /// Build and insert \p Res = COPY Op
   ///
   /// Register-to-register COPY sets \p Res to \p Op.
   ///
   /// \pre setBasicBlock or setMI must have been called.
   ///
   /// \return a MachineInstrBuilder for the newly created instruction.
-  MachineInstrBuilder buildCopy(const DstOp &Res, const SrcOp &Op);
+  MachineInstrBuilder buildCopy(const DstOp &Res, const SrcOp &Op,
+                                unsigned Subreg = 0);
 
   /// Build and insert `Res = G_LOAD Addr, MMO`.
   ///
   /// Loads the value stored at \p Addr. Puts the result in \p Res.
   ///
   /// \pre setBasicBlock or setMI must have been called.
   /// \pre \p Res must be a generic virtual register.
   /// \pre \p Addr must be a generic virtual register with pointer type.

llvm/trunk/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp

 }
 
 MachineInstrBuilder MachineIRBuilder::buildBrIndirect(unsigned Tgt) {
   assert(getMRI()->getType(Tgt).isPointer() && "invalid branch destination");
   return buildInstr(TargetOpcode::G_BRINDIRECT).addUse(Tgt);
 }
 
 MachineInstrBuilder MachineIRBuilder::buildCopy(const DstOp &Res,
-                                                const SrcOp &Op) {
-  return buildInstr(TargetOpcode::COPY, Res, Op);
+                                                const SrcOp &Op,
+                                                unsigned Subreg) {
+  auto Copy = buildInstr(TargetOpcode::COPY, Res, Op);
+  Copy->getOperand(1).setSubReg(Subreg);
+  return Copy;
 }
 
 MachineInstrBuilder MachineIRBuilder::buildConstant(const DstOp &Res,
                                                     const ConstantInt &Val) {
   LLT Ty = Res.getLLTTy(*getMRI());
   LLT EltTy = Ty.getScalarType();
   assert(EltTy.getScalarSizeInBits() == Val.getBitWidth() &&
          "creating constant with the wrong size");
case TargetOpcode::G_FPTRUNC: {
     assert(SrcOps.size() == 1 && "Invalid Srcs");
     validateTruncExt(DstOps[0].getLLTTy(*getMRI()),
                      SrcOps[0].getLLTTy(*getMRI()), false);
     break;
   }
   case TargetOpcode::COPY:
     assert(DstOps.size() == 1 && "Invalid Dst");
     assert(SrcOps.size() == 1 && "Invalid Srcs");
-    assert(DstOps[0].getLLTTy(*getMRI()) == LLT() ||
-           SrcOps[0].getLLTTy(*getMRI()) == LLT() ||
-           DstOps[0].getLLTTy(*getMRI()) == SrcOps[0].getLLTTy(*getMRI()));
     break;
   case TargetOpcode::G_FCMP:
   case TargetOpcode::G_ICMP: {
     assert(DstOps.size() == 1 && "Invalid Dst Operands");
     assert(SrcOps.size() == 3 && "Invalid Src Operands");
     // For F/ICMP, the first src operand is the predicate, followed by
     // the two comparands.
     assert(SrcOps[0].getSrcOpKind() == SrcOp::SrcType::Ty_Predicate &&

llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp

 /// Helper function for selectCopy. Inserts a subregister copy from
 /// \p *From to \p *To, linking it up to \p I.
 ///
 /// e.g, given I = "Dst = COPY SrcReg", we'll transform that into
 ///
 /// CopyReg (From class) = COPY SrcReg
 /// SubRegCopy (To class) = COPY CopyReg:SubReg
 /// Dst = COPY SubRegCopy
-static bool selectSubregisterCopy(MachineInstr &I, const TargetInstrInfo &TII,
-                                  MachineRegisterInfo &MRI,
+static bool selectSubregisterCopy(MachineInstr &I, MachineRegisterInfo &MRI,
                                   const RegisterBankInfo &RBI, unsigned SrcReg,
                                   const TargetRegisterClass *From,
                                   const TargetRegisterClass *To,
                                   unsigned SubReg) {
-  unsigned CopyReg = MRI.createVirtualRegister(From);
-  BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(AArch64::COPY), CopyReg)
-      .addUse(SrcReg);
-  unsigned SubRegCopy = MRI.createVirtualRegister(To);
-  BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(TargetOpcode::COPY),
-          SubRegCopy)
-      .addUse(CopyReg, 0, SubReg);
+  MachineIRBuilder MIB(I);
+  auto Copy = MIB.buildCopy({From}, {SrcReg});
+  auto SubRegCopy = MIB.buildCopy({To}, {Copy}, SubReg);
   MachineOperand &RegOp = I.getOperand(1);
-  RegOp.setReg(SubRegCopy);
+  RegOp.setReg(SubRegCopy.getReg(0));
 
   // It's possible that the destination register won't be constrained. Make
   // sure that happens.
   if (!TargetRegisterInfo::isPhysicalRegister(I.getOperand(0).getReg()))
     RBI.constrainGenericRegister(I.getOperand(0).getReg(), *To, MRI);
 
   return true;
 }
if (DstRegBank.getID() != SrcRegBank.getID()) {
         // Get the appropriate subregister for the destination.
         unsigned SubReg = 0;
         if (!getSubRegForClass(DstRC, TRI, SubReg)) {
           LLVM_DEBUG(dbgs() << "Couldn't determine subregister for copy.\n");
           return false;
         }
 
         // Now, insert a subregister copy using the new register class.
-        selectSubregisterCopy(I, TII, MRI, RBI, SrcReg, SubregRC, DstRC,
-                              SubReg);
+        selectSubregisterCopy(I, MRI, RBI, SrcReg, SubregRC, DstRC, SubReg);
         return CheckCopy();
       }
 
       else if (DstRegBank.getID() == AArch64::GPRRegBankID && DstSize == 32 &&
                SrcSize == 16) {
         // Special case for FPR16 to GPR32.
         // FIXME: This can probably be generalized like the above case.
         unsigned PromoteReg =
bool AArch64InstructionSelector::select(MachineInstr &I,
   }
 
   if (selectImpl(I, CoverageInfo))
     return true;
 
   LLT Ty =
       I.getOperand(0).isReg() ? MRI.getType(I.getOperand(0).getReg()) : LLT{};
 
+  MachineIRBuilder MIB(I);
+
   switch (Opcode) {
   case TargetOpcode::G_BRCOND: {
     if (Ty.getSizeInBits() > 32) {
       // We shouldn't need this on AArch64, but it would be implemented as an
       // EXTRACT_SUBREG followed by a TBNZW because TBNZX has no encoding if the
       // bit being tested is < 32.
       LLVM_DEBUG(dbgs() << "G_BRCOND has type: " << Ty
                         << ", expected at most 32-bits");
if (isFP) {
       const TargetRegisterClass &GPRRC =
           DefSize == 32 ? AArch64::GPR32RegClass : AArch64::GPR64RegClass;
       const TargetRegisterClass &FPRRC =
           DefSize == 32 ? AArch64::FPR32RegClass : AArch64::FPR64RegClass;
 
       const unsigned DefGPRReg = MRI.createVirtualRegister(&GPRRC);
       MachineOperand &RegOp = I.getOperand(0);
       RegOp.setReg(DefGPRReg);
-
-      BuildMI(MBB, std::next(I.getIterator()), I.getDebugLoc(),
-              TII.get(AArch64::COPY))
-          .addDef(DefReg)
-          .addUse(DefGPRReg);
+      MIB.setInsertPt(MIB.getMBB(), std::next(I.getIterator()));
+      MIB.buildCopy({DefReg}, {DefGPRReg});
 
       if (!RBI.constrainGenericRegister(DefReg, FPRRC, MRI)) {
         LLVM_DEBUG(dbgs() << "Failed to constrain G_FCONSTANT def operand\n");
         return false;
       }
 
       MachineOperand &ImmOp = I.getOperand(1);
       // FIXME: Is going through int64_t always correct?
case TargetOpcode::G_EXTRACT: {
 
     if (SrcSize < 64) {
       assert(SrcSize == 32 && DstTy.getSizeInBits() == 16 &&
              "unexpected G_EXTRACT types");
       return constrainSelectedInstRegOperands(I, TII, TRI, RBI);
     }
 
     unsigned DstReg = MRI.createGenericVirtualRegister(LLT::scalar(64));
-    BuildMI(MBB, std::next(I.getIterator()), I.getDebugLoc(),
-            TII.get(AArch64::COPY))
-        .addDef(I.getOperand(0).getReg())
-        .addUse(DstReg, 0, AArch64::sub_32);
+    MIB.setInsertPt(MIB.getMBB(), std::next(I.getIterator()));
+    MIB.buildCopy({I.getOperand(0).getReg()}, {DstReg}, AArch64::sub_32);
     RBI.constrainGenericRegister(I.getOperand(0).getReg(),
                                  AArch64::GPR32RegClass, MRI);
     I.getOperand(0).setReg(DstReg);
 
     return constrainSelectedInstRegOperands(I, TII, TRI, RBI);
   }
 
   case TargetOpcode::G_INSERT: {
MachineInstr *AArch64InstructionSelector::emitExtractVectorElt(
   }
 
   // The register that we're going to copy into.
   unsigned InsertReg = VecReg;
   if (!DstReg)
     DstReg = MRI.createVirtualRegister(DstRC);
   // If the lane index is 0, we just use a subregister COPY.
   if (LaneIdx == 0) {
-    auto CopyMI =
-        BuildMI(MIRBuilder.getMBB(), MIRBuilder.getInsertPt(),
-                MIRBuilder.getDL(), TII.get(TargetOpcode::COPY), *DstReg)
-            .addUse(VecReg, 0, ExtractSubReg);
+    auto Copy = MIRBuilder.buildCopy({*DstReg}, {VecReg}, ExtractSubReg);
     RBI.constrainGenericRegister(*DstReg, *DstRC, MRI);
-    return &*CopyMI;
+    return &*Copy;
   }
 
   // Lane copies require 128-bit wide registers. If we're dealing with an
   // unpacked vector, then we need to move up to that width. Insert an implicit
   // def and a subregister insert to get us there.
   if (VecTy.getSizeInBits() != 128) {
     MachineInstr *ScalarToVector = emitScalarToVector(
         VecTy.getSizeInBits(), &AArch64::FPR128RegClass, VecReg, MIRBuilder);
bool AArch64InstructionSelector::selectUnmergeValues(
   (void)WideTy;
   assert(WideTy.isVector() && "can only unmerge from vector types!");
   assert(WideTy.getSizeInBits() > NarrowTy.getSizeInBits() &&
          "source register size too small!");
 
   if (!NarrowTy.isScalar())
     return selectSplitVectorUnmerge(I, MRI);
 
+  MachineIRBuilder MIB(I);
+
   // Choose a lane copy opcode and subregister based off of the size of the
   // vector's elements.
   unsigned CopyOpc = 0;
   unsigned ExtractSubReg = 0;
   if (!getLaneCopyOpcode(CopyOpc, ExtractSubReg, NarrowTy.getSizeInBits()))
     return false;
 
   // Set up for the lane copies.
if (NarrowTy.getSizeInBits() * NumElts == 128) {
     }
   }
 
   // Now that we've created any necessary subregister inserts, we can
   // create the copies.
   //
   // Perform the first copy separately as a subregister copy.
   unsigned CopyTo = I.getOperand(0).getReg();
-  MachineInstr &FirstCopy =
-      *BuildMI(MBB, I, I.getDebugLoc(), TII.get(TargetOpcode::COPY), CopyTo)
-           .addUse(InsertRegs[0], 0, ExtractSubReg);
-  constrainSelectedInstRegOperands(FirstCopy, TII, TRI, RBI);
+  auto FirstCopy = MIB.buildCopy({CopyTo}, {InsertRegs[0]}, ExtractSubReg);
+  constrainSelectedInstRegOperands(*FirstCopy, TII, TRI, RBI);
 
   // Now, perform the remaining copies as vector lane copies.
   unsigned LaneIdx = 1;
   for (unsigned InsReg : InsertRegs) {
     unsigned CopyTo = I.getOperand(LaneIdx).getReg();
     MachineInstr &CopyInst =
         *BuildMI(MBB, I, I.getDebugLoc(), TII.get(CopyOpc), CopyTo)
              .addUse(InsReg)
IndexLoad =
         emitScalarToVector(64, &AArch64::FPR128RegClass,
                            IndexLoad->getOperand(0).getReg(), MIRBuilder);
 
     auto TBL1 = MIRBuilder.buildInstr(
         AArch64::TBLv16i8One, {&AArch64::FPR128RegClass},
         {Concat->getOperand(0).getReg(), IndexLoad->getOperand(0).getReg()});
     constrainSelectedInstRegOperands(*TBL1, TII, TRI, RBI);
 
-    auto Copy = BuildMI(*I.getParent(), I, I.getDebugLoc(),
-                        TII.get(TargetOpcode::COPY), I.getOperand(0).getReg())
-                    .addUse(TBL1->getOperand(0).getReg(), 0, AArch64::dsub);
+    auto Copy =
+        MIRBuilder.buildCopy({I.getOperand(0).getReg()}, {TBL1}, AArch64::dsub);
     RBI.constrainGenericRegister(Copy.getReg(0), AArch64::FPR64RegClass, MRI);
     I.eraseFromParent();
     return true;
   }
 
   // For TBL2 we need to emit a REG_SEQUENCE to tie together two consecutive
   // Q registers for regalloc.
   auto RegSeq = MIRBuilder
if (SubReg != AArch64::ssub && SubReg != AArch64::dsub) {
       LLVM_DEBUG(dbgs() << "Unsupported destination size! (" << DstSize
                         << "\n");
       return false;
     }
 
     unsigned Reg = MRI.createVirtualRegister(RC);
     unsigned DstReg = I.getOperand(0).getReg();
 
-    // MIRBuilder doesn't let us create uses with subregs & flags, so use
-    // BuildMI here instead.
-    BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(TargetOpcode::COPY),
-            DstReg)
-        .addUse(DstVec, 0, SubReg);
+    MIRBuilder.buildCopy({DstReg}, {DstVec}, SubReg);
     MachineOperand &RegOp = I.getOperand(1);
     RegOp.setReg(Reg);
     RBI.constrainGenericRegister(DstReg, *RC, MRI);
   } else {
     // We don't need a subregister copy. Save a copy by re-using the
     // destination register on the final insert.
     assert(PrevMI && "PrevMI was null?");
     PrevMI->getOperand(0).setReg(I.getOperand(0).getReg());