[DebugInfo][Instr] Track subregisters across stack spills/restores
authorJeremy Morse <jeremy.morse@sony.com>
Fri, 22 Oct 2021 18:10:05 +0000 (19:10 +0100)
committerJeremy Morse <jeremy.morse@sony.com>
Fri, 22 Oct 2021 18:20:55 +0000 (19:20 +0100)
Sometimes we 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

This patch takes a different approach: it adds another index to
MLocTracker that identifies a size/offset within a stack slot. A location
on the stack is then a pari of {FrameIndex, SlotNum}. Spilling and
restoring now involves pairing up the src/dest register numbers, and the
dest/src stack position to be transferred to/from. Location coverage
improves as a result, compile-time performance decreases, alas.

One limitation is that if a PHI occurs inside a stack slot:

    DBG_PHI %stack.0, 1

We don't know how large the resulting value is, and so might have
difficulty picking which value to use. DBG_PHI might need to be augmented
in the future with such a size.

Unit tests added ensure that spills and restores correctly transfer to
positions in the Location => Value map, and that different register classes
written to the stack will correctly clobber all other positions in the
stack slot.

Differential Revision: https://reviews.llvm.org/D112133

llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
llvm/test/DebugInfo/MIR/InstrRef/livedebugvalues_stackslot_subregs.mir [new file with mode: 0644]
llvm/test/DebugInfo/MIR/InstrRef/livedebugvalues_subreg_substitutions.mir
llvm/unittests/CodeGen/InstrRefLDVTest.cpp

index 4f151d7..7fd023d 100644 (file)
@@ -148,15 +148,6 @@ static cl::opt<bool> EmulateOldLDV("emulate-old-livedebugvalues", cl::Hidden,
                                    cl::desc("Act like old LiveDebugValues did"),
                                    cl::init(false));
 
-/// 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 converting machine value locations and variable values into
 /// variable locations (the output of LiveDebugValues), recorded as DBG_VALUEs
 /// specifying block live-in locations and transfers within blocks.
