diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h --- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h +++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h @@ -157,6 +157,26 @@ 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 @@ /// 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 @@ /// keep a set of them here. SmallSet 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 SpillLocs; // If we discover a new machine location, assign it an mphi with this @@ -332,12 +368,29 @@ /// 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, 32> Masks; + /// Pair for describing a position within a stack slot -- first the size in + /// bits, then the offset. + typedef std::pair 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 StackSlotIdxes; + + /// Inverse of StackSlotIdxes. + DenseMap 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 @@ 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 @@ // 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 @@ /// 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 @@ /// 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 @@ /// 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 @@ 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 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 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 @@ private: MachineDominatorTree *DomTree; const TargetRegisterInfo *TRI; + const MachineRegisterInfo *MRI; const TargetInstrInfo *TII; const TargetFrameLowering *TFI; const MachineFrameInfo *MFI; @@ -733,12 +817,12 @@ /// If a given instruction is identified as a spill, return the spill slot /// and set \p Reg to the spilled register. - Optional isRestoreInstruction(const MachineInstr &MI, + Optional 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/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp --- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp +++ b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp @@ -148,15 +148,6 @@ 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 @@ // 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 @@ 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 @@ 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 @@ // 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 @@ 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 @@ // 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 @@ {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 @@ 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 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 CandidateSizes = {64, 32, 16, 8}; + Optional Result = None; + Optional 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 @@ return Reg != 0; } -Optional +Optional InstrRefBasedLDV::isRestoreInstruction(const MachineInstr &MI, MachineFunction *MF, unsigned &Reg) { if (!MI.hasOneMemOperand()) @@ -1307,9 +1381,15 @@ 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 Loc; LLVM_DEBUG(dbgs() << "Examining instruction: "; MI.dump();); @@ -1317,74 +1397,94 @@ // 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 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 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 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 @@ // 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 @@ this->DomTree = DomTree; TRI = MF.getSubtarget().getRegisterInfo(); + MRI = &MF.getRegInfo(); TII = MF.getSubtarget().getInstrInfo(); TFI = MF.getSubtarget().getFrameLowering(); TFI->getCalleeSaves(MF, CalleeSavedRegs); 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 --- /dev/null +++ b/llvm/test/DebugInfo/MIR/InstrRef/livedebugvalues_stackslot_subregs.mir @@ -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 +... diff --git a/llvm/test/DebugInfo/MIR/InstrRef/livedebugvalues_subreg_substitutions.mir b/llvm/test/DebugInfo/MIR/InstrRef/livedebugvalues_subreg_substitutions.mir --- a/llvm/test/DebugInfo/MIR/InstrRef/livedebugvalues_subreg_substitutions.mir +++ b/llvm/test/DebugInfo/MIR/InstrRef/livedebugvalues_subreg_substitutions.mir @@ -86,13 +86,16 @@ 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 diff --git a/llvm/unittests/CodeGen/InstrRefLDVTest.cpp b/llvm/unittests/CodeGen/InstrRefLDVTest.cpp --- a/llvm/unittests/CodeGen/InstrRefLDVTest.cpp +++ b/llvm/unittests/CodeGen/InstrRefLDVTest.cpp @@ -635,12 +635,14 @@ // 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 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 @@ 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 @@ EXPECT_EQ(RcxVal, RcxDefVal); } +TEST_F(InstrRefLDVTest, MTransferSubregSpills) { + SmallVector 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();