[DebugInfo][InstrRef] Track subregisters in stack slots

[jmorse]

Summary

Occasionally we will generate code that writes to a subregister, then spills / restores a super-register to the stack, for example:

$eax = MOV32ri 0 
MOV64mr $rsp, 1, $noreg, 16, $noreg, $rax
$rcx = MOV64rm $rsp, 1, $noreg, 8, $noreg

On x86 at least, memory instructions are widened to 32/64 bits to achieve the same spill/restore but save on instruction length. Instr-Ref LiveDebugValues isn't able to follow the value however, as it only stores the value in $rax, not the one in $eax.

Address this by tracking all the subregister values of anything that's stored to the stack. Multiple LocIdx locations are created for each slot, each paired with a subregister number, and any spill/restore instruction will transfer all subregister values to/from the stack locations.

While we're here, add a little more type safety by wrapping stack slot numbers in a class, to avoid them being automatically treated as a register. Overload getLocID so that a subregister index *must* be provided if you fetch a stack slot location. Convert getRegMLoc to return an optional, so that we get assertions if the two are ever mixed.

When transferring subregister values on spill and restore, the number transferred are limited to be only those that will fit in the stack slot, by the getStackSlotMaxSuperReg method. This is because AArch64 has some extensions that allow huge chunks of general purpose registers to be addressed en-mass, and so the full register hierarchy can have a
superregister covering almost all the general purpose registers. This can violate an assumption I've made, that LLVM doesn't spill non-zero bit offset subregisters into the same slot as other registers, i.e.:

MOV8mr $rsp, 1, $noreg, 16, $noreg, $ah
MOV64mr $rsp, 1, $noreg, 16, $noreg, $rax

Which would be more difficult to address if encountered.

[jmorse]

Superceeded by D112133

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

public:
   bool operator!=(const DbgValueProperties &Other) const {
     return !(*this == Other);
   }
 
   const DIExpression *DIExpr;
   bool Indirect;
 };
 
+/// Thin wrapper around an integer -- designed to give more type safety to
+/// spill location numbers.
+class SpillLocationNo {
+public:
+  explicit SpillLocationNo(unsigned SpillNo) : SpillNo(SpillNo) { }
+  unsigned SpillNo;
+  unsigned id() const { return SpillNo; }
+};
+
 /// Tracker for what values are in machine locations. Listens to the Things
 /// being Done by various instructions, and maintains a table of what machine
 /// locations have what values (as defined by a ValueIDNum).
 ///
 /// There are potentially a much larger number of machine locations on the
 /// target machine than the actual working-set size of the function. On x86 for
 /// example, we're extremely unlikely to want to track values through control
 /// or debug registers. To avoid doing so, MLocTracker has several layers of
public:
   UniqueVector<SpillLoc> SpillLocs;
 
   // If we discover a new machine location, assign it an mphi with this
   // block number.
   unsigned CurBB;
 
   /// Cached local copy of the number of registers the target has.
   unsigned NumRegs;
+  /// Cached local copy of the number of subregisters the target has.
+  unsigned NumSubRegs;
 
   /// Collection of register mask operands that have been observed. Second part
   /// of pair indicates the instruction that they happened in. Used to
   /// reconstruct where defs happened if we start tracking a location later
   /// on.
   SmallVector<std::pair<const MachineOperand *, unsigned>, 32> Masks;
 
   /// Iterator for locations and the values they contain. Dereferencing
public:
   };
 
   MLocTracker(MachineFunction &MF, const TargetInstrInfo &TII,
               const TargetRegisterInfo &TRI, const TargetLowering &TLI)
       : MF(MF), TII(TII), TRI(TRI), TLI(TLI),
         LocIdxToIDNum(ValueIDNum::EmptyValue),
         LocIdxToLocID(0) {
     NumRegs = TRI.getNumRegs();
+    NumSubRegs = TRI.getNumSubRegIndices();
     reset();
     LocIDToLocIdx.resize(NumRegs, LocIdx::MakeIllegalLoc());
     assert(NumRegs < (1u << NUM_LOC_BITS)); // Detect bit packing failure
 
     // Always track SP. This avoids the implicit clobbering caused by regmasks
     // from affectings its values. (LiveDebugValues disbelieves calls and
     // regmasks that claim to clobber SP).
     Register SP = TLI.getStackPointerRegisterToSaveRestore();
     if (SP) {
-      unsigned ID = getLocID(SP, false);
+      unsigned ID = getLocID(SP);
       (void)lookupOrTrackRegister(ID);
     }
   }
 