@@ -671,12 +662,43 @@ MLocTracker::MLocTracker(MachineFunction &MF, const TargetInstrInfo &TII,
   // regmasks that claim to clobber SP).
   Register SP = TLI.getStackPointerRegisterToSaveRestore();
   if (SP) {
-    unsigned ID = getLocID(SP, false);
+    unsigned ID = getLocID(SP);
     (void)lookupOrTrackRegister(ID);
 
     for (MCRegAliasIterator RAI(SP, &TRI, true); RAI.isValid(); ++RAI)
       SPAliases.insert(*RAI);
   }
+
+  // Build some common stack positions -- full registers being spilt to the
+  // stack.
+  StackSlotIdxes.insert({{8, 0}, 0});
+  StackSlotIdxes.insert({{16, 0}, 1});
+  StackSlotIdxes.insert({{32, 0}, 2});
+  StackSlotIdxes.insert({{64, 0}, 3});
+  StackSlotIdxes.insert({{128, 0}, 4});
+  StackSlotIdxes.insert({{256, 0}, 5});
+  StackSlotIdxes.insert({{512, 0}, 6});
+
+  // Traverse all the subregister idxes, and ensure there's an index for them.
+  // Duplicates are no problem: we're interested in their position in the
+  // stack slot, we don't want to type the slot.
+  for (unsigned int I = 1; I < TRI.getNumSubRegIndices(); ++I) {
+    unsigned Size = TRI.getSubRegIdxSize(I);
+    unsigned Offs = TRI.getSubRegIdxOffset(I);
+    unsigned Idx = StackSlotIdxes.size();
+
+    // Some subregs have -1, -2 and so forth fed into their fields, to mean
+    // special backend things. Ignore those.
+    if (Size > 60000 || Offs > 60000)
+      continue;
+
+    StackSlotIdxes.insert({{Size, Offs}, Idx});
+  }
+
+  for (auto &Idx : StackSlotIdxes)
+    StackIdxesToPos[Idx.second] = Idx.first;
+
+  NumSlotIdxes = StackSlotIdxes.size();
 }
 
 LocIdx MLocTracker::trackRegister(unsigned ID) {
@@ -715,30 +737,40 @@ void MLocTracker::writeRegMask(const MachineOperand *MO, unsigned CurBB,
   Masks.push_back(std::make_pair(MO, InstID));
 }
 
-LocIdx MLocTracker::getOrTrackSpillLoc(SpillLoc L) {
-  unsigned SpillID = SpillLocs.idFor(L);
-  if (SpillID == 0) {
-    SpillID = SpillLocs.insert(L);
-    unsigned L = getLocID(SpillID, true);
-    LocIdx Idx = LocIdx(LocIdxToIDNum.size()); // New idx
-    LocIdxToIDNum.grow(Idx);
-    LocIdxToLocID.grow(Idx);
-    LocIDToLocIdx.push_back(Idx);
-    LocIdxToLocID[Idx] = L;
-    return Idx;
-  } else {
-    unsigned L = getLocID(SpillID, true);
-    LocIdx Idx = LocIDToLocIdx[L];
-    return Idx;
+SpillLocationNo MLocTracker::getOrTrackSpillLoc(SpillLoc L) {
+  SpillLocationNo SpillID(SpillLocs.idFor(L));
+  if (SpillID.id() == 0) {
+    // Spill location is untracked: create record for this one, and all
+    // subregister slots too.
+    SpillID = SpillLocationNo(SpillLocs.insert(L));
+    for (unsigned StackIdx = 0; StackIdx < NumSlotIdxes; ++StackIdx) {
+      unsigned L = getSpillIDWithIdx(SpillID, StackIdx);
+      LocIdx Idx = LocIdx(LocIdxToIDNum.size()); // New idx
+      LocIdxToIDNum.grow(Idx);
+      LocIdxToLocID.grow(Idx);
+      LocIDToLocIdx.push_back(Idx);
+      LocIdxToLocID[Idx] = L;
+      // Initialize to PHI value; corresponds to the location's live-in value
+      // during transfer function construction.
+      LocIdxToIDNum[Idx] = ValueIDNum(CurBB, 0, Idx);
+    }
   }
+  return SpillID;
 }
 
 std::string MLocTracker::LocIdxToName(LocIdx Idx) const {
   unsigned ID = LocIdxToLocID[Idx];
-  if (ID >= NumRegs)
-    return Twine("slot ").concat(Twine(ID - NumRegs)).str();
-  else
+  if (ID >= NumRegs) {
+    StackSlotPos Pos = locIDToSpillIdx(ID);
+    ID -= NumRegs;
+    unsigned Slot = ID / NumSlotIdxes;
+    return Twine("slot ")
+        .concat(Twine(Slot).concat(Twine(" sz ").concat(Twine(Pos.first)
+        .concat(Twine(" offs ").concat(Twine(Pos.second))))))
+        .str();
+  } else {
     return TRI.getRegAsmName(ID).str();
+  }
 }
 
 std::string MLocTracker::IDAsString(const ValueIDNum &Num) const {
@@ -778,15 +810,32 @@ MachineInstrBuilder MLocTracker::emitLoc(Optional<LocIdx> MLoc,
     MIB.addReg(0);
   } else if (LocIdxToLocID[*MLoc] >= NumRegs) {
     unsigned LocID = LocIdxToLocID[*MLoc];
-    const SpillLoc &Spill = SpillLocs[LocID - NumRegs + 1];
-
-    auto *TRI = MF.getSubtarget().getRegisterInfo();
-    Expr = TRI->prependOffsetExpression(Expr, DIExpression::ApplyOffset,
-                                        Spill.SpillOffset);
-    unsigned Base = Spill.SpillBase;
-    MIB.addReg(Base);
-    MIB.addImm(0);
+    SpillLocationNo SpillID = locIDToSpill(LocID);
+    StackSlotPos StackIdx = locIDToSpillIdx(LocID);
+    unsigned short Offset = StackIdx.second;
+
+    // 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 (Offset == 0) {
+      const SpillLoc &Spill = SpillLocs[SpillID.id()];
+      Expr = TRI.prependOffsetExpression(Expr, DIExpression::ApplyOffset,
+                                         Spill.SpillOffset);
+      unsigned Base = Spill.SpillBase;
+      MIB.addReg(Base);
+      MIB.addImm(0);
+    } else {
+      // This is a stack location with a weird subregister offset: emit an undef
+      // DBG_VALUE instead.
+      MIB.addReg(0);
+      MIB.addReg(0);
+    }
   } else {
+    // Non-empty, non-stack slot, must be a plain register.
     unsigned LocID = LocIdxToLocID[*MLoc];
     MIB.addReg(LocID);
     if (Properties.Indirect)
@@ -820,7 +869,7 @@ bool InstrRefBasedLDV::isCalleeSaved(LocIdx L) const {
 // void InstrRefBasedLDV::printVarLocInMBB(..)
 #endif
 
-SpillLoc
+SpillLocationNo
 InstrRefBasedLDV::extractSpillBaseRegAndOffset(const MachineInstr &MI) {
   assert(MI.hasOneMemOperand() &&
          "Spill instruction does not have exactly one memory operand?");
@@ -832,7 +881,7 @@ InstrRefBasedLDV::extractSpillBaseRegAndOffset(const MachineInstr &MI) {
   const MachineBasicBlock *MBB = MI.getParent();
   Register Reg;
   StackOffset Offset = TFI->getFrameIndexReference(*MBB->getParent(), FI, Reg);
-  return {Reg, Offset};
+  return MTracker->getOrTrackSpillLoc({Reg, Offset});
 }
 
 /// End all previous ranges related to @MI and start a new range from @MI
@@ -968,7 +1017,7 @@ bool InstrRefBasedLDV::transferDebugInstrRef(MachineInstr &MI,
     // 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) {
@@ -1126,9 +1175,7 @@ bool InstrRefBasedLDV::transferDebugPHI(MachineInstr &MI) {
         {InstrNum, MI.getParent(), Num, MTracker->lookupOrTrackRegister(Reg)});
     DebugPHINumToValue.push_back(PHIRec);
 
-    // Subsequent register operations, or variable locations, might occur for
-    // any of the subregisters of this DBG_PHIs operand. Ensure that all
-    // registers aliasing this register are tracked.
+    // Ensure this register is tracked.
     for (MCRegAliasIterator RAI(MO.getReg(), TRI, true); RAI.isValid(); ++RAI)
       MTracker->lookupOrTrackRegister(*RAI);
   } else {
@@ -1141,19 +1188,46 @@ bool InstrRefBasedLDV::transferDebugPHI(MachineInstr &MI) {
     if (MFI->isDeadObjectIndex(FI))
       return true;
 
-    // Identify this spill slot.
+    // Identify this spill slot, ensure it's tracked.
     Register Base;
     StackOffset Offs = TFI->getFrameIndexReference(*MI.getMF(), FI, Base);
     SpillLoc SL = {Base, Offs};
-    Optional<ValueIDNum> Num = MTracker->readSpill(SL);
+    SpillLocationNo SpillNo = MTracker->getOrTrackSpillLoc(SL);
+
+    // Problem: what value should we extract from the stack? LLVM does not
+    // record what size the last store to the slot was, and it would become
+    // sketchy after stack slot colouring anyway. Take a look at what values
+    // are stored on the stack, and pick the largest one that wasn't def'd
+    // by a spill (i.e., the value most likely to have been def'd in a register
+    // and then spilt.
+    std::array<unsigned, 4> CandidateSizes = {64, 32, 16, 8};
+    Optional<ValueIDNum> Result = None;
+    Optional<LocIdx> SpillLoc = None;
+    for (unsigned int I = 0; I < CandidateSizes.size(); ++I) {
+      unsigned SpillID = MTracker->getLocID(SpillNo, {CandidateSizes[I], 0});
+      SpillLoc = MTracker->getSpillMLoc(SpillID);
+      ValueIDNum Val = MTracker->readMLoc(*SpillLoc);
+      // If this value was defined in it's own position, then it was probably
+      // an aliasing index of a small value that was spilt.
+      if (Val.getLoc() != SpillLoc->asU64()) {
+        Result = Val;
+        break;
+      }
+    }
 
-    if (!Num)
-      // Nothing ever writes to this slot. Curious, but nothing we can do.
-      return true;
+    // If we didn't find anything, we're probably looking at a PHI, or a memory
+    // store folded into an instruction. FIXME: Take a guess that's it's 64
+    // bits. This isn't ideal, but tracking the size that the spill is
+    // "supposed" to be is more complex, and benefits a small number of
+    // locations.
+    if (!Result) {
+      unsigned SpillID = MTracker->getLocID(SpillNo, {64, 0});
+      SpillLoc = MTracker->getSpillMLoc(SpillID);
+      Result = MTracker->readMLoc(*SpillLoc);
+    }
 
     // Record this DBG_PHI for later analysis.
-    auto DbgPHI = DebugPHIRecord(
-        {InstrNum, MI.getParent(), *Num, *MTracker->getSpillMLoc(SL)});
+    auto DbgPHI = DebugPHIRecord({InstrNum, MI.getParent(), *Result, *SpillLoc});
     DebugPHINumToValue.push_back(DbgPHI);
   }
 
@@ -1285,7 +1359,7 @@ bool InstrRefBasedLDV::isLocationSpill(const MachineInstr &MI,
   return Reg != 0;
 }
 
-Optional<SpillLoc>
+Optional<SpillLocationNo>
 InstrRefBasedLDV::isRestoreInstruction(const MachineInstr &MI,
                                        MachineFunction *MF, unsigned &Reg) {
   if (!MI.hasOneMemOperand())
@@ -1307,9 +1381,15 @@ bool InstrRefBasedLDV::transferSpillOrRestoreInst(MachineInstr &MI) {
   if (EmulateOldLDV)
     return false;
 
+  // Strictly limit ourselves to plain loads and stores, not all instructions
+  // that can access the stack.
+  int DummyFI = -1;
+  if (!TII->isStoreToStackSlotPostFE(MI, DummyFI) &&
+      !TII->isLoadFromStackSlotPostFE(MI, DummyFI))
+    return false;
+
   MachineFunction *MF = MI.getMF();
   unsigned Reg;
-  Optional<SpillLoc> Loc;
 
   LLVM_DEBUG(dbgs() << "Examining instruction: "; MI.dump(););
 
@@ -1317,74 +1397,94 @@ bool InstrRefBasedLDV::transferSpillOrRestoreInst(MachineInstr &MI) {
   // 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) {
-        // Un-set this location before clobbering, so that we don't salvage
-        // the variable location back to the same place.
-        MTracker->setMLoc(*MLoc, ValueIDNum::EmptyValue);
+    SpillLocationNo Loc = extractSpillBaseRegAndOffset(MI);
+
+    // Un-set this location and clobber, so that earlier locations don't
+    // continue past this store.
+    for (unsigned SlotIdx = 0; SlotIdx < MTracker->NumSlotIdxes; ++SlotIdx) {
+      unsigned SpillID = MTracker->getSpillIDWithIdx(Loc, SlotIdx);
+      Optional<LocIdx> MLoc = MTracker->getSpillMLoc(SpillID);
+      if (!MLoc)
+        continue;
+
+      // We need to over-write the stack slot with something (here, a def at
+      // this instruction) to ensure no values are preserved in this stack slot
+      // after the spill. It also prevents TTracker from trying to recover the
+      // location and re-installing it in the same place.
+      ValueIDNum Def(CurBB, CurInst, *MLoc);
+      MTracker->setMLoc(*MLoc, Def);
+      if (TTracker)
         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);
+    SpillLocationNo Loc = extractSpillBaseRegAndOffset(MI);
+
+    auto DoTransfer = [&](Register SrcReg, unsigned SpillID) {
+      auto ReadValue = MTracker->readReg(SrcReg);
+      LocIdx DstLoc = MTracker->getSpillMLoc(SpillID);
+      MTracker->setMLoc(DstLoc, ReadValue);
 
-    // 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)};
+      if (TTracker) {
+        LocIdx SrcLoc = MTracker->getRegMLoc(SrcReg);
+        TTracker->transferMlocs(SrcLoc, DstLoc, MI.getIterator());
+      }
+    };
 
-    MTracker->setSpill(*Loc, ValueID);
-    auto OptSpillLocIdx = MTracker->getSpillMLoc(*Loc);
-    assert(OptSpillLocIdx && "Spill slot set but has no LocIdx?");
-    LocIdx SpillLocIdx = *OptSpillLocIdx;
+    // Then, transfer subreg bits.
+    for (MCSubRegIterator SRI(Reg, TRI, false); SRI.isValid(); ++SRI) {
+      // Ensure this reg is tracked,
+      (void)MTracker->lookupOrTrackRegister(*SRI);
+      unsigned SubregIdx = TRI->getSubRegIndex(Reg, *SRI);
+      unsigned SpillID = MTracker->getLocID(Loc, SubregIdx);
+      DoTransfer(*SRI, SpillID);
+    }
 
-    // Tell TransferTracker about this spill, produce DBG_VALUEs for it.
-    if (TTracker)
-      TTracker->transferMlocs(MTracker->getRegMLoc(Reg), SpillLocIdx,
-                              MI.getIterator());
+    // Directly lookup size of main source reg, and transfer.
+    unsigned Size = TRI->getRegSizeInBits(Reg, *MRI);
+    unsigned SpillID = MTracker->getLocID(Loc, {Size, 0});
+    DoTransfer(Reg, SpillID);
   } else {
-    if (!(Loc = isRestoreInstruction(MI, MF, Reg)))
+    Optional<SpillLocationNo> OptLoc = isRestoreInstruction(MI, MF, Reg);
+    if (!OptLoc)
       return false;
+    SpillLocationNo Loc = *OptLoc;
 
-    // Is there a value to be restored?
-    auto OptValueID = MTracker->readSpill(*Loc);
-    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.
-      for (MCRegAliasIterator RAI(Reg, TRI, true); RAI.isValid(); ++RAI)
-        MTracker->defReg(*RAI, CurBB, CurInst);
+    // Assumption: we're reading from the base of the stack slot, not some
+    // offset into it. It seems very unlikely LLVM would ever generate
+    // restores where this wasn't true. This then becomes a question of what
+    // subregisters in the destination register line up with positions in the
+    // stack slot.
 
-      // Now override the reg we're restoring to.
-      MTracker->setReg(Reg, ValueID);
+    // Def all registers that alias the destination.
+    for (MCRegAliasIterator RAI(Reg, TRI, true); RAI.isValid(); ++RAI)
+      MTracker->defReg(*RAI, CurBB, CurInst);
+
+    // Now find subregisters within the destination register, and load values
+    // from stack slot positions.
+    auto DoTransfer = [&](Register DestReg, unsigned SpillID) {
+      LocIdx SrcIdx = MTracker->getSpillMLoc(SpillID);
+      auto ReadValue = MTracker->readMLoc(SrcIdx);
+      MTracker->setReg(DestReg, ReadValue);
+
+      if (TTracker) {
+        LocIdx DstLoc = MTracker->getRegMLoc(DestReg);
+        TTracker->transferMlocs(SrcIdx, DstLoc, MI.getIterator());
+      }
+    };
 
-      // Report this restore to the transfer tracker too.
-      if (TTracker)
-        TTracker->transferMlocs(SpillLocIdx, MTracker->getRegMLoc(Reg),
-                                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);
+    for (MCSubRegIterator SRI(Reg, TRI, false); SRI.isValid(); ++SRI) {
+      unsigned Subreg = TRI->getSubRegIndex(Reg, *SRI);
+      unsigned SpillID = MTracker->getLocID(Loc, Subreg);
+      DoTransfer(*SRI, SpillID);
     }
+
+    // Directly look up this registers slot idx by size, and transfer.
+    unsigned Size = TRI->getRegSizeInBits(Reg, *MRI);
+    unsigned SpillID = MTracker->getLocID(Loc, {Size, 0});
+    DoTransfer(Reg, SpillID);
   }
   return true;
 }
@@ -1624,7 +1724,7 @@ void InstrRefBasedLDV::produceMLocTransferFunction(
     // 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];
 
@@ -2645,6 +2745,7 @@ bool InstrRefBasedLDV::ExtendRanges(MachineFunction &MF,
 
   this->DomTree = DomTree;
   TRI = MF.getSubtarget().getRegisterInfo();
+  MRI = &MF.getRegInfo();
   TII = MF.getSubtarget().getInstrInfo();
   TFI = MF.getSubtarget().getFrameLowering();
   TFI->getCalleeSaves(MF, CalleeSavedRegs);
index b211154..48a03f8 100644 (file)
@@ -157,6 +157,26 @@ public:
   static ValueIDNum EmptyValue;
 };
 
+/// 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; }
+
+  bool operator<(const SpillLocationNo &Other) const {
+    return SpillNo < Other.SpillNo;
+  }
+
+  bool operator==(const SpillLocationNo &Other) const {
+    return SpillNo == Other.SpillNo;
+  }
+  bool operator!=(const SpillLocationNo &Other) const {
+    return !(*this == Other);
+  }
+};
+
 /// Meta qualifiers for a value. Pair of whatever expression is used to qualify
 /// the the value, and Boolean of whether or not it's indirect.
 class DbgValueProperties {
@@ -276,19 +296,35 @@ public:
 /// 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
-/// indirection going on, with two kinds of ``location'':
-///  * A LocID uniquely identifies a register or spill location, with a
-///    predictable value.
-///  * A LocIdx is a key (in the database sense) for a LocID and a ValueIDNum.
-/// Whenever a location is def'd or used by a MachineInstr, we automagically
-/// create a new LocIdx for a location, but not otherwise. This ensures we only
-/// account for locations that are actually used or defined. The cost is another
-/// vector lookup (of LocID -> LocIdx) over any other implementation. This is
-/// fairly cheap, and the compiler tries to reduce the working-set at any one
-/// time in the function anyway.
+/// indirection going on, described below, to avoid unnecessarily tracking
+/// any location.
+///
+/// Here's a sort of diagram of the indexes, read from the bottom up:
+///
+///           Size on stack   Offset on stack
+///                 \              /
+///          Stack Idx (Where in slot is this?)
+///                         /
+///                        /
+/// Slot Num (%stack.0)   /
+/// FrameIdx => SpillNum /
+///              \      /
+///           SpillID (int)              Register number (int)
+///                      \                  /
+///                      LocationID => LocIdx
+///                                |
+///                       LocIdx => ValueIDNum
+///
+/// The aim here is that the LocIdx => ValueIDNum vector is just an array of
+/// values in numbered locations, so that later analyses can ignore whether the
+/// location is a register or otherwise. To map a register / spill location to
+/// a LocIdx, you have to use the (sparse) LocationID => LocIdx map. And to
+/// build a LocationID for a stack slot, you need to combine identifiers for
+/// which stack slot it is and where within that slot is being described.
 ///
-/// Register mask operands completely blow this out of the water; I've just
-/// piled hacks on top of hacks to get around that.
+/// Register mask operands cause trouble by technically defining every register;
+/// various hacks are used to avoid tracking registers that are never read and
+/// only written by regmasks.
 class MLocTracker {
 public:
   MachineFunction &MF;
@@ -321,8 +357,8 @@ public:
   /// keep a set of them here.
   SmallSet<Register, 8> SPAliases;
 
-  /// Unique-ification of spill slots. Used to number them -- their LocID
-  /// number is the index in SpillLocs minus one plus NumRegs.
+  /// Unique-ification of spill. Used to number them -- their LocID number is
+  /// the index in SpillLocs minus one plus NumRegs.
   UniqueVector<SpillLoc> SpillLocs;
 
   // If we discover a new machine location, assign it an mphi with this
@@ -332,12 +368,29 @@ public:
   /// Cached local copy of the number of registers the target has.
   unsigned NumRegs;
 
+  /// Number of slot indexes the target has -- distinct segments of a stack
+  /// slot that can take on the value of a subregister, when a super-register
+  /// is written to the stack.
+  unsigned NumSlotIdxes;
+
   /// 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;
 
+  /// Pair for describing a position within a stack slot -- first the size in
+  /// bits, then the offset.
+  typedef std::pair<unsigned short, unsigned short> StackSlotPos;
+
+  /// Map from a size/offset pair describing a position in a stack slot, to a
+  /// numeric identifier for that position. Allows easier identification of
+  /// individual positions.
+  DenseMap<StackSlotPos, unsigned> StackSlotIdxes;
+
+  /// Inverse of StackSlotIdxes.
+  DenseMap<unsigned, StackSlotPos> StackIdxesToPos;
+
   /// Iterator for locations and the values they contain. Dereferencing
   /// produces a struct/pair containing the LocIdx key for this location,
   /// and a reference to the value currently stored. Simplifies the process
@@ -374,10 +427,57 @@ public:
   MLocTracker(MachineFunction &MF, const TargetInstrInfo &TII,
               const TargetRegisterInfo &TRI, const TargetLowering &TLI);
 
-  /// 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 a Register. Provides some small amount of
+  /// type safety.
+  /// \param Reg The register we're looking up.
+  unsigned getLocID(Register Reg) { return Reg.id(); }
+
+  /// Produce location ID number for a spill position.
+  /// \param Spill The number of the spill we're fetching the location for.
+  /// \param SpillSubReg Subregister within the spill we're addressing.
+  unsigned getLocID(SpillLocationNo Spill, unsigned SpillSubReg) {
+    unsigned short Size = TRI.getSubRegIdxSize(SpillSubReg);
+    unsigned short Offs = TRI.getSubRegIdxOffset(SpillSubReg);
+    return getLocID(Spill, {Size, Offs});
+  }
+
+  /// Produce location ID number for a spill position.
+  /// \param Spill The number of the spill we're fetching the location for.
+  /// \apram SpillIdx size/offset within the spill slot to be addressed.
+  unsigned getLocID(SpillLocationNo Spill, StackSlotPos Idx) {
+    unsigned SlotNo = Spill.id() - 1;
+    SlotNo *= NumSlotIdxes;
+    assert(StackSlotIdxes.find(Idx) != StackSlotIdxes.end());
+    SlotNo += StackSlotIdxes[Idx];
+    SlotNo += NumRegs;
+    return SlotNo;
+  }
+
+  /// Given a spill number, and a slot within the spill, calculate the ID number
+  /// for that location.
+  unsigned getSpillIDWithIdx(SpillLocationNo Spill, unsigned Idx) {
+    unsigned SlotNo = Spill.id() - 1;
+    SlotNo *= NumSlotIdxes;
+    SlotNo += Idx;
+    SlotNo += NumRegs;
+    return SlotNo;
+  }
+
+  /// Return the spill number that a location ID corresponds to.
+  SpillLocationNo locIDToSpill(unsigned ID) const {
+    assert(ID >= NumRegs);
+    ID -= NumRegs;
+    // Truncate away the index part, leaving only the spill number.
+    ID /= NumSlotIdxes;
+    return SpillLocationNo(ID + 1); // The UniqueVector is one-based.
+  }
+
+  /// Returns the spill-slot size/offs that a location ID corresponds to.
+  StackSlotPos locIDToSpillIdx(unsigned ID) const {
+    assert(ID >= NumRegs);
+    ID -= NumRegs;
+    unsigned Idx = ID % NumSlotIdxes;
+    return StackIdxesToPos.find(Idx)->second;
   }
 
   unsigned getNumLocs(void) const { return LocIdxToIDNum.size(); }
@@ -419,6 +519,8 @@ public:
     // SpillLocs.reset(); XXX UniqueVector::reset assumes a SpillLoc casts from
     // 0
     SpillLocs = decltype(SpillLocs)();
+    StackSlotIdxes.clear();
+    StackIdxesToPos.clear();
 
     LocIDToLocIdx.resize(NumRegs, LocIdx::MakeIllegalLoc());
   }
@@ -456,7 +558,7 @@ public:
   /// 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;
@@ -465,13 +567,13 @@ public:
   /// 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];
   }
@@ -481,14 +583,16 @@ public:
   /// 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);
+    unsigned ID = getLocID(R);
+    assert(ID < LocIDToLocIdx.size());
+    assert(LocIDToLocIdx[ID] != UINT_MAX); // Sentinal for IndexedMap.
     return LocIDToLocIdx[ID];
   }
 
@@ -498,33 +602,12 @@ public:
   void writeRegMask(const MachineOperand *MO, unsigned CurBB, unsigned InstID);
 
   /// Find LocIdx for SpillLoc \p L, creating a new one if it's not tracked.
-  LocIdx getOrTrackSpillLoc(SpillLoc L);
-
-  /// Set the value stored in a spill slot.
-  void setSpill(SpillLoc L, ValueIDNum ValueID) {
-    LocIdx Idx = getOrTrackSpillLoc(L);
-    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)
-      return None;
-
-    unsigned LocID = getLocID(SpillID, true);
-    LocIdx Idx = LocIDToLocIdx[LocID];
-    return LocIdxToIDNum[Idx];
-  }
+  SpillLocationNo getOrTrackSpillLoc(SpillLoc L);
 
-  /// 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)
-      return None;
-    unsigned LocNo = getLocID(SpillID, true);
-    return LocIDToLocIdx[LocNo];
+  // Get LocIdx of a spill ID.
+  LocIdx getSpillMLoc(unsigned SpillID) {
+    assert(LocIDToLocIdx[SpillID] != UINT_MAX); // Sentinal for IndexedMap.
+    return LocIDToLocIdx[SpillID];
   }
 
   /// Return true if Idx is a spill machine location.
@@ -651,6 +734,7 @@ public:
 private:
   MachineDominatorTree *DomTree;
   const TargetRegisterInfo *TRI;
+  const MachineRegisterInfo *MRI;
   const TargetInstrInfo *TII;
   const TargetFrameLowering *TFI;
   const MachineFrameInfo *MFI;
@@ -733,12 +817,12 @@ private:
 
   /// If a given instruction is identified as a spill, return the spill slot
   /// and set \p Reg to the spilled register.
-  Optional<SpillLoc> isRestoreInstruction(const MachineInstr &MI,
+  Optional<SpillLocationNo> 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);
+  /// Given a spill instruction, extract the spill slot information, ensure it's
+  /// tracked, and return the spill number.
+  SpillLocationNo extractSpillBaseRegAndOffset(const MachineInstr &MI);
 
   /// Observe a single instruction while stepping through a block.
   void process(MachineInstr &MI, ValueIDNum **MLiveOuts = nullptr,
diff --git a/llvm/test/DebugInfo/MIR/InstrRef/livedebugvalues_stackslot_subregs.mir b/llvm/test/DebugInfo/MIR/InstrRef/livedebugvalues_stackslot_subregs.mir
new file mode 100644 (file)
index 0000000..9cf1e4c
--- /dev/null
@@ -0,0 +1,56 @@
+# 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
+...
index df16069..26cdcf2 100644 (file)
@@ -86,13 +86,16 @@ body:  |
     MOV64mr $rsp, 1, $noreg, 16, $noreg, $rax :: (store 8 into %stack.0)
     $rax = MOV64ri 0, debug-location !12
     ; CHECK:      $rax = MOV64ri 0
-    ; The value is now located in a spill slot; currently InstrRefBasedLDV
-    ; can't express subregister locations inside spills. These should all
-    ; end up being $noreg, but could be improved in the future.
+    ; The value is now located in a spill slot, as a subregister within the
+    ; slot, which InstrRefBasedLDV should be able to find.
     DBG_INSTR_REF 1, 0, !11, !DIExpression(), debug-location !12
     ; CHECK-NEXT: DBG_INSTR_REF 1, 0
-    ; CHECK-NEXT: DBG_VALUE $noreg
+    ; CHECK-NEXT: DBG_VALUE $rsp, 0, !{{[0-9]*}}, !DIExpression(DW_OP_constu, 8, DW_OP_minus)
     DBG_INSTR_REF 5, 0, !11, !DIExpression(), debug-location !12
+    ; This and the next DBG_INSTR_REF refer to a value that is on the stack, but
+    ; is located at a non-zero offset from the start of the slot -- $ah within
+    ; $rax is 8 bits in. Today, InstrRefBasedLDV can't express this. It also
+    ; doesn't seem likely to be profitable.
     ; CHECK-NEXT: DBG_INSTR_REF 5, 0
     ; CHECK-NEXT: DBG_VALUE $noreg
     DBG_INSTR_REF 8, 0, !11, !DIExpression(), debug-location !12
index 7904320..a0fd10f 100644 (file)
@@ -635,12 +635,14 @@ TEST_F(InstrRefLDVTest, MTransferCopies) {
   // it's not completely clear why, but here we only care about correctly
   // identifying the slot, not that all the surrounding data is correct.
   SpillLoc L = {getRegByName("RSP"), StackOffset::getFixed(-8)};
-  Optional<ValueIDNum> V = MTracker->readSpill(L);
-  ASSERT_TRUE(V);
+  SpillLocationNo SpillNo = MTracker->getOrTrackSpillLoc(L);
+  unsigned SpillLocID = MTracker->getLocID(SpillNo, {64, 0});
+  LocIdx SpillLoc = MTracker->getSpillMLoc(SpillLocID);
+  ValueIDNum V = MTracker->readMLoc(SpillLoc);
   Register RAX = getRegByName("RAX");
   LocIdx RaxLoc = MTracker->getRegMLoc(RAX);
   ValueIDNum Cmp(0, 1, RaxLoc);
-  EXPECT_EQ(*V, Cmp);
+  EXPECT_EQ(V, Cmp);
 
   // A spill and restore should be recognised.
   MF = readMIRBlock(
@@ -662,7 +664,8 @@ TEST_F(InstrRefLDVTest, MTransferCopies) {
   ValueIDNum RbxVal = MTracker->readMLoc(RbxLoc);
   EXPECT_EQ(RbxVal, Cmp);
 
-  // FIXME: future work, make sure all the subregisters are transferred too.
+  // Testing that all the subregisters are transferred happens in
+  // MTransferSubregSpills.
 
   // Copies and x86 movs should be recognised and honoured. In addition, all
   // of the subregisters should be copied across too.
@@ -724,6 +727,160 @@ TEST_F(InstrRefLDVTest, MTransferCopies) {
   EXPECT_EQ(RcxVal, RcxDefVal);
 }
 
+TEST_F(InstrRefLDVTest, MTransferSubregSpills) {
+  SmallVector<MLocTransferMap, 1> TransferMap;
+  MachineFunction *MF = readMIRBlock(
+   "    $rax = MOV64ri 0\n"
+   "    MOV64mr $rsp, 1, $noreg, 16, $noreg, $rax :: (store 8 into %stack.0)\n"
+   "    $rbx = MOV64rm $rsp, 1, $noreg, 0, $noreg :: (load 8 from %stack.0)\n"
+   "    RETQ\n");
+  setupLDVObj(MF);
+  TransferMap.clear();
+  TransferMap.resize(1);
+  produceMLocTransferFunction(*MF, TransferMap, 1);
+
+  // Check that all the subregs of rax and rbx contain the same values. One
+  // should completely transfer to the other.
+  const char *ARegs[] = {"AL", "AH", "AX", "EAX", "HAX", "RAX"};
+  const char *BRegs[] = {"BL", "BH", "BX", "EBX", "HBX", "RBX"};
+  for (unsigned int I = 0; I < 6; ++I) {
+    LocIdx A = MTracker->getRegMLoc(getRegByName(ARegs[I]));
+    LocIdx B = MTracker->getRegMLoc(getRegByName(BRegs[I]));
+    EXPECT_EQ(MTracker->readMLoc(A), MTracker->readMLoc(B));
+  }
+
+  // Explicitly check what's in the different subreg slots, on the stack.
+  // Pair up subreg idx fields with the corresponding subregister in $rax.
+  MLocTracker::StackSlotPos SubRegIdxes[] = {{8, 0}, {8, 8}, {16, 0}, {32, 0}, {64, 0}};
+  const char *SubRegNames[] = {"AL", "AH", "AX", "EAX", "RAX"};
+  for (unsigned int I = 0; I < 5; ++I) {
+    // Value number where it's defined,
+    LocIdx RegLoc = MTracker->getRegMLoc(getRegByName(SubRegNames[I]));
+    ValueIDNum DefNum(0, 1, RegLoc);
+    // Read the corresponding subreg field from the stack.
+    SpillLoc L = {getRegByName("RSP"), StackOffset::getFixed(-8)};
+    SpillLocationNo SpillNo = MTracker->getOrTrackSpillLoc(L);
+    unsigned SpillID = MTracker->getLocID(SpillNo, SubRegIdxes[I]);
+    LocIdx SpillLoc = MTracker->getSpillMLoc(SpillID);
+    ValueIDNum SpillValue = MTracker->readMLoc(SpillLoc);
+    EXPECT_EQ(DefNum, SpillValue);
+  }
+
+  // If we have exactly the same code, but we write $eax to the stack slot after
+  // $rax, then we should still have exactly the same output in the lower five
+  // subregisters. Storing $eax to the start of the slot will overwrite with the
+  // same values. $rax, as an aliasing register, should be reset to something
+  // else by that write.
+  // In theory, we could try and recognise that we're writing the _same_ values
+  // to the stack again, and so $rax doesn't need to be reset to something else.
+  // It seems vanishingly unlikely that LLVM would generate such code though,
+  // so the benefits would be small.
+  MF = readMIRBlock(
+   "    $rax = MOV64ri 0\n"
+   "    MOV64mr $rsp, 1, $noreg, 16, $noreg, $rax :: (store 8 into %stack.0)\n"
+   "    MOV32mr $rsp, 1, $noreg, 16, $noreg, $eax :: (store 4 into %stack.0)\n"
+   "    $rbx = MOV64rm $rsp, 1, $noreg, 0, $noreg :: (load 8 from %stack.0)\n"
+   "    RETQ\n");
+  setupLDVObj(MF);
+  TransferMap.clear();
+  TransferMap.resize(1);
+  produceMLocTransferFunction(*MF, TransferMap, 1);
+
+  // Check lower five registers up to and include $eax == $ebx,
+  for (unsigned int I = 0; I < 5; ++I) {
+    LocIdx A = MTracker->getRegMLoc(getRegByName(ARegs[I]));
+    LocIdx B = MTracker->getRegMLoc(getRegByName(BRegs[I]));
+    EXPECT_EQ(MTracker->readMLoc(A), MTracker->readMLoc(B));
+  }
+
+  // $rbx should contain something else; today it's a def at the spill point
+  // of the 4 byte value.
+  SpillLoc L = {getRegByName("RSP"), StackOffset::getFixed(-8)};
+  SpillLocationNo SpillNo = MTracker->getOrTrackSpillLoc(L);
+  unsigned SpillID = MTracker->getLocID(SpillNo, {64, 0});
+  LocIdx Spill64Loc = MTracker->getSpillMLoc(SpillID);
+  ValueIDNum DefAtSpill64(0, 3, Spill64Loc);
+  LocIdx RbxLoc = MTracker->getRegMLoc(getRegByName("RBX"));
+  EXPECT_EQ(MTracker->readMLoc(RbxLoc), DefAtSpill64);
+
+  // Same again, test that the lower four subreg slots on the stack are the
+  // value defined by $rax in instruction 1.
+  for (unsigned int I = 0; I < 4; ++I) {
+    // Value number where it's defined,
+    LocIdx RegLoc = MTracker->getRegMLoc(getRegByName(SubRegNames[I]));
+    ValueIDNum DefNum(0, 1, RegLoc);
+    // Read the corresponding subreg field from the stack.
+    SpillNo = MTracker->getOrTrackSpillLoc(L);
+    SpillID = MTracker->getLocID(SpillNo, SubRegIdxes[I]);
+    LocIdx SpillLoc = MTracker->getSpillMLoc(SpillID);
+    ValueIDNum SpillValue = MTracker->readMLoc(SpillLoc);
+    EXPECT_EQ(DefNum, SpillValue);
+  }
+
+  // Stack slot for $rax should be a different value, today it's EmptyValue.
+  ValueIDNum SpillValue = MTracker->readMLoc(Spill64Loc);
+  EXPECT_EQ(SpillValue, DefAtSpill64);
+
+  // If we write something to the stack, then over-write with some register
+  // from a completely different hierarchy, none of the "old" values should be
+  // readable.
+  // NB: slight hack, store 16 in to a 8 byte stack slot.
+  MF = readMIRBlock(
+   "    $rax = MOV64ri 0\n"
+   "    MOV64mr $rsp, 1, $noreg, 16, $noreg, $rax :: (store 8 into %stack.0)\n"
+   "    $xmm0 = IMPLICIT_DEF\n"
+   "    MOVUPDmr $rsp, 1, $noreg, 16, $noreg, killed $xmm0 :: (store (s128) into %stack.0)\n"
+   "    $rbx = MOV64rm $rsp, 1, $noreg, 0, $noreg :: (load 8 from %stack.0)\n"
+   "    RETQ\n");
+  setupLDVObj(MF);
+  TransferMap.clear();
+  TransferMap.resize(1);
+  produceMLocTransferFunction(*MF, TransferMap, 1);
+
+  for (unsigned int I = 0; I < 5; ++I) {
+    // Read subreg fields from the stack.
+    SpillLocationNo SpillNo = MTracker->getOrTrackSpillLoc(L);
+    unsigned SpillID = MTracker->getLocID(SpillNo, SubRegIdxes[I]);
+    LocIdx SpillLoc = MTracker->getSpillMLoc(SpillID);
+    ValueIDNum SpillValue = MTracker->readMLoc(SpillLoc);
+
+    // Value should be defined by the spill-to-xmm0 instr, get value of a def
+    // at the point of the spill.
+    ValueIDNum SpillDef(0, 4, SpillLoc);
+    EXPECT_EQ(SpillValue, SpillDef);
+  }
+
+  // Read xmm0's position and ensure it has a value. Should be the live-in
+  // value to the block, as IMPLICIT_DEF isn't a real def.
+  SpillNo = MTracker->getOrTrackSpillLoc(L);
+  SpillID = MTracker->getLocID(SpillNo, {128, 0});
+  LocIdx Spill128Loc = MTracker->getSpillMLoc(SpillID);
+  SpillValue = MTracker->readMLoc(Spill128Loc);
+  Register XMM0 = getRegByName("XMM0");
+  LocIdx Xmm0Loc = MTracker->getRegMLoc(XMM0);
+  EXPECT_EQ(ValueIDNum(0, 0, Xmm0Loc), SpillValue);
+
+  // What happens if we spill ah to the stack, then load al? It should find
+  // the same value.
+  MF = readMIRBlock(
+   "    $rax = MOV64ri 0\n"
+   "    MOV8mr $rsp, 1, $noreg, 16, $noreg, $ah :: (store 1 into %stack.0)\n"
+   "    $al = MOV8rm $rsp, 1, $noreg, 0, $noreg :: (load 1 from %stack.0)\n"
+   "    RETQ\n");
+  setupLDVObj(MF);
+  TransferMap.clear();
+  TransferMap.resize(1);
+  produceMLocTransferFunction(*MF, TransferMap, 1);
+
+  Register AL = getRegByName("AL");
+  Register AH = getRegByName("AH");
+  LocIdx AlLoc = MTracker->getRegMLoc(AL);
+  LocIdx AhLoc = MTracker->getRegMLoc(AH);
+  ValueIDNum AHDef(0, 1, AhLoc);
+  ValueIDNum ALValue = MTracker->readMLoc(AlLoc);
+  EXPECT_EQ(ALValue, AHDef);
+}
+
 TEST_F(InstrRefLDVTest, MLocSingleBlock) {
   // Test some very simple properties about interpreting the transfer function.
   setupSingleBlock();