[GlobalISel] Add missing operand update when copy is required

[ehjogab]

Summary

When constraining an operand register using constrainOperandRegClass(),
the function may emit a COPY in case the provided register class does
not match the current operand register class. However, the operand
itself is not updated to make use of the COPY, thereby resulting in
incorrect code. This patch fixes that bug by updating the machine
operand accordingly.

[arsenm]

Is a test possible?

llvm/lib/CodeGen/GlobalISel/InstructionSelector.cpp

38

Does this need to call the change observer? Is a test possible?

[ehjogab]

llvm/lib/CodeGen/GlobalISel/InstructionSelector.cpp

38

I'm not familiar with the change observer, so I don't know whether it needs to be called. If you know, can you please inform me on how the change observer works and is intended to be used?

The test case requires a target where you have a register bank that can map to at least two register classes, and I haven't found any target with that. Due to how target is currently implemented, we use a single register bank as an intermediary step in porting our own target to global-isel, and that's when I stumbled upon this bug.

To convince oneself that this is a bug, look at how constrainOperandRegToRegClass() is called. There is only one place, in the implementation of GIR_ConstrainOperandRC in llvm/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h. There, it does nothing to update the instruction in case a copy is emitted. Also, the documentation for constrainOperandRegToRegClass() says the following (see bold):

/// Constrain a register operand of an instruction \p I to a specified 
/// register class. This could involve inserting COPYs before (for uses) or
/// after (for defs) and **may replace the operand of \p I.**
/// \returns whether operand regclass constraining succeeded.

[ehjogab]

Bump

[arsenm]

llvm/lib/CodeGen/GlobalISel/InstructionSelector.cpp

38

Looking at how constrainOperandRegClass is implemented, I think the intent was it would handle all of the instruction modifications. It should be responsible for updating the operand, not the caller here (actually InstructionSelector::constrainOperandRegToRegClass seems like a fairly pointless wrapper function)

[ehjogab]

llvm/lib/CodeGen/GlobalISel/InstructionSelector.cpp

38

I find three places where constrainOperandRegClass() is called and its result is used to update the operand. So I disagree with the original intent being that constrainOperandRegClass() should also update the operand. There is also a constrainOperandRegClass() for SelectionISel and there the result is sometimes used to update the operand, so I believe that the behavior for GlobalISel was simply copied from how SelectionISel does it.

But I also agree that constrainOperandRegToRegClass() is quite useless, and since all cases where constrainOperandRegClass() is invoked the result is used to update the operand, maybe it is a better idea to let constrainOperandRegClass() do it for us. I will update the patch accordingly.

[ehjogab]

Machine operand is now updated in constrainOperandRegClass()

[arsenm]

llvm/lib/CodeGen/GlobalISel/Utils.cpp

80–88

I think I misread this the first time through and thought this was replacing uses. However I'm still a bit confused.

This calls changingAllUsesOfReg after the potential change from constrainRegToClass (this will also happen even if constrainRegToClass ends up being a no-op when the register already was a compatible class). I'm further confused since the existing cases get away without calling the observer. I'm guessing this is because this is already called in contexts changing the instruction, or not bothering with the observer? If we don't need to call the observer here, I would prefer the first version but I'm unclear on whether it's really needed here at all

[ehjogab]

llvm/lib/CodeGen/GlobalISel/Utils.cpp

80–88

This functions may replace both uses as well as defs, and the bug this patch was originally intended to fix concerned lack of update for defs.

I agree that things are not clear with the observer is intended to be called and when it is strictly necessary to do so. It would be good to have someone with such knowledge to have a look at this patch.

[ehjogab]

@aditya_nandakumar Can you please take a look at this patch? We are unsure on whether the observer is used correctly here.

[dsanders]

llvm/include/llvm/CodeGen/GlobalISel/Utils.h

72–74

Just to mention it, there may be some consequences to adding 'or after' to this. I don't know if it's still the case or if we fixed them all but some passes used to be a bit picky on exactly where it was safe to insert instructions around the 'current' instruction and would fail to act on instructions or crash due to invalidated iterators or newly freed instructions.

llvm/lib/CodeGen/GlobalISel/InstructionSelector.cpp

38

I'm not familiar with the change observer, so I don't know whether it needs to be called. If you know, can you please inform me on how the change observer works and is intended to be used?