-  /// Produce location ID number for indexing LocIDToLocIdx. Takes the register
-  /// or spill number, and flag for whether it's a spill or not.
-  unsigned getLocID(Register RegOrSpill, bool isSpill) {
-    return (isSpill) ? RegOrSpill.id() + NumRegs - 1 : RegOrSpill.id();
+  /// Produce location ID number for indexing LocIDToLocIdx. A no-op, but
+  /// provides more type safety for the future.
+  /// \param Reg The register we're looking up.
+  unsigned getLocID(Register Reg) {
+    return Reg.id();
+  }
+
+  /// Produce location ID number for indexing LocIDToLocIdx.
+  /// \param Spill The ID of the spill we're fetching the location for.
+  /// \apram SpillSubReg Subregister within the spill we're addressing.
+  unsigned getLocID(SpillLocationNo Spill, unsigned SpillSubReg) {
+    unsigned SlotNo = Spill.id() - 1;
+    SlotNo *= NumSubRegs;
+    SlotNo += SpillSubReg;
+    SlotNo += NumRegs;
+    return SlotNo;
+  }
+
+  unsigned locIDToSpill(unsigned ID) const {
+    assert(ID >= NumRegs);
+    ID -= NumRegs;
+    ID /= NumSubRegs;
+    return ID + 1; // The UniqueVector is one-based.
+  }
+
+  unsigned locIDToSpillSubReg(unsigned ID) const {
+    assert(ID >= NumRegs);
+    ID -= NumRegs;
+    unsigned SubReg = ID % NumSubRegs;
+    return SubReg;
   }
 
   /// Accessor for reading the value at Idx.
   ValueIDNum getNumAtPos(LocIdx Idx) const {
     assert(Idx.asU64() < LocIdxToIDNum.size());
     return LocIdxToIDNum[Idx];
   }
 
if (Index.isIllegal())
       Index = trackRegister(ID);
     return Index;
   }
 
   /// Record a definition of the specified register at the given block / inst.
   /// This doesn't take a ValueIDNum, because the definition and its location
   /// are synonymous.
   void defReg(Register R, unsigned BB, unsigned Inst) {
-    unsigned ID = getLocID(R, false);
+    unsigned ID = getLocID(R);
     LocIdx Idx = lookupOrTrackRegister(ID);
     ValueIDNum ValueID = {BB, Inst, Idx};
     LocIdxToIDNum[Idx] = ValueID;
   }
 
   /// Set a register to a value number. To be used if the value number is
   /// known in advance.
   void setReg(Register R, ValueIDNum ValueID) {
-    unsigned ID = getLocID(R, false);
+    unsigned ID = getLocID(R);
     LocIdx Idx = lookupOrTrackRegister(ID);
     LocIdxToIDNum[Idx] = ValueID;
   }
 
   ValueIDNum readReg(Register R) {
-    unsigned ID = getLocID(R, false);
+    unsigned ID = getLocID(R);
     LocIdx Idx = lookupOrTrackRegister(ID);
     return LocIdxToIDNum[Idx];
   }
 
   /// Reset a register value to zero / empty. Needed to replicate the
   /// VarLoc implementation where a copy to/from a register effectively
   /// clears the contents of the source register. (Values can only have one
   ///  machine location in VarLocBasedImpl).
   void wipeRegister(Register R) {
-    unsigned ID = getLocID(R, false);
+    unsigned ID = getLocID(R);
     LocIdx Idx = LocIDToLocIdx[ID];
     LocIdxToIDNum[Idx] = ValueIDNum::EmptyValue;
   }
 
   /// Determine the LocIdx of an existing register.
