Support for function-live-in virtual registers

[sunfish]

Summary

The WebAssembly virtual ISA has an infinite virtual register set, and function arguments are passed in registers. We're currently using pseudo-instructions (eg. ARGUMENT_i32) inserted into the entry block which provide register definitions, however it's awkward to prevent other instructions from being scheduled above them, and subsequently to prevent register coloring from clobbering the argument registers before they are properly defined.

The patch here provides a different option: make it possible to have live-in virtual registers. This way, their liveness is naturally correct and liveness-oriented passes tend to handle them correctly by default, and it eliminates the need for the awkward pseudo-instructions.

The tricky part is that such virtual registers no longer have a defining instruction, which some parts of CodeGen currently assume, particularly the passes that operate on SSA form. To address this, this patch adds a getVRegDefMBB(), for use when code is calling getVRegDef() just to get the block the register is defined in (and this is what motivates making MachineRegisterInfo's MachineFunction non-const), and adds special checks when code is calling getVRegDef() because it actually wants the defining instruction.

[qcolombet]

Hi Dan,

I am puzzled by the following statement:

however it's awkward to prevent other instructions from being scheduled above them, and subsequently to prevent register coloring from clobbering the argument registers before they are properly defined.

Since you have an infinite number of registers, how do you end up with other registers clobbering the arguments registers?
Are these arguments registers actual physical registers?!

To be frank, I do not like the approach that’s why I would prefer to understand what are the constraints of the problem first.

That being said, let me comment a bit on the proposal. You are restricting the live-in information to the entry block and that sounds wrong. Indeed, I guess if you go down that road, you want to model arguments of landing pad the same way and the related blocks are not the entry block anymore!

Thanks,
-Quentin

[sunfish]

Since you have an infinite number of registers, how do you end up with other registers > clobbering the arguments registers?
Are these arguments registers actual physical registers?!

We have an infinite number of registers, but it's still desirable to minimize the total number of registers, so we're doing coloring. It's similar to stack slot coloring; there are an unbounded number of stack slots, but it's desirable to minimize the total size of the stack.

To be frank, I do not like the approach that’s why I would prefer to understand what are the
constraints of the problem first.

We have arguments which are virtual registers. They are conceptually live-in to the function. How should we model this?

Right now we are modelling this with instructions in the entry block that define the values. This is not robust because the scheduler can move them below other instructions. Modelling them as live-in to the function would be a very natural way to express what we mean here.

That being said, let me comment a bit on the proposal. You are restricting the live-in information to the entry block and that sounds wrong. Indeed, I guess if you go down that road, you want to model arguments of landing pad the same way and the related blocks are not the entry block anymore!

I don't understand. The patch here models the registers as live-in to the function, using the same live-in mechanism that already exists in CodeGen for physical registers. Registers that are live-in to a function are naturally live-in to the entry block.

[qcolombet]

Hi Dan,

We have an infinite number of registers, but it's still desirable to minimize the total number of registers, so we're doing coloring.

So you technically don’t have a clobbering problem. You would just like to have them appear at the beginning of the function so that it is easier for the consumers of the web assembly IR to lower the ABI properly.

What am I saying is that the constraints you have are soft. You could imagine a different way of telling the consumers of the web assembly IR what are the arguments of the function.
By the way, if you get rid of those ARGUMENT instructions, how do the consumers of the web assembly IR know what they have to lower for the ABI?

I don't understand. The patch here models the registers as live-in to the function[...]. Registers that are live-in to a function are naturally live-in to the entry block.

Yes, but what I am saying is that you are restricting the notion of live-in for a virtual register to the entry block, whereas the one for physreg applies to all blocks. This is this asymmetry that I feel is wrong.
Wouldn’t it make sense to instead generalize the notion of physreg for your goals?

I think that if I know how the consumer of the web assembly lower the ABI (I.e., my first question), it would be easier to advice on the best way to tackle this.

What do you think?

Cheers,
-Quentin

[sunfish]

So you technically don’t have a clobbering problem. You would just like to have them appear at the beginning of the function so that it is easier for the consumers of the web assembly IR to lower the ABI properly.

We actually want to color these registers along with the other registers, and this requires us to have precise liveness information.

What am I saying is that the constraints you have are soft. You could imagine a different way of telling the consumers of the web assembly IR what are the arguments of the function.
By the way, if you get rid of those ARGUMENT instructions, how do the consumers of the web assembly IR know what they have to lower for the ABI?

The signature of a function is explicit, so a consumer knows how many arguments there are. In a function with N arguments, the first N register numbers are the arguments (the numbering is not LLVM's virtual register numbering). Other than being initialized to the incoming argument values, these registers thereafter are identical to regular registers.

Also, if there is another way to solve the problem, I'd be very interested to hear more about it.

Yes, but what I am saying is that you are restricting the notion of live-in for a virtual register to the entry block, whereas the one for physreg applies to all blocks. This is this asymmetry that I feel is wrong.
Wouldn’t it make sense to instead generalize the notion of physreg for your goals?

Perhaps this assymetry could be said to be a reflection of the assymetry in the way LiveVariables and LiveIntervals track liveness of virtual versus physical registers.

Generalizing the concept of physregs to support variable numbers of physregs would be another possible solution. Would you prefer that? I expect it would require much more invasive changes.

[qcolombet]

Hi Dan,

Also, if there is another way to solve the problem, I'd be very interested to hear more about it.

I am not sure what to suggest given that I do not know all the parameters of the problem.
In particular, what does it mean for WebAssembly to color?

You are saying that you have an infinite number of physical registers, but on the other hands we don't support having a variable number of physical registers. How does this work?

My problem with the approach is that the scope seems so narrow that it feels to me like a hack.
E.g.,

1. / Return the machine basic block that contains the definition of the / specified virtual register or null if none is found. This assumes that /// the code is in SSA form, so there should only be one definition. MachineBasicBlock *getVRegDefMBB(unsigned Reg) const;

What happen when the code is not in SSA anymore?
Say you have:
foo(v1)
if ()

v3 = ...
v2 = phi(v1, v3)

Which gives after coalescing:
foo(v1)
if ()

v1 = ...

How do you represent that?

In particular, we will see only one def if we start iterating over the def-uses.

The only place where virtual registers can be live-in are function entry and this is never called out and it is not future proof IMHO.
For instance, what will happen when you will want to model things like:
invoke foo landing pad bb1

The virtual registers for the landing pad (i.e., the arguments of the funclet) would need to be live-in of bb1 only, if you want a precise liveness.

All of these sounds to me like we do want physical register semantics, but we are not willing to pay the price for it.

So yes, I would certainly prefer having a variable number of physical registers and I do understand this is much more involved. For now, the proposed approach looks more error prone to me than the current implementation with the ARGUMENT instruction, where the scope is limited to web assembly and where the generic passes know how to deal with it (modulo the placement problem).

What about an attribute or something that says don't move those instructions?
I am guessing we must have something similar for barrier like instruction.

Perhaps this assymetry could be said to be a reflection of the assymetry in the way LiveVariables and LiveIntervals track liveness of virtual versus physical registers.

To be fair, those two liveness tracking are used differently and I think it would be nice to kill the LiveVariables at some point.

Generalizing the concept of physregs to support variable numbers of physregs would be another possible solution. Would you prefer that?

I admit I would prefer that.

Cheers,
-Quentin

[sunfish]

Make getVRegDefMBB return NULL if a register is live-in and has other defs besides, meaning it is not in SSA form. This is more consistent with the existing getVRegDef.

[sunfish]

I am not sure what to suggest given that I do not know all the parameters of the problem.
In particular, what does it mean for WebAssembly to color?

It's the same thing as lib/CodeGen/StackSlotColoring.cpp, but with virtual registers instead of stack slots. Here's the code:

http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/WebAssembly/WebAssemblyRegColoring.cpp?revision=253626&view=markup

The only tricky part is moving ARGUMENT instructions back to the top of the function and recomputing liveness, and that's the very code that the patch here is seeking to eliminate.

You are saying that you have an infinite number of physical registers, but on the other hands we don't support having a variable number of physical registers. How does this work?

The machine has infinite registers. We currently model them in LLVM with virtual registers.

Return the machine basic block that contains the definition of the / specified virtual register or null if none is found. This assumes that /// the code is in SSA form, so there should only be one definition. MachineBasicBlock *getVRegDefMBB(unsigned Reg) const;

What happen when the code is not in SSA anymore?

getVRegDefMBB is modeled directly after getVRegDef which already exists and already works this way. All uses of getVRegDefMBB were previously users of getVRegDef.

That said, this did prompt me to notice a bug; getVRegDef returns nullptr if the register has multiple defines; I've now updated the patch to make getVRegDefMBB check the same condition.

The only place where virtual registers can be live-in are function entry and this is never called out and it is not future proof IMHO.
For instance, what will happen when you will want to model things like:
invoke foo landing pad bb1

The virtual registers for the landing pad (i.e., the arguments of the funclet) would need to be live-in of bb1 only, if you want a precise liveness.

This is a basic asymmetry that already exists; machine basic blocks don't track virtual registers in their live-in sets.

Also, the return value of the call is not live across the unwind edge, so we're not going to be tempted to specially create newly live virtual registers along that edge.

All of these sounds to me like we do want physical register semantics, but we are not willing to pay the price for it.

What do you mean by "physical register semantics"?

So yes, I would certainly prefer having a variable number of physical registers and I do understand this is much more involved. For now, the proposed approach looks more error prone to me than the current implementation with the ARGUMENT instruction, where the scope is limited to web assembly and where the generic passes know how to deal with it (modulo the placement problem).

What about an attribute or something that says don't move those instructions?
I am guessing we must have something similar for barrier like instruction.

It's not easy to do, because it isn't enough to say "this instruction can't move", because we also need to say "other instructions can't move past this one, even if they have no side effects". And if we say that, then we have to think about whether it's valid to do MachineLICM, MachineCSE, or even register allocation itself in the vicinity of such a powerful instruction. It would end up being a fairly complex mechanism.

Perhaps this assymetry could be said to be a reflection of the assymetry in the way LiveVariables and LiveIntervals track liveness of virtual versus physical registers.

To be fair, those two liveness tracking are used differently and I think it would be nice to kill the LiveVariables at some point.

The following asymmetry in the MIR data structures already exists:

      Machine  MBB     MF
      Operand  LiveIn  LiveIn
     +-------+-------+-------+
Phys |   Y   |  Y    |   Y   |
Virt |   Y   |  N    |   N   |

My change would change the Func LiveIn column for virtual registers. This isn't introducing any new asymetry.

In fact, it's increasing symmetry because a register lifetime can start either at an instruction or before the function starts.

[sunfish]

getVRegDef actually asserts if there isn't exactly one def, rather than returning nullptr, so make getVRegDefMBB do that too.

[qcolombet]

Hi Dan,

Thanks for your patience, taking some time to catch up :).

What do you mean by "physical register semantics”?

I mean having a register live-in of a function. This is not something that, at least, I expect from a virtual register.
It may be quite of a stretch, but the reason is because those live-ins have in fact several definitions, i.e., the setting of the arguments in each caller, whereas virtual registers are supposed to have just one (in the case of live-ins of a function, a reasonable representation, though not practical, would be a phi of each argument from the callers). The physical registers are IMO a cheap and accurate way to pieces the different live-ranges together.
That is why I believe this is better represented if it stays in the "physical register” semantic world.

This is a basic asymmetry that already exists; machine basic blocks don't track virtual registers in their live-in sets.

That is what I am saying and here, to me, we are making things worse, because we do not have such tracking but for the entry block. The fact that you said it is the live-in of the function instead of the live-in of the basic block is a subtle difference that may bit us.

My change would change the Func LiveIn column for virtual registers. This isn't introducing any new asymmetry.

Agree, but I don’t like the distinction between live-in of basic block and live-in of function. Right now, both physical and virtual registers are consistent with this respect, i.e., both have either have basic block *and* function live-in semantic available or basic block *and* function live-in semantic *not* available!

In fact, it's increasing symmetry because a register lifetime can start either at an instruction or before the function starts.

I disagree, this introduces a more subtle dissymmetry.

To summarize, I really believe the way to go is to being able to change the number of physical registers dynamically, without impacting the compile time of the targets that know this number statically. Another option could be to change the way we represent phi to go toward a swift IR like representation, i.e., arguments of basic block and use that on the entry block.

I reckon both options are involved, but I guess we are not in a hurry, so we can shot for the proper fix :).
Personally I prefer the physical registers approach.