If you change the MIR (or are about to in some cases like erasingInstr()), you need to tell the observers. The observer is a way for something (typically the core loop of a pass) to keep track of what's going on so it can maintain it's data structures (e.g. avoid visiting deleted instructions), or report/analyze it. Other observers analyze lost DebugLoc's or simply print debugging output

Some observers can be fairly tolerant to failing to call them (especially the changingInstr/changedInstr functions as they were added later) but it's still a bug not to.

To convince oneself that this is a bug, look at how constrainOperandRegToRegClass() is called. There is only one
place, in the implementation of GIR_ConstrainOperandRC in
llvm/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h. There, it does nothing to update the instruction in
case a copy is emitted.

FWIW, it doesn't necessarily need to update the instruction immediately here but it's not doing the other method either. The other method is to have a subsequent GIR_* opcode that updates the uses but that would require us to record the result somewhere and we don't. I think the right method is to update as part of the GIR_ConstrainOperandRC as the other one will bloat the state machine a lot to handle a fairly rare case.

actually InstructionSelector::constrainOperandRegToRegClass seems like a fairly pointless wrapper function

Yep, it looks like it's no longer needed. A couple years back, tablegen emitted C++ directly and it was providing a lot of value for compile-time of the compiler itself. Eventually the compile-time of the generated code got excessive and we implemented the state machine

I find three places where constrainOperandRegClass() is called and its result is used to update the operand. So I disagree with the original intent being that constrainOperandRegClass() should also update the operand.

It's a bit of both really. The intent is for the caller to update the instruction it was working on and for the callee to update any other instructions that needed to be changed because of the constraint. The reason for the division is that the caller might be making other changes too that it either already has or is about to report. It's a bug to call changedInstr() and then make further changes without a new changingInstr() and it's undesirable to over-report changes by calling changingInstr()/changedInstr() multiple times for one change.

llvm/lib/CodeGen/GlobalISel/Utils.cpp

80–88

I've commented on this above (which I think is the same thread and phabricator has just failed to track it properly)

[ehjogab]

llvm/lib/CodeGen/GlobalISel/InstructionSelector.cpp

38

Thanks for your detailed reply, Daniel!

If you change the MIR (or are about to in some cases like erasingInstr()), you need to tell the observers. The observer is a way for something (typically the core loop of a pass) to keep track of what's going on so it can maintain it's data structures (e.g. avoid visiting deleted instructions), or report/analyze it. Other observers analyze lost DebugLoc's or simply print debugging output

Some observers can be fairly tolerant to failing to call them (especially the changingInstr/changedInstr functions as they were added later) but it's still a bug not to.

Okay, then I think I got the hang of it.

FWIW, it doesn't necessarily need to update the instruction immediately here but it's not doing the other method either. The other method is to have a subsequent GIR_* opcode that updates the uses but that would require us to record the result somewhere and we don't. I think the right method is to update as part of the GIR_ConstrainOperandRC as the other one will bloat the state machine a lot to handle a fairly rare case.

I think having a subsequent GIR_* opcode to do the updates is overkill, and would indeed needlessly bloat the state machine.

It's a bit of both really. The intent is for the caller to update the instruction it was working on and for the callee to update any other instructions that needed to be changed because of the constraint. The reason for the division is that the caller might be making other changes too that it either already has or is about to report. It's a bug to call changedInstr() and then make further changes without a new changingInstr() and it's undesirable to over-report changes by calling changingInstr()/changedInstr() multiple times for one change.

In light of this comment, then the correct way of handling this would be for constrainOperandRegClass() to insert the COPYs but not update the instruction itself (i.e. as it was before). I have, however, looked at the places where constrainOperandRegClass() is called and in none do they themselves invoke the observer, so it would be "okay" to move the operand update into constrainOperandRegClass() (i.e. as it is now in the patch). Also, it is more convenient to let constrainOperandRegClass() do the operand update since it must be done anyhow.

llvm/lib/CodeGen/GlobalISel/Utils.cpp

80–88

This calls changingAllUsesOfReg after the potential change from constrainRegToClass (this will also happen even if constrainRegToClass ends up being a no-op when the register already was a compatible class). I'm further confused since the existing cases get away without calling the observer. I'm guessing this is because this is already called in contexts changing the instruction, or not bothering with the observer? If we don't need to call the observer here, I would prefer the first version but I'm unclear on whether it's really needed here at all