-  LocIdx getRegMLoc(Register R) {
-    unsigned ID = getLocID(R, false);
+  Optional<LocIdx> getRegMLoc(Register R) {
+    unsigned ID = getLocID(R);
+    if (LocIDToLocIdx[ID] == UINT_MAX) // Sentinal for IndexedMap.
+      return None;
     return LocIDToLocIdx[ID];
   }
 
   /// Record a RegMask operand being executed. Defs any register we currently
   /// track, stores a pointer to the mask in case we have to account for it
   /// later.
   void writeRegMask(const MachineOperand *MO, unsigned CurBB, unsigned InstID) {
     // Ensure SP exists, so that we don't override it later.
     Register SP = TLI.getStackPointerRegisterToSaveRestore();
 
     // Def any register we track have that isn't preserved. The regmask
     // terminates the liveness of a register, meaning its value can't be
     // relied upon -- we represent this by giving it a new value.
     for (auto Location : locations()) {
       unsigned ID = LocIdxToLocID[Location.Idx];
       // Don't clobber SP, even if the mask says it's clobbered.
       if (ID < NumRegs && ID != SP && MO->clobbersPhysReg(ID))
         defReg(ID, CurBB, InstID);
     }
     Masks.push_back(std::make_pair(MO, InstID));
   }
 
   /// Find LocIdx for SpillLoc \p L, creating a new one if it's not tracked.
-  LocIdx getOrTrackSpillLoc(SpillLoc L) {
-    unsigned SpillID = SpillLocs.idFor(L);
-    if (SpillID == 0) {
-      SpillID = SpillLocs.insert(L);
-      unsigned L = getLocID(SpillID, true);
+  LocIdx getOrTrackSpillLoc(SpillLoc L, unsigned ReqdSubReg) {
+    SpillLocationNo SpillID(SpillLocs.idFor(L));
+    if (SpillID.id() == 0) {
+      SpillID = SpillLocationNo(SpillLocs.insert(L));
+      for (unsigned SubReg = 0; SubReg < NumSubRegs; ++SubReg) {
+        unsigned L = getLocID(SpillID, SubReg);
         LocIdx Idx = LocIdx(LocIdxToIDNum.size()); // New idx
         LocIdxToIDNum.grow(Idx);
         LocIdxToLocID.grow(Idx);
         LocIDToLocIdx.push_back(Idx);
         LocIdxToLocID[Idx] = L;
-      return Idx;
+        LocIdxToIDNum[Idx] = ValueIDNum(CurBB, 0, Idx); // Set as MPhi
+      }
+      unsigned L = getLocID(SpillID, ReqdSubReg);
+      return LocIDToLocIdx[L];
     } else {
-      unsigned L = getLocID(SpillID, true);
+      unsigned L = getLocID(SpillID, ReqdSubReg);
       LocIdx Idx = LocIDToLocIdx[L];
       return Idx;
     }
   }
 
   /// Set the value stored in a spill slot.
-  void setSpill(SpillLoc L, ValueIDNum ValueID) {
-    LocIdx Idx = getOrTrackSpillLoc(L);
+  void setSpill(SpillLoc L, unsigned SubReg, ValueIDNum ValueID) {
+    LocIdx Idx = getOrTrackSpillLoc(L, SubReg);
     LocIdxToIDNum[Idx] = ValueID;
   }
 
   /// Read whatever value is in a spill slot, or None if it isn't tracked.
-  Optional<ValueIDNum> readSpill(SpillLoc L) {
-    unsigned SpillID = SpillLocs.idFor(L);
-    if (SpillID == 0)
+  Optional<ValueIDNum> readSpill(SpillLoc L, unsigned SubReg) {
+    SpillLocationNo SpillID(SpillLocs.idFor(L));
+    if (SpillID.id() == 0)
       return None;
 
-    unsigned LocID = getLocID(SpillID, true);
+    unsigned LocID = getLocID(SpillID, SubReg);
     LocIdx Idx = LocIDToLocIdx[LocID];
     return LocIdxToIDNum[Idx];
   }
 
   /// Determine the LocIdx of a spill slot. Return None if it previously
   /// hasn't had a value assigned.
-  Optional<LocIdx> getSpillMLoc(SpillLoc L) {
-    unsigned SpillID = SpillLocs.idFor(L);
-    if (SpillID == 0)
+  Optional<LocIdx> getSpillMLoc(SpillLoc L, unsigned SubReg) {
+    SpillLocationNo SpillID(SpillLocs.idFor(L));
+    if (SpillID.id() == 0)
       return None;
-    unsigned LocNo = getLocID(SpillID, true);
+    unsigned LocNo = getLocID(SpillID, SubReg);
     return LocIDToLocIdx[LocNo];
   }
 
   /// Return true if Idx is a spill machine location.
   bool isSpill(LocIdx Idx) const {
     return LocIdxToLocID[Idx] >= NumRegs;
   }
 
   MLocIterator begin() {
     return MLocIterator(LocIdxToIDNum, 0);
   }
 
   MLocIterator end() {
     return MLocIterator(LocIdxToIDNum, LocIdxToIDNum.size());
   }
 
   /// Return a range over all locations currently tracked.
   iterator_range<MLocIterator> locations() {
     return llvm::make_range(begin(), end());
   }
 
   std::string LocIdxToName(LocIdx Idx) const {
     unsigned ID = LocIdxToLocID[Idx];
-    if (ID >= NumRegs)
-      return Twine("slot ").concat(Twine(ID - NumRegs)).str();
-    else
+    if (ID >= NumRegs) {
+      ID -= NumRegs;
+      unsigned Slot = ID / NumSubRegs;
+      unsigned SubReg = ID % NumSubRegs;
+      return Twine("slot ").concat(Twine(Slot).concat(Twine(" subreg ")
+                           .concat(Twine(SubReg)))).str();
+    } else {
       return TRI.getRegAsmName(ID).str();
     }
+  }
 
   std::string IDAsString(const ValueIDNum &Num) const {
     std::string DefName = LocIdxToName(Num.getLoc());
     return Num.asString(DefName);
   }
 
   LLVM_DUMP_METHOD
   void dump() {
MachineInstrBuilder emitLoc(Optional<LocIdx> MLoc, const DebugVariable &Var,
 
     const DIExpression *Expr = Properties.DIExpr;
     if (!MLoc) {
       // No location -> DBG_VALUE $noreg
       MIB.addReg(0, RegState::Debug);
       MIB.addReg(0, RegState::Debug);
     } else if (LocIdxToLocID[*MLoc] >= NumRegs) {
       unsigned LocID = LocIdxToLocID[*MLoc];
-      const SpillLoc &Spill = SpillLocs[LocID - NumRegs + 1];
-
+      unsigned SpillID = locIDToSpill(LocID);
+      unsigned SubReg = locIDToSpillSubReg(LocID);
       auto *TRI = MF.getSubtarget().getRegisterInfo();
+
+      // TODO: support variables that are located in spill slots, with non-zero
+      // offsets from the start of the spill slot. It would require some more
+      // complex DIExpression calculations. This doesn't seem to be produced by
+      // LLVM right now, so don't try and support it.
+      // Accept no-subregister slots and subregisters where the offset is zero.
+      // The consumer should already have type information to work out how large
+      // the variable is.
+      if (SubReg == 0 || TRI->getSubRegIdxOffset(SubReg) == 0) {
+        const SpillLoc &Spill = SpillLocs[SpillID];
         Expr = TRI->prependOffsetExpression(Expr, DIExpression::ApplyOffset,
                                             Spill.SpillOffset);
         unsigned Base = Spill.SpillBase;
         MIB.addReg(Base, RegState::Debug);
         MIB.addImm(0);
       } else {
+        // If this is a subregister we don't currently describe, emit an undef
+        // DBG_VALUE instead.
+        MIB.addReg(0, RegState::Debug);
+        MIB.addReg(0, RegState::Debug);
+      }
+    } else {
+      // Non-empty, non-stack slot, must be a plain register.
       unsigned LocID = LocIdxToLocID[*MLoc];
       MIB.addReg(LocID, RegState::Debug);
       if (Properties.Indirect)
         MIB.addImm(0);
       else
         MIB.addReg(0, RegState::Debug);
     }
 
if (!MO.isReg() || MO.getReg() == 0) {
         ActiveVLocs.erase(It);
      }
       // Any use-before-defs no longer apply.
       UseBeforeDefVariables.erase(Var);
       return;
     }
 
     Register Reg = MO.getReg();
-    LocIdx NewLoc = MTracker->getRegMLoc(Reg);
+    LocIdx NewLoc = *MTracker->getRegMLoc(Reg);
     redefVar(MI, Properties, NewLoc);
   }
 
   /// Handle a change in variable location within a block. Terminate the
   /// variables current location, and record the value it now refers to, so
   /// that we can detect location transfers later on.
   void redefVar(const MachineInstr &MI, const DbgValueProperties &Properties,
                 Optional<LocIdx> OptNewLoc) {
private:
   /// and set \p Reg to the spilled register.
   Optional<SpillLoc> isRestoreInstruction(const MachineInstr &MI,
                                           MachineFunction *MF, unsigned &Reg);
 
   /// Given a spill instruction, extract the register and offset used to
   /// address the spill slot in a target independent way.
   SpillLoc extractSpillBaseRegAndOffset(const MachineInstr &MI);
 
+  /// Fetches: the largest super-register of BaseReg that will fit in the slot.
+  /// This allows us to identify all of the subreg indexes that might live in
+  /// the slot, in case a small subreg is spilt on top of a larger register.
+  /// \param FI Frame index of the relevant stack slot.
+  /// \param BaseReg Register, possibly a sub-register, that is spilt into the
+  ///        stack slot.
+  Register getStackSlotMaxSuperReg(const MachineFunction &MF, int FI,
+                                   Register BaseReg) const;
+
   /// Observe a single instruction while stepping through a block.
   void process(MachineInstr &MI, ValueIDNum **MLiveOuts = nullptr,
                ValueIDNum **MLiveIns = nullptr);
 
   /// Examines whether \p MI is a DBG_VALUE and notifies trackers.
   /// \returns true if MI was recognized and processed.
   bool transferDebugValue(const MachineInstr &MI);
 
if (InstrIt != DebugInstrNumToInstr.end()) {
 
     // Pick out the designated operand.
     assert(OpNo < TargetInstr.getNumOperands());
     const MachineOperand &MO = TargetInstr.getOperand(OpNo);
 
     // Today, this can only be a register.
     assert(MO.isReg() && MO.isDef());
 
-    unsigned LocID = MTracker->getLocID(MO.getReg(), false);
+    unsigned LocID = MTracker->getLocID(MO.getReg());
     LocIdx L = MTracker->LocIDToLocIdx[LocID];
     NewID = ValueIDNum(BlockNo, InstrIt->second.second, L);
   } else if (PHIIt != DebugPHINumToValue.end() && PHIIt->InstrNum == InstNo) {
     // It's actually a PHI value. Which value it is might not be obvious, use
     // the resolver helper to find out.
     NewID = resolveDbgPHIs(*MI.getParent()->getParent(), MLiveOuts, MLiveIns,
                            MI, InstNo);
   }
if (MO.isReg()) {
     auto PHIRec = DebugPHIRecord(
         {InstrNum, MI.getParent(), Num, MTracker->lookupOrTrackRegister(Reg)});
     DebugPHINumToValue.push_back(PHIRec);
   } else {
     // The value is whatever's in this stack slot.
     assert(MO.isFI());
     unsigned FI = MO.getIndex();
 
+    // We could track a subregister within a slot where a PHI occurs...
+    // however it's not clear that this ever happens. It should be Easy (TM)
+    // to express that if it happens. In the mean time, treat the DBG_PHI as
+    // always referring to the whole slot, with no subreg.
+    unsigned SubReg = 0;
+
     // If the stack slot is dead, then this was optimized away.
     // FIXME: stack slot colouring should account for slots that get merged.
     if (MFI->isDeadObjectIndex(FI))
       return true;
 
     // Identify this spill slot.
     Register Base;
     StackOffset Offs = TFI->getFrameIndexReference(*MI.getMF(), FI, Base);
     SpillLoc SL = {Base, Offs};
-    Optional<ValueIDNum> Num = MTracker->readSpill(SL);
+    Optional<ValueIDNum> Num = MTracker->readSpill(SL, SubReg);
 
     if (!Num)
       // Nothing ever writes to this slot. Curious, but nothing we can do.
       return true;
 
     // Record this DBG_PHI for later analysis.
     auto DbgPHI = DebugPHIRecord(
-        {InstrNum, MI.getParent(), *Num, *MTracker->getSpillMLoc(SL)});
+        {InstrNum, MI.getParent(), *Num, *MTracker->getSpillMLoc(SL, SubReg)});
     DebugPHINumToValue.push_back(DbgPHI);
   }
 
   return true;
 }
 
 void InstrRefBasedLDV::transferRegisterDef(MachineInstr &MI) {
   // Meta Instructions do not affect the debug liveness of any register they
for (MCSubRegIndexIterator SRI(SrcRegNum, TRI); SRI.isValid(); ++SRI) {
     if (!DstSubReg)
       continue;
 
     // Do copy. There are two matching subregisters, the source value should
     // have been def'd when the super-reg was, the latter might not be tracked
     // yet.
     // This will force SrcSubReg to be tracked, if it isn't yet.
     (void)MTracker->readReg(SrcSubReg);
-    LocIdx SrcL = MTracker->getRegMLoc(SrcSubReg);
-    assert(SrcL.asU64());
+    LocIdx SrcL = *MTracker->getRegMLoc(SrcSubReg);
     (void)MTracker->readReg(DstSubReg);
-    LocIdx DstL = MTracker->getRegMLoc(DstSubReg);
-    assert(DstL.asU64());
-    (void)DstL;
     ValueIDNum CpyValue = {SrcValue.getBlock(), SrcValue.getInst(), SrcL};
 
     MTracker->setReg(DstSubReg, CpyValue);
   }
 }
 
 bool InstrRefBasedLDV::isSpillInstruction(const MachineInstr &MI,
                                           MachineFunction *MF) {
InstrRefBasedLDV::isRestoreInstruction(const MachineInstr &MI,
   // operand.
   if (MI.getRestoreSize(TII)) {
     Reg = MI.getOperand(0).getReg();
     return extractSpillBaseRegAndOffset(MI);
   }
   return None;
 }
 
+Register
+InstrRefBasedLDV::getStackSlotMaxSuperReg(const MachineFunction &MF, int FI, Register BaseReg) const {
+  int64_t SlotSize = MFI->getObjectSize(FI);
+
+  // Find the greatest super register.
+  Register SuperReg = BaseReg;
+  do {
+    auto Range = TRI->superregs(SuperReg);
+    if (Range.empty())
+      break;
+
+    // Will the super-register of the current register fit in the stack slot?
+    Register Tmp = *Range.begin();
+    unsigned Size = TRI->getRegSizeInBits(Tmp, MF.getRegInfo()) / 8;
+    if (Size > SlotSize)
+      // It's bigger; stop exploring.
+      break;
+
+    SuperReg = *Range.begin();
+  } while (true);
+
+  // We assume that only zero-offset subregisters get spilt; for example on
+  // x86, if there's a slot that'll fit $rax, LLVM won't spill $ah into it.
+  unsigned Subreg = TRI->getSubRegIndex(SuperReg, BaseReg);
+  assert(!Subreg || !TRI->getSubRegIdxOffset(Subreg));
+
+  return SuperReg;
+}
+
 bool InstrRefBasedLDV::transferSpillOrRestoreInst(MachineInstr &MI) {
   // XXX -- it's too difficult to implement VarLocBasedImpl's  stack location
   // limitations under the new model. Therefore, when comparing them, compare
   // versions that don't attempt spills or restores at all.
   if (EmulateOldLDV)
     return false;
 
+  // Strictly limit ourselves to plain loads and stores, not all instructions
+  // that can access the stack. Fetch the frame index too.
+  int FI = -1;
+  if (!TII->isStoreToStackSlotPostFE(MI, FI) &&
+      !TII->isLoadFromStackSlotPostFE(MI, FI))
+     return false;
+
   MachineFunction *MF = MI.getMF();
   unsigned Reg;
   Optional<SpillLoc> Loc;
 
   LLVM_DEBUG(dbgs() << "Examining instruction: "; MI.dump(););
 
   // First, if there are any DBG_VALUEs pointing at a spill slot that is
   // written to, terminate that variable location. The value in memory
   // will have changed. DbgEntityHistoryCalculator doesn't try to detect this.
   if (isSpillInstruction(MI, MF)) {
     Loc = extractSpillBaseRegAndOffset(MI);
 
     if (TTracker) {
-      Optional<LocIdx> MLoc = MTracker->getSpillMLoc(*Loc);
-      if (MLoc)
+      // Terminate all subregister fields in the slot too.
+      for (unsigned SubReg = 0; SubReg < MTracker->NumSubRegs; ++SubReg) {
+        if (Optional<LocIdx> MLoc = MTracker->getSpillMLoc(*Loc, SubReg))
           TTracker->clobberMloc(*MLoc, MI.getIterator());
       }
     }
+  }
 
   // Try to recognise spill and restore instructions that may transfer a value.
   if (isLocationSpill(MI, MF, Reg)) {
     Loc = extractSpillBaseRegAndOffset(MI);
     auto ValueID = MTracker->readReg(Reg);
 
+    // Find the greatest super register for this slot.
+    Register GreatestSubreg = getStackSlotMaxSuperReg(*MF, FI, Reg);
+
     // If the location is empty, produce a phi, signify it's the live-in value.
     if (ValueID.getLoc() == 0)
-      ValueID = {CurBB, 0, MTracker->getRegMLoc(Reg)};
+      ValueID = {CurBB, 0, *MTracker->getRegMLoc(Reg)};
+
+    // Ensure tracked.
+    MTracker->getOrTrackSpillLoc(*Loc, 0);
 
-    MTracker->setSpill(*Loc, ValueID);
-    auto OptSpillLocIdx = MTracker->getSpillMLoc(*Loc);
-    assert(OptSpillLocIdx && "Spill slot set but has no LocIdx?");
-    LocIdx SpillLocIdx = *OptSpillLocIdx;
+    // First def all the subregs,
+    for (unsigned SubReg = 0; SubReg < MTracker->NumSubRegs; ++SubReg) {
+      ValueIDNum Clob = {CurBB, CurInst, *MTracker->getSpillMLoc(*Loc, SubReg)};
+      MTracker->setSpill(*Loc, SubReg, Clob);
+    }
+
+    // Then, transfer subreg bits.
+    for (MCSubRegIterator foo(Reg, TRI, true); foo.isValid(); ++foo) {
+      unsigned Subreg = TRI->getSubRegIndex(GreatestSubreg, *foo);
+      // Risk that this reg isn't yet tracked,
+      LocIdx SrcIdx = MTracker->lookupOrTrackRegister(*foo);
+      ValueIDNum ValTwo = MTracker->readReg(*foo);
+      MTracker->setSpill(*Loc, Subreg, ValTwo);
+      LocIdx DstLoc = *MTracker->getSpillMLoc(*Loc, Subreg);
 
-    // Tell TransferTracker about this spill, produce DBG_VALUEs for it.
       if (TTracker)
-      TTracker->transferMlocs(MTracker->getRegMLoc(Reg), SpillLocIdx,
-                              MI.getIterator());
+        TTracker->transferMlocs(SrcIdx, DstLoc, MI.getIterator());
+    }
   } else {
     if (!(Loc = isRestoreInstruction(MI, MF, Reg)))
       return false;
 
     // Is there a value to be restored?
-    auto OptValueID = MTracker->readSpill(*Loc);
+    auto OptValueID = MTracker->readSpill(*Loc, 0);
     if (OptValueID) {
-      ValueIDNum ValueID = *OptValueID;
-      LocIdx SpillLocIdx = *MTracker->getSpillMLoc(*Loc);
-      // XXX -- can we recover sub-registers of this value? Until we can, first
-      // overwrite all defs of the register being restored to.
+      // Assumption: we're reading from the base of the stack slot, not some
+      // offset into it. This then becomes a question of what subregisters line
+      // up with that point.
+
+      // Find the greatest super register.
+      Register GreatestSubreg = getStackSlotMaxSuperReg(*MF, FI, Reg);
+
+      // Def all aliasing values,
       for (MCRegAliasIterator RAI(Reg, TRI, true); RAI.isValid(); ++RAI)
         MTracker->defReg(*RAI, CurBB, CurInst);
 
-      // Now override the reg we're restoring to.
-      MTracker->setReg(Reg, ValueID);
+      ValueIDNum ValueID = *OptValueID;
+
+      // Now load all subregs.
+      for (MCSubRegIterator foo(Reg, TRI, true); foo.isValid(); ++foo) {
+        unsigned Subreg = TRI->getSubRegIndex(GreatestSubreg, *foo);
+        LocIdx SrcIdx = *MTracker->getRegMLoc(*foo);
+        ValueIDNum ValTwo = {ValueID.getBlock(), ValueID.getInst(), SrcIdx};
+        auto ReadValue = *MTracker->readSpill(*Loc, Subreg);
+        MTracker->setReg(*foo, ReadValue);
+        LocIdx DstLoc = *MTracker->getSpillMLoc(*Loc, Subreg);
 
-      // Report this restore to the transfer tracker too.
         if (TTracker)
-        TTracker->transferMlocs(SpillLocIdx, MTracker->getRegMLoc(Reg),
-                                MI.getIterator());
+          TTracker->transferMlocs(SrcIdx, DstLoc, MI.getIterator());
+      }
     } else {
       // There isn't anything in the location; not clear if this is a code path
       // that still runs. Def this register anyway just in case.
       for (MCRegAliasIterator RAI(Reg, TRI, true); RAI.isValid(); ++RAI)
         MTracker->defReg(*RAI, CurBB, CurInst);
 
-      // Force the spill slot to be tracked.
-      LocIdx L = MTracker->getOrTrackSpillLoc(*Loc);
-
-      // Set the restored value to be a machine phi number, signifying that it's
-      // whatever the spills live-in value is in this block. Definitely has
-      // a LocIdx due to the setSpill above.
-      ValueIDNum ValueID = {CurBB, 0, L};
-      MTracker->setReg(Reg, ValueID);
-      MTracker->setSpill(*Loc, ValueID);
+      // Force the spill slot to be tracked. Pass in zero subreg, getOrTrack..
+      // will create all the subreg points.
+      (void)MTracker->getOrTrackSpillLoc(*Loc, 0);
     }
   }
   return true;
 }
 
 bool InstrRefBasedLDV::transferRegisterCopy(MachineInstr &MI) {
   auto DestSrc = TII->isCopyInstr(MI);
   if (!DestSrc)
bool InstrRefBasedLDV::transferRegisterCopy(MachineInstr &MI) {
 
   // Copy MTracker info, including subregs if available.
   InstrRefBasedLDV::performCopy(SrcReg, DestReg);
 
   // Only produce a transfer of DBG_VALUE within a block where old LDV
   // would have. We might make use of the additional value tracking in some
   // other way, later.
   if (TTracker && isCalleeSavedReg(DestReg) && SrcRegOp->isKill())
-    TTracker->transferMlocs(MTracker->getRegMLoc(SrcReg),
-                            MTracker->getRegMLoc(DestReg), MI.getIterator());
+    TTracker->transferMlocs(*MTracker->getRegMLoc(SrcReg),
+                            *MTracker->getRegMLoc(DestReg), MI.getIterator());
 
   // VarLocBasedImpl would quit tracking the old location after copying.
   if (EmulateOldLDV && SrcReg != DestReg)
     MTracker->defReg(SrcReg, CurBB, CurInst);
 
   // Finally, the copy might have clobbered variables based on the destination
   // register. Tell TTracker about it, in case a backup location exists.
   if (TTracker) {
     for (MCRegAliasIterator RAI(DestReg, TRI, true); RAI.isValid(); ++RAI) {
-      LocIdx ClobberedLoc = MTracker->getRegMLoc(*RAI);
-      TTracker->clobberMloc(ClobberedLoc, MI.getIterator(), false);
+      if (auto OptClobberedLoc = MTracker->getRegMLoc(*RAI)) {
+        TTracker->clobberMloc(*OptClobberedLoc, MI.getIterator(), false);
+      }
     }
   }
 
   return true;
 }
 
 /// Accumulate a mapping between each DILocalVariable fragment and other
 /// fragments of that DILocalVariable which overlap. This reduces work during
void InstrRefBasedLDV::produceMLocTransferFunction(
   for (unsigned int I = 0; I < MaxNumBlocks; ++I) {
     BitVector &BV = BlockMasks[I];
     BV.flip();
     BV &= UsedRegs;
     // This produces all the bits that we clobber, but also use. Check that
     // they're all clobbered or at least set in the designated transfer
     // elem.
     for (unsigned Bit : BV.set_bits()) {
-      unsigned ID = MTracker->getLocID(Bit, false);
+      unsigned ID = MTracker->getLocID(Bit);
       LocIdx Idx = MTracker->LocIDToLocIdx[ID];
       auto &TransferMap = MLocTransfer[I];
 
       // Install a value representing the fact that this location is effectively
       // written to in this block. As there's no reserved value, instead use
       // a value number that is never generated. Pick the value number for the
       // first instruction in the block, def'ing this location, which we know
       // this block never used anyway.

llvm/test/DebugInfo/MIR/AArch64/subreg-in-spill-transfer.mir

+# RUN: llc %s -run-pass=livedebugvalues -march=aarch64 -o - \
+# RUN:     -experimental-debug-variable-locations | FileCheck %s
+#
+# Test that a spill then restore of a variable in a subregister is tracked by
+# LiveDebugValues through the stack. Similar to recognise-spill-restore.mir,
+# but $w8 is written too then $x8 stored / restored.
+#
+# CHECK: DBG_VALUE $sp, 0, {{.*}} !DIExpression(DW_OP_plus_uconst, 8),
+# CHECK: DBG_VALUE $w0, $noreg
+--- |
+  ; ModuleID = 'test.ll'
+  source_filename = "test.ll"
+  target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+  
+  define i64 @test(i64 %foo) !dbg !7 {
+    ret i64 0, !dbg !12
+  }
+  
+  ; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+  declare void @llvm.dbg.value(metadata, metadata, metadata)
+  
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!3, !4, !5}
+  !llvm.ident = !{!6}
+  
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, nameTableKind: None)
+  !1 = !DIFile(filename: "test.c", directory: "/tmp/out.c")
+  !2 = !{}
+  !3 = !{i32 7, !"Dwarf Version", i32 4}
+  !4 = !{i32 2, !"Debug Info Version", i32 3}
+  !5 = !{i32 1, !"wchar_size", i32 4}
+  !6 = !{!""}
+  !7 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 3, type: !8, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+  !8 = !DISubroutineType(types: !9)
+  !9 = !{!10, !11, !11}
+  !10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !11 = !DIBasicType(name: "long int", size: 64, encoding: DW_ATE_signed)
+  !12 = !DILocation(line: 0, scope: !7)
+  !13 = !{i32 1}
+  !14 = !DILocalVariable(name: "bar", arg: 1, scope: !7, file: !1, line: 3, type: !11)
+
+...
+---
+name:            test
+alignment:       4
+tracksRegLiveness: true
+liveins:
+  - { reg: '$x0' }
+frameInfo:
+  stackSize:       112
+  maxAlignment:    8
+  maxCallFrameSize: 0
+stack:
+  - { id: 0, type: spill-slot, offset: -104, size: 8, alignment: 8 }
+  - { id: 1, type: spill-slot, offset: -8, size: 8, alignment: 8, callee-saved-register: '$x19' }
+  - { id: 2, type: spill-slot, offset: -16, size: 8, alignment: 8, callee-saved-register: '$x20' }
+  - { id: 3, type: spill-slot, offset: -24, size: 8, alignment: 8, callee-saved-register: '$x21' }
+  - { id: 4, type: spill-slot, offset: -32, size: 8, alignment: 8, callee-saved-register: '$x22' }
+  - { id: 5, type: spill-slot, offset: -40, size: 8, alignment: 8, callee-saved-register: '$x23' }
+  - { id: 6, type: spill-slot, offset: -48, size: 8, alignment: 8, callee-saved-register: '$x24' }
+  - { id: 7, type: spill-slot, offset: -56, size: 8, alignment: 8, callee-saved-register: '$x25' }
+  - { id: 8, type: spill-slot, offset: -64, size: 8, alignment: 8, callee-saved-register: '$x26' }
+  - { id: 9, type: spill-slot, offset: -72, size: 8, alignment: 8, callee-saved-register: '$x27' }
+  - { id: 10, type: spill-slot, offset: -80, size: 8, alignment: 8, callee-saved-register: '$x28' }
+  - { id: 11, type: spill-slot, offset: -88, size: 8, alignment: 8, callee-saved-register: '$lr' }
+  - { id: 12, type: spill-slot, offset: -96, size: 8, alignment: 8, callee-saved-register: '$fp' }
+machineFunctionInfo:
+  hasRedZone:      false
+body:             |
+  bb.0 (%ir-block.0):
+    liveins: $x0, $lr, $fp, $x27, $x28, $x25, $x26, $x23, $x24, $x21, $x22, $x19, $x20
+  
+    $sp = frame-setup SUBXri $sp, 112, 0
+    frame-setup STPXi killed $fp, killed $lr, $sp, 2 :: (store 8 into %stack.12), (store 8 into %stack.11)
+    frame-setup STPXi killed $x28, killed $x27, $sp, 4 :: (store 8 into %stack.10), (store 8 into %stack.9)
+    frame-setup STPXi killed $x26, killed $x25, $sp, 6 :: (store 8 into %stack.8), (store 8 into %stack.7)
+    frame-setup STPXi killed $x24, killed $x23, $sp, 8 :: (store 8 into %stack.6), (store 8 into %stack.5)
+    frame-setup STPXi killed $x22, killed $x21, $sp, 10 :: (store 8 into %stack.4), (store 8 into %stack.3)
+    frame-setup STPXi killed $x20, killed $x19, $sp, 12 :: (store 8 into %stack.2), (store 8 into %stack.1)
+    frame-setup CFI_INSTRUCTION def_cfa_offset 112
+    frame-setup CFI_INSTRUCTION offset $w19, -8, debug-location !12
+    frame-setup CFI_INSTRUCTION offset $w20, -16, debug-location !12
+    frame-setup CFI_INSTRUCTION offset $w21, -24, debug-location !12
+    frame-setup CFI_INSTRUCTION offset $w22, -32, debug-location !12
+    frame-setup CFI_INSTRUCTION offset $w23, -40, debug-location !12
+    frame-setup CFI_INSTRUCTION offset $w24, -48, debug-location !12
+    frame-setup CFI_INSTRUCTION offset $w25, -56, debug-location !12
+    frame-setup CFI_INSTRUCTION offset $w26, -64, debug-location !12
+    frame-setup CFI_INSTRUCTION offset $w27, -72, debug-location !12
+    frame-setup CFI_INSTRUCTION offset $w28, -80, debug-location !12
+    frame-setup CFI_INSTRUCTION offset $w30, -88, debug-location !12
+    frame-setup CFI_INSTRUCTION offset $w29, -96, debug-location !12
+    renamable $w8 = ADDWri killed renamable $w0, 1, 0, debug-instr-number 1, debug-location !12
+    STRXui killed renamable $x8, $sp, 1 :: (store 8 into %stack.0)
+    ; Deleted inlineasm clobbering things here for brevity,
+    DBG_INSTR_REF 1, 0, !14, !DIExpression(), debug-location !12
+    $x0 = LDRXui $sp, 1 :: (load 8 from %stack.0)
+    ; This store added to evict the variable value from the stack, forces
+    ; LiveDebugValues to relocate it to $x0
+    STRXui $xzr, $sp, 1 :: (store 8 into %stack.0)
+    $x20, $x19 = frame-destroy LDPXi $sp, 12, debug-location !12 :: (load 8 from %stack.2), (load 8 from %stack.1)
+    $x22, $x21 = frame-destroy LDPXi $sp, 10, debug-location !12 :: (load 8 from %stack.4), (load 8 from %stack.3)
+    $x24, $x23 = frame-destroy LDPXi $sp, 8, debug-location !12 :: (load 8 from %stack.6), (load 8 from %stack.5)
+    $x26, $x25 = frame-destroy LDPXi $sp, 6, debug-location !12 :: (load 8 from %stack.8), (load 8 from %stack.7)
+    $x28, $x27 = frame-destroy LDPXi $sp, 4, debug-location !12 :: (load 8 from %stack.10), (load 8 from %stack.9)
+    $fp, $lr = frame-destroy LDPXi $sp, 2, debug-location !12 :: (load 8 from %stack.12), (load 8 from %stack.11)
+    $sp = frame-destroy ADDXri $sp, 112, 0, debug-location !12
+    RET undef $lr, implicit killed $x0, debug-location !12
+
+...

llvm/test/DebugInfo/MIR/InstrRef/livedebugvalues_stackslot_subregs.mir

+# RUN: llc %s -march=x86-64 -run-pass=livedebugvalues -experimental-debug-variable-locations -o - 2>&1 | FileCheck %s
+#
+# Test that we can spill and restore through subregisters too. In this test,
+# eax is def'd and then spilt, but as part of a larger register.
+--- |
+  define i8 @test(i32 %bar) local_unnamed_addr !dbg !7 {
+  entry:
+    ret i8 0, !dbg !12
+  }
+
+  declare dso_local void @ext(i64)
+
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!3, !4, !5, !6}
+  !0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+  !1 = !DIFile(filename: "foo.cpp", directory: ".")
+  !2 = !DIBasicType(name: "int", size: 8, encoding: DW_ATE_signed)
+  !3 = !{i32 2, !"Dwarf Version", i32 4}
+  !4 = !{i32 2, !"Debug Info Version", i32 3}
+  !5 = !{i32 1, !"wchar_size", i32 2}
+  !6 = !{i32 7, !"PIC Level", i32 2}
+  !7 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: !1, file: !1, line: 6, type: !8, scopeLine: 6, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !10)
+  !8 = !DISubroutineType(types: !9)
+  !9 = !{!2, !2}
+  !10 = !{!11}
+  !11 = !DILocalVariable(name: "baz", scope: !7, file: !1, line: 7, type: !2)
+  !12 = !DILocation(line: 10, scope: !7)
+...
+---
+name: test
+tracksRegLiveness: true
+liveins:
+  - { reg: '$rdi', virtual-reg: '' }
+stack:
+  - { id: 0, name: '', type: spill-slot, offset: -16, size: 8, alignment: 8,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+body:  |
+  bb.0:
+  liveins: $rdi, $rax, $rbx
+    $eax = MOV32ri 0, debug-instr-number 1
+    $edi = COPY $eax
+    MOV64mr $rsp, 1, $noreg, 16, $noreg, $rdi :: (store 8 into %stack.0)
+    $rsi = MOV64rm $rsp, 1, $noreg, 8, $noreg :: (load 8 from %stack.0)
+
+    ; Store a random value onto stack, forces value to be in one place only.
+    ; Clobber other registers that contain the value we're to track.
+    MOV64mr $rsp, 1, $noreg, 16, $noreg, $rbx :: (store 8 into %stack.0)
+    $rax = MOV64ri 0
+    $rdi = MOV64ri 0
+
+    DBG_INSTR_REF 1, 0, !11, !DIExpression(), debug-location !12
+    ; CHECK:      DBG_INSTR_REF
+    ; CHECK-NEXT: DBG_VALUE $esi
+    RETQ $rsi, debug-location !12
+...