Cheers,
-Quentin

[sunfish]

Closing the phabricator entry for this patch, which is no longer active.

include/llvm/CodeGen/MachineRegisterInfo.h

class Delegate {
     virtual void anchor();
   public:
     virtual void MRI_NoteNewVirtualRegister(unsigned Reg) = 0;
 
     virtual ~Delegate() {}
   };
 
 private:
-  const MachineFunction *MF;
+  MachineFunction *MF;
   Delegate *TheDelegate;
 
   /// IsSSA - True when the machine function is in SSA form and virtual
   /// registers have a single def.
   bool IsSSA;
 
   /// TracksLiveness - True while register liveness is being tracked accurately.
   /// Basic block live-in lists, kill flags, and implicit defs may not be
private:
   /// Live in values are typically arguments in registers.  LiveIn values are
   /// allowed to have virtual registers associated with them, stored in the
   /// second element.
   std::vector<std::pair<unsigned, unsigned> > LiveIns;
 
   MachineRegisterInfo(const MachineRegisterInfo&) = delete;
   void operator=(const MachineRegisterInfo&) = delete;
 public:
-  explicit MachineRegisterInfo(const MachineFunction *MF);
+  explicit MachineRegisterInfo(MachineFunction *MF);
 
   const TargetRegisterInfo *getTargetRegisterInfo() const {
     return MF->getSubtarget().getRegisterInfo();
   }
 
   void resetDelegate(Delegate *delegate) {
     // Ensure another delegate does not take over unless the current
     // delegate first unattaches itself. If we ever need to multicast
public:
   /// form, so there should only be one definition.
   MachineInstr *getVRegDef(unsigned Reg) const;
 
   /// getUniqueVRegDef - Return the unique machine instr that defines the
   /// specified virtual register or null if none is found.  If there are
   /// multiple definitions or no definition, return null.
   MachineInstr *getUniqueVRegDef(unsigned Reg) const;
 
+  /// Return the machine basic block that contains the definition of the
+  /// specified virtual register or null if none is found. This assumes that
+  /// the code is in SSA form, so there should only be one definition.
+  MachineBasicBlock *getVRegDefMBB(unsigned Reg) const;
+
   /// clearKillFlags - Iterate over all the uses of the given register and
   /// clear the kill flag from the MachineOperand. This function is used by
   /// optimization passes which extend register lifetimes and need only
   /// preserve conservative kill flag information.
   void clearKillFlags(unsigned Reg) const;
 
 #ifndef NDEBUG
   void dumpUses(unsigned RegNo) const;

lib/CodeGen/LiveRangeCalc.cpp

bool LiveRangeCalc::findReachingDefs(LiveRange &LR, MachineBasicBlock &UseMBB,
   // Remember if we have seen more than one value.
   bool UniqueVNI = true;
   VNInfo *TheVNI = nullptr;
 
   // Using Seen as a visited set, perform a BFS for all reaching defs.
   for (unsigned i = 0; i != WorkList.size(); ++i) {
     MachineBasicBlock *MBB = MF->getBlockNumbered(WorkList[i]);
 
+    // For the entry block, check for the value being live-in to the function.
+    if (MBB == &MF->front() && MRI->isLiveIn(PhysReg)) {
+      assert(MBB->pred_empty());
+      SlotIndex Start = Indexes->getMBBStartIdx(&MF->front());
+      VNInfo *VNI = LR.getNextValue(Start, *Alloc);
+      if (TheVNI && TheVNI != VNI)
+        UniqueVNI = false;
+      TheVNI = VNI;
+      continue;
+    }
+
 #ifndef NDEBUG
     if (MBB->pred_empty()) {
       MBB->getParent()->verify();
       errs() << "Use of " << PrintReg(PhysReg)
              << " does not have a corresponding definition on every path:\n";
       const MachineInstr *MI = Indexes->getInstructionFromIndex(Use);
       if (MI != nullptr)
         errs() << Use << " " << *MI;

lib/CodeGen/LiveVariables.cpp

while (!WorkList.empty()) {
     MachineBasicBlock *Pred = WorkList.back();
     WorkList.pop_back();
     MarkVirtRegAliveInBlock(VRInfo, DefBlock, Pred, WorkList);
   }
 }
 
 void LiveVariables::HandleVirtRegUse(unsigned reg, MachineBasicBlock *MBB,
                                      MachineInstr *MI) {
-  assert(MRI->getVRegDef(reg) && "Register use before def!");
+  assert(MRI->getVRegDefMBB(reg) && "Register use before def!");
 
   unsigned BBNum = MBB->getNumber();
 
   VarInfo& VRInfo = getVarInfo(reg);
 
   // Check to see if this basic block is already a kill block.
   if (!VRInfo.Kills.empty() && VRInfo.Kills.back()->getParent() == MBB) {
     // Yes, this register is killed in this basic block already. Increase the
#endif
   //     |   t1 = ...
   //     |  ... = ... t1 ...
   //     |      |
   //     `------'
   //
   // where there is a use in a PHI node that's a predecessor to the defining
   // block. We don't want to mark all predecessors as having the value "alive"
   // in this case.
-  if (MBB == MRI->getVRegDef(reg)->getParent()) return;
+  if (MBB == MRI->getVRegDefMBB(reg)) return;
 
   // Add a new kill entry for this basic block. If this virtual register is
   // already marked as alive in this basic block, that means it is alive in at
   // least one of the successor blocks, it's not a kill.
   if (!VRInfo.AliveBlocks.test(BBNum))
     VRInfo.Kills.push_back(MI);
 
   // Update all dominating blocks to mark them as "known live".
   for (MachineBasicBlock::const_pred_iterator PI = MBB->pred_begin(),
          E = MBB->pred_end(); PI != E; ++PI)
-    MarkVirtRegAliveInBlock(VRInfo, MRI->getVRegDef(reg)->getParent(), *PI);
+    MarkVirtRegAliveInBlock(VRInfo, MRI->getVRegDefMBB(reg), *PI);
 }
 
 void LiveVariables::HandleVirtRegDef(unsigned Reg, MachineInstr *MI) {
   VarInfo &VRInfo = getVarInfo(Reg);
 
   if (VRInfo.AliveBlocks.empty())
     // If vr is not alive in any block, then defaults to dead.
     VRInfo.Kills.push_back(MI);
void LiveVariables::runOnBlock(MachineBasicBlock *MBB, const unsigned NumRegs) {
   // if they have PHI nodes, and if so, we simulate an assignment at the end
   // of the current block.
   if (!PHIVarInfo[MBB->getNumber()].empty()) {
     SmallVectorImpl<unsigned> &VarInfoVec = PHIVarInfo[MBB->getNumber()];
 
     for (SmallVectorImpl<unsigned>::iterator I = VarInfoVec.begin(),
            E = VarInfoVec.end(); I != E; ++I)
       // Mark it alive only in the block we are representing.
-      MarkVirtRegAliveInBlock(getVarInfo(*I),MRI->getVRegDef(*I)->getParent(),
-                              MBB);
+      MarkVirtRegAliveInBlock(getVarInfo(*I), MRI->getVRegDefMBB(*I), MBB);
   }
 
   // MachineCSE may CSE instructions which write to non-allocatable physical
   // registers across MBBs. Remember if any reserved register is liveout.
   SmallSet<unsigned, 4> LiveOuts;
   for (MachineBasicBlock::const_succ_iterator SI = MBB->succ_begin(),
          SE = MBB->succ_end(); SI != SE; ++SI) {
     MachineBasicBlock *SuccMBB = *SI;
bool LiveVariables::VarInfo::isLiveIn(const MachineBasicBlock &MBB,
                                       MachineRegisterInfo &MRI) {
   unsigned Num = MBB.getNumber();
 
   // Reg is live-through.
   if (AliveBlocks.test(Num))
     return true;
 
   // Registers defined in MBB cannot be live in.
-  const MachineInstr *Def = MRI.getVRegDef(Reg);
-  if (Def && Def->getParent() == &MBB)
+  if (MRI.getVRegDefMBB(Reg) == &MBB)
     return false;
 
  // Reg was not defined in MBB, was it killed here?
   return findKill(&MBB);
 }
 
 bool LiveVariables::isLiveOut(unsigned Reg, const MachineBasicBlock &MBB) {
   LiveVariables::VarInfo &VI = getVarInfo(Reg);

lib/CodeGen/MachineCSE.cpp

for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
     MachineOperand &MO = MI->getOperand(i);
     if (!MO.isReg() || !MO.isUse())
       continue;
     unsigned Reg = MO.getReg();
     if (!TargetRegisterInfo::isVirtualRegister(Reg))
       continue;
     bool OnlyOneUse = MRI->hasOneNonDBGUse(Reg);
     MachineInstr *DefMI = MRI->getVRegDef(Reg);
-    if (!DefMI->isCopy())
+    if (!DefMI || !DefMI->isCopy())
       continue;
     unsigned SrcReg = DefMI->getOperand(1).getReg();
     if (!TargetRegisterInfo::isVirtualRegister(SrcReg))
       continue;
     if (DefMI->getOperand(0).getSubReg())
       continue;
     // FIXME: We should trivially coalesce subregister copies to expose CSE
     // opportunities on instructions with truncated operands (see

lib/CodeGen/MachineLICM.cpp

if (TargetRegisterInfo::isPhysicalRegister(Reg)) {
         // which would clobber it.
         return false;
       }
     }
 
     if (!MO.isUse())
       continue;
 
-    assert(MRI->getVRegDef(Reg) &&
+    assert(MRI->getVRegDefMBB(Reg) &&
            "Machine instr not mapped for this vreg?!");
 
     // If the loop contains the definition of an operand, then the instruction
     // isn't loop invariant.
-    if (CurLoop->contains(MRI->getVRegDef(Reg)))
+    if (CurLoop->contains(MRI->getVRegDefMBB(Reg)))
       return false;
   }
 
   // If we got this far, the instruction is loop invariant!
   return true;
 }
 
 

lib/CodeGen/MachineRegisterInfo.cpp

 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetSubtargetInfo.h"
 
 using namespace llvm;
 
 // Pin the vtable to this file.
 void MachineRegisterInfo::Delegate::anchor() {}
 
-MachineRegisterInfo::MachineRegisterInfo(const MachineFunction *MF)
+MachineRegisterInfo::MachineRegisterInfo(MachineFunction *MF)
   : MF(MF), TheDelegate(nullptr), IsSSA(true), TracksLiveness(true),
     TracksSubRegLiveness(false) {
   VRegInfo.reserve(256);
   RegAllocHints.reserve(256);
   UsedPhysRegMask.resize(getTargetRegisterInfo()->getNumRegs());
 
   // Create the physreg use/def lists.
   PhysRegUseDefLists.resize(getTargetRegisterInfo()->getNumRegs(), nullptr);

 MachineInstr *MachineRegisterInfo::getUniqueVRegDef(unsigned Reg) const {
   if (def_empty(Reg)) return nullptr;
   def_instr_iterator I = def_instr_begin(Reg);
   if (std::next(I) != def_instr_end())
     return nullptr;
   return &*I;
 }
 
+/// Return the machine basic block that contains the definition of the
+/// specified virtual register or null if none is found. This assumes that
+/// the code is in SSA form, so there should only be one definition.
+MachineBasicBlock *MachineRegisterInfo::getVRegDefMBB(unsigned Reg) const {
+  bool isLiveIn = false;
+  for (const auto &Pair : LiveIns)
+    if (Pair.first == Reg) {
+      isLiveIn = true;
+      break;
+    }
+
+  def_instr_iterator I = def_instr_begin(Reg);
+  assert(((!I.atEnd() && std::next(I).atEnd()) + isLiveIn <= 1) &&
+         "getVRegDefMBB assumes a single definition or no definition");
+  return !I.atEnd() ? I->getParent() :
+         isLiveIn ? &MF->front() :
+         nullptr;
+}
+
 bool MachineRegisterInfo::hasOneNonDBGUse(unsigned RegNo) const {
   use_nodbg_iterator UI = use_nodbg_begin(RegNo);
   if (UI == use_nodbg_end())
     return false;
   return ++UI == use_nodbg_end();
 }
 
 /// clearKillFlags - Iterate over all the uses of the given register and
if (LiveIns[i].second) {
         // Emit a copy.
         BuildMI(*EntryMBB, EntryMBB->begin(), DebugLoc(),
                 TII.get(TargetOpcode::COPY), LiveIns[i].second)
           .addReg(LiveIns[i].first);
 
         // Add the register to the entry block live-in set.
         EntryMBB->addLiveIn(LiveIns[i].first);
       }
-    } else {
+    } else if (TargetRegisterInfo::isPhysicalRegister(LiveIns[i].first)) {
       // Add the register to the entry block live-in set.
       EntryMBB->addLiveIn(LiveIns[i].first);
     }
 }
 
 LaneBitmask MachineRegisterInfo::getMaxLaneMaskForVReg(unsigned Reg) const {
   // Lane masks are only defined for vregs.
   assert(TargetRegisterInfo::isVirtualRegister(Reg));

lib/CodeGen/MachineVerifier.cpp

if (!regsLive.count(Reg)) {
                 Bad = false;
                 break;
               }
             }
           }
         }
         if (Bad)
           report("Using an undefined physical register", MO, MONum);
-      } else if (MRI->def_empty(Reg)) {
+      } else if (MRI->def_empty(Reg) && !MRI->isLiveIn(Reg)) {
         report("Reading virtual register without a def", MO, MONum);
       } else {
         BBInfo &MInfo = MBBInfoMap[MI->getParent()];
         // We don't know which virtual registers are live in, so only complain
         // if vreg was killed in this MBB. Otherwise keep track of vregs that
         // must be live in. PHI instructions are handled separately.
         if (MInfo.regsKilled.count(Reg))
           report("Using a killed virtual register", MO, MONum);

lib/CodeGen/PeepholeOptimizer.cpp

if (Res.isValid()) {
     // Update the result before moving up in the use-def chain
     // with the instruction containing the last found sources.
     Res.setInst(Def);
 
     // If we can still move up in the use-def chain, move to the next
     // definition.
     if (!TargetRegisterInfo::isPhysicalRegister(Reg) && OneRegSrc) {
       Def = MRI.getVRegDef(Reg);
+      if (Def) {
         DefIdx = MRI.def_begin(Reg).getOperandNo();
         DefSubReg = Res.getSrcSubReg(0);
         return Res;
       }
     }
+  }
   // If we end up here, this means we will not be able to find another source
   // for the next iteration. Make sure any new call to getNextSource bails out
   // early by cutting the use-def chain.
   Def = nullptr;
   return Res;
 }

lib/CodeGen/RegisterCoalescer.cpp

if (!canJoinPhys(CP)) {
       bool IsDefCopy;
       if (reMaterializeTrivialDef(CP, CopyMI, IsDefCopy))
         return true;
       if (IsDefCopy)
         Again = true;  // May be possible to coalesce later.
       return false;
     }
   } else {
-    // When possible, let DstReg be the larger interval.
-    if (!CP.isPartial() && LIS->getInterval(CP.getSrcReg()).size() >
-                           LIS->getInterval(CP.getDstReg()).size())
+    // When possible, let DstReg be the larger interval, or the interval for
+    // a live-in virtual register since it has no def.
+    if (!CP.isPartial()) {
+      LiveInterval &Src = LIS->getInterval(CP.getSrcReg());
+      LiveInterval &Dst = LIS->getInterval(CP.getDstReg());
+      if (Src.size() > Dst.size() || LIS->isLiveInToMBB(Src, &MF->front()))
         CP.flip();
+    }
 
     DEBUG({
       dbgs() << "\tConsidering merging to "
              << TRI->getRegClassName(CP.getNewRC()) << " with ";
       if (CP.getDstIdx() && CP.getSrcIdx())
         dbgs() << PrintReg(CP.getDstReg()) << " in "
                << TRI->getSubRegIndexName(CP.getDstIdx()) << " and "
                << PrintReg(CP.getSrcReg()) << " in "