I took a closer look and I believe that as it is now is correct:

1. If the class is compatible, then we have only updated the register class. If the register is a use, then it means that we have also incurred a change that affects the instruction defining that register, and hence need to invoke changedInstr(); this we do. Lastly, we trigger changingAllUsesOfReg() to indicate a change that affects all uses of a register.
2. If the class is not compatible, then the register is left unaffected and instead we introduce a new register along with a COPY. Independent of whether the register is a use or def, we affect no other instruction than the instruction that the operand belongs to. Hence we only need to invoke changedInstr().

The only potential bug here is that we invoke both changedInstr() and changingAllUsesOfReg() when the register is already in the correct class, and hence there is no update. I don't know whether that's a common case, though...

[ehjogab]

Any feedback on my latest comments?
Should the patch be accepted as is, or does it require more changes?

[dsanders]

LGTM with the changingInstr() to match the changedInstr() on the setReg() path.

We ought to address the other issues too but they're pre-existing bugs so I don't want to block your patch on them. We can address them later

llvm/lib/CodeGen/GlobalISel/InstructionSelector.cpp

38

I agree that the operand update can be inside constrainOperandRegClass() but the changingInstr()/changedInstr() calls for that instruction need to either be outside or the internal calls should be optional if for when you're already in a changing/changed region.

I have, however, looked at the places where constrainOperandRegClass() is called and in none do they themselves invoke the observer

I'm surprised none of them call it but changed/changing was added much later than the others so I'm not surprised there's some omissions. At some point I need to run more passes with the lost DebugLoc observer and that will probably detect a fair few of them.

I think having a subsequent GIR_* opcode to do the updates is overkill, and would indeed needlessly bloat the state machine.

I agree

llvm/lib/CodeGen/GlobalISel/Utils.cpp

80–88

The only potential bug here is that we invoke both changedInstr() and changingAllUsesOfReg() when the register is already in the correct class, and hence there is no update. I don't know whether that's a common case, though...

We should probably fix that but I don't think we need to do it in this patch as it's not exactly wrong, it's just overreporting.

Along the same lines, this function generally seems to get changingInstr() wrong. For the new changedInstr() on the setReg() path it's missing and for the existing changingAllUsesOfReg() it's happening after the change it's reporting. It's supposed to happen before the change made by constrainRegToClass() if it opts to constrain using the same register. The setReg() path is easy to fix as we can just call changingInstr() just before the setReg() so I think we should go ahead and do that, but the changingAllUsesOfReg() is going to be much harder (although it shouldn't block your patch as it's pre-existing)

[ehjogab]

Added changingInstr() before setReg()

[uabelho]

I'll submit this for @ehjogab in a little bit.

llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h

protected:
   }
   virtual bool testMIPredicate_MI(
       unsigned, const MachineInstr &,
       const std::array<const MachineOperand *, 3> &Operands) const {
     llvm_unreachable(
         "Subclasses must override this with a tablegen-erated function");
   }
 
-  /// Constrain a register operand of an instruction \p I to a specified
-  /// register class. This could involve inserting COPYs before (for uses) or
-  /// after (for defs) and may replace the operand of \p I.
-  /// \returns whether operand regclass constraining succeeded.
-  bool constrainOperandRegToRegClass(MachineInstr &I, unsigned OpIdx,
-                                     const TargetRegisterClass &RC,
-                                     const TargetInstrInfo &TII,
-                                     const TargetRegisterInfo &TRI,
-                                     const RegisterBankInfo &RBI) const;
-
   bool isOperandImmEqual(const MachineOperand &MO, int64_t Value,
                          const MachineRegisterInfo &MRI) const;
 
   /// Return true if the specified operand is a G_PTR_ADD with a G_CONSTANT on the
   /// right-hand side. GlobalISel's separation of pointer and integer types
   /// means that we don't need to worry about G_OR with equivalent semantics.
   bool isBaseWithConstantOffset(const MachineOperand &Root,
                                 const MachineRegisterInfo &MRI) const;

llvm/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h

case GIR_CustomOperandRenderer: {
                                                      OpIdx);
       break;
     }
     case GIR_ConstrainOperandRC: {
       int64_t InsnID = MatchTable[CurrentIdx++];
       int64_t OpIdx = MatchTable[CurrentIdx++];
       int64_t RCEnum = MatchTable[CurrentIdx++];
       assert(OutMIs[InsnID] && "Attempted to add to undefined instruction");
-      constrainOperandRegToRegClass(*OutMIs[InsnID].getInstr(), OpIdx,
-                                    *TRI.getRegClass(RCEnum), TII, TRI, RBI);
+      MachineInstr &I = *OutMIs[InsnID].getInstr();
+      MachineFunction &MF = *I.getParent()->getParent();
+      MachineRegisterInfo &MRI = MF.getRegInfo();
+      const TargetRegisterClass &RC = *TRI.getRegClass(RCEnum);
+      MachineOperand &MO = I.getOperand(OpIdx);
+      constrainOperandRegClass(MF, TRI, MRI, TII, RBI, I, RC, MO);
       DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
                       dbgs() << CurrentIdx << ": GIR_ConstrainOperandRC(OutMIs["
                              << InsnID << "], " << OpIdx << ", " << RCEnum
                              << ")\n");
       break;
     }
 
     case GIR_ConstrainSelectedInstOperands: {

llvm/include/llvm/CodeGen/GlobalISel/Utils.h

 /// \return The virtual register constrained to the right register class.
 Register constrainRegToClass(MachineRegisterInfo &MRI,
                              const TargetInstrInfo &TII,
                              const RegisterBankInfo &RBI, Register Reg,
                              const TargetRegisterClass &RegClass);
 
 /// Constrain the Register operand OpIdx, so that it is now constrained to the
 /// TargetRegisterClass passed as an argument (RegClass).
-/// If this fails, create a new virtual register in the correct class and
-/// insert a COPY before \p InsertPt if it is a use or after if it is a
-/// definition. The debug location of \p InsertPt is used for the new copy.
+/// If this fails, create a new virtual register in the correct class and insert
+/// a COPY before \p InsertPt if it is a use or after if it is a definition.
+/// In both cases, the function also updates the register of RegMo. The debug
+/// location of \p InsertPt is used for the new copy.
 ///
 /// \return The virtual register constrained to the right register class.
 Register constrainOperandRegClass(const MachineFunction &MF,
                                   const TargetRegisterInfo &TRI,
                                   MachineRegisterInfo &MRI,
                                   const TargetInstrInfo &TII,
                                   const RegisterBankInfo &RBI,
                                   MachineInstr &InsertPt,
                                   const TargetRegisterClass &RegClass,
-                                  const MachineOperand &RegMO);
+                                  MachineOperand &RegMO);
 
-/// Try to constrain Reg so that it is usable by argument OpIdx of the
-/// provided MCInstrDesc \p II. If this fails, create a new virtual
-/// register in the correct class and insert a COPY before \p InsertPt
-/// if it is a use or after if it is a definition.
+/// Try to constrain Reg so that it is usable by argument OpIdx of the provided
+/// MCInstrDesc \p II. If this fails, create a new virtual register in the
+/// correct class and insert a COPY before \p InsertPt if it is a use or after
+/// if it is a definition. In both cases, the function also updates the register
+/// of RegMo.
 /// This is equivalent to constrainOperandRegClass(..., RegClass, ...)
 /// with RegClass obtained from the MCInstrDesc. The debug location of \p
 /// InsertPt is used for the new copy.
 ///
 /// \return The virtual register constrained to the right register class.
 Register constrainOperandRegClass(const MachineFunction &MF,
                                   const TargetRegisterInfo &TRI,
                                   MachineRegisterInfo &MRI,
                                   const TargetInstrInfo &TII,
                                   const RegisterBankInfo &RBI,
                                   MachineInstr &InsertPt, const MCInstrDesc &II,
-                                  const MachineOperand &RegMO, unsigned OpIdx);
+                                  MachineOperand &RegMO, unsigned OpIdx);
 
 /// Mutate the newly-selected instruction \p I to constrain its (possibly
 /// generic) virtual register operands to the instruction's register class.
 /// This could involve inserting COPYs before (for uses) or after (for defs).
 /// This requires the number of operands to match the instruction description.
 /// \returns whether operand regclass constraining succeeded.
 ///
 // FIXME: Not all instructions have the same number of operands. We should

llvm/lib/CodeGen/GlobalISel/InstructionSelector.cpp

 
 using namespace llvm;
 
 InstructionSelector::MatcherState::MatcherState(unsigned MaxRenderers)
     : Renderers(MaxRenderers), MIs() {}
 
 InstructionSelector::InstructionSelector() = default;
 
-bool InstructionSelector::constrainOperandRegToRegClass(
-    MachineInstr &I, unsigned OpIdx, const TargetRegisterClass &RC,
-    const TargetInstrInfo &TII, const TargetRegisterInfo &TRI,
-    const RegisterBankInfo &RBI) const {
-  MachineBasicBlock &MBB = *I.getParent();
-  MachineFunction &MF = *MBB.getParent();
-  MachineRegisterInfo &MRI = MF.getRegInfo();
-
-  return constrainOperandRegClass(MF, TRI, MRI, TII, RBI, I, RC,
-                                  I.getOperand(OpIdx));
-}
-
 bool InstructionSelector::isOperandImmEqual(
     const MachineOperand &MO, int64_t Value,
     const MachineRegisterInfo &MRI) const {
   if (MO.isReg() && MO.getReg())
     if (auto VRegVal = getConstantVRegValWithLookThrough(MO.getReg(), MRI))
       return VRegVal->Value.getSExtValue() == Value;
   return false;
 }
 
 bool InstructionSelector::isBaseWithConstantOffset(
     const MachineOperand &Root, const MachineRegisterInfo &MRI) const {

llvm/lib/CodeGen/GlobalISel/Utils.cpp

Register llvm::constrainRegToClass(MachineRegisterInfo &MRI,
 
   return Reg;
 }
 
 Register llvm::constrainOperandRegClass(
     const MachineFunction &MF, const TargetRegisterInfo &TRI,
     MachineRegisterInfo &MRI, const TargetInstrInfo &TII,
     const RegisterBankInfo &RBI, MachineInstr &InsertPt,
-    const TargetRegisterClass &RegClass, const MachineOperand &RegMO) {
+    const TargetRegisterClass &RegClass, MachineOperand &RegMO) {
   Register Reg = RegMO.getReg();
   // Assume physical registers are properly constrained.
   assert(Register::isVirtualRegister(Reg) && "PhysReg not implemented");
 
   Register ConstrainedReg = constrainRegToClass(MRI, TII, RBI, Reg, RegClass);
   // If we created a new virtual register because the class is not compatible
   // then create a copy between the new and the old register.
   if (ConstrainedReg != Reg) {
     MachineBasicBlock::iterator InsertIt(&InsertPt);
     MachineBasicBlock &MBB = *InsertPt.getParent();
     if (RegMO.isUse()) {
       BuildMI(MBB, InsertIt, InsertPt.getDebugLoc(),
               TII.get(TargetOpcode::COPY), ConstrainedReg)
           .addReg(Reg);
     } else {
       assert(RegMO.isDef() && "Must be a definition");
       BuildMI(MBB, std::next(InsertIt), InsertPt.getDebugLoc(),
               TII.get(TargetOpcode::COPY), Reg)
           .addReg(ConstrainedReg);
     }
+    if (GISelChangeObserver *Observer = MF.getObserver()) {
+      Observer->changingInstr(*RegMO.getParent());
+    }
+    RegMO.setReg(ConstrainedReg);
+    if (GISelChangeObserver *Observer = MF.getObserver()) {
+      Observer->changedInstr(*RegMO.getParent());
+    }
   } else {
     if (GISelChangeObserver *Observer = MF.getObserver()) {
       if (!RegMO.isDef()) {
         MachineInstr *RegDef = MRI.getVRegDef(Reg);
         Observer->changedInstr(*RegDef);
       }
       Observer->changingAllUsesOfReg(MRI, Reg);
       Observer->finishedChangingAllUsesOfReg();
     }
   }
   return ConstrainedReg;
 }
 
 Register llvm::constrainOperandRegClass(
     const MachineFunction &MF, const TargetRegisterInfo &TRI,
     MachineRegisterInfo &MRI, const TargetInstrInfo &TII,
     const RegisterBankInfo &RBI, MachineInstr &InsertPt, const MCInstrDesc &II,
-    const MachineOperand &RegMO, unsigned OpIdx) {
+    MachineOperand &RegMO, unsigned OpIdx) {
   Register Reg = RegMO.getReg();
   // Assume physical registers are properly constrained.
   assert(Register::isVirtualRegister(Reg) && "PhysReg not implemented");
 
   const TargetRegisterClass *RegClass = TII.getRegClass(II, OpIdx, &TRI, MF);
   // Some of the target independent instructions, like COPY, may not impose any
   // register class constraints on some of their operands: If it's a use, we can
   // skip constraining as the instruction defining the register would constrain
for (unsigned OpI = 0, OpE = I.getNumExplicitOperands(); OpI != OpE; ++OpI) {
     // Register operands with a value of 0 (e.g. predicate operands) don't need
     // to be constrained.
     if (Reg == 0)
       continue;
 
     // If the operand is a vreg, we should constrain its regclass, and only
     // insert COPYs if that's impossible.
     // constrainOperandRegClass does that for us.
-    MO.setReg(constrainOperandRegClass(MF, TRI, MRI, TII, RBI, I, I.getDesc(),
-                                       MO, OpI));
+    constrainOperandRegClass(MF, TRI, MRI, TII, RBI, I, I.getDesc(), MO, OpI);
 
     // Tie uses to defs as indicated in MCInstrDesc if this hasn't already been
     // done.
     if (MO.isUse()) {
       int DefIdx = I.getDesc().getOperandConstraint(OpI, MCOI::TIED_TO);
       if (DefIdx != -1 && !I.isRegTiedToUseOperand(DefIdx))
         I.tieOperands(DefIdx, OpI);
     }

llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp

bool AArch64CallLowering::lowerTailCall(
   }
 
   // Now we can add the actual call instruction to the correct basic block.
   MIRBuilder.insertInstr(MIB);
 
   // If Callee is a reg, since it is used by a target specific instruction,
   // it must have a register class matching the constraint of that instruction.
   if (Info.Callee.isReg())
-    MIB->getOperand(0).setReg(constrainOperandRegClass(
-        MF, *TRI, MRI, *MF.getSubtarget().getInstrInfo(),
-        *MF.getSubtarget().getRegBankInfo(), *MIB, MIB->getDesc(), Info.Callee,
-        0));
+    constrainOperandRegClass(MF, *TRI, MRI, *MF.getSubtarget().getInstrInfo(),
+                             *MF.getSubtarget().getRegBankInfo(), *MIB,
+                             MIB->getDesc(), Info.Callee, 0);
 
   MF.getFrameInfo().setHasTailCall();
   Info.LoweredTailCall = true;
   return true;
 }
 
 bool AArch64CallLowering::lowerCall(MachineIRBuilder &MIRBuilder,
                                     CallLoweringInfo &Info) const {
bool AArch64CallLowering::lowerCall(MachineIRBuilder &MIRBuilder,
 
   // Now we can add the actual call instruction to the correct basic block.
   MIRBuilder.insertInstr(MIB);
 
   // If Callee is a reg, since it is used by a target specific
   // instruction, it must have a register class matching the
   // constraint of that instruction.
   if (Info.Callee.isReg())
-    MIB->getOperand(0).setReg(constrainOperandRegClass(
-        MF, *TRI, MRI, *MF.getSubtarget().getInstrInfo(),
-        *MF.getSubtarget().getRegBankInfo(), *MIB, MIB->getDesc(), Info.Callee,
-        0));
+    constrainOperandRegClass(MF, *TRI, MRI, *MF.getSubtarget().getInstrInfo(),
+                             *MF.getSubtarget().getRegBankInfo(), *MIB,
+                             MIB->getDesc(), Info.Callee, 0);
 
   // Finally we can copy the returned value back into its virtual-register. In
   // symmetry with the arguments, the physical register must be an
   // implicit-define of the call instruction.
   if (!Info.OrigRet.Ty->isVoidTy()) {
     CCAssignFn *RetAssignFn = TLI.CCAssignFnForReturn(Info.CallConv);
     CallReturnHandler Handler(MIRBuilder, MRI, MIB, RetAssignFn);
     if (!handleAssignments(MIRBuilder, InArgs, Handler))