From f5e1b718a675a4449b71423f04d38e1e93045105 Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Mon, 28 Oct 2019 14:23:50 +0000 Subject: [DebugInfo] MachineSink: find more DBG_VALUEs to sink In the Pre-RA machine sinker, previously we were relying on all DBG_VALUEs being immediately after the instruction that defined their operands. This isn't a valid assumption, as a variable location change doesn't necessarily correspond to where the value is computed. In this patch, we collect DBG_VALUEs that might need sinking as we walk through a block, and sink all of them if their defining instruction is sunk. This patch adds some copy propagation too, so that if we sink a copy inst, the now non-dominated paths can use the copy source for the variable location. Differential Revision: https://reviews.llvm.org/D58386 --- llvm/lib/CodeGen/MachineSink.cpp | 101 +++++++++++++++++++++++++++++++++------ 1 file changed, 86 insertions(+), 15 deletions(-) (limited to 'llvm/lib/CodeGen/MachineSink.cpp') diff --git a/llvm/lib/CodeGen/MachineSink.cpp b/llvm/lib/CodeGen/MachineSink.cpp index 4c55158a004..81fd30ca052 100644 --- a/llvm/lib/CodeGen/MachineSink.cpp +++ b/llvm/lib/CodeGen/MachineSink.cpp @@ -105,6 +105,9 @@ namespace { using AllSuccsCache = std::map>; + // Remember debug uses of vregs seen, so we know what to sink out of blocks. + DenseMap> SeenDbgUsers; + public: static char ID; // Pass identification @@ -132,6 +135,7 @@ namespace { private: bool ProcessBlock(MachineBasicBlock &MBB); + void ProcessDbgInst(MachineInstr &MI); bool isWorthBreakingCriticalEdge(MachineInstr &MI, MachineBasicBlock *From, MachineBasicBlock *To); @@ -153,8 +157,14 @@ namespace { MachineBasicBlock *To, bool BreakPHIEdge); bool SinkInstruction(MachineInstr &MI, bool &SawStore, - AllSuccsCache &AllSuccessors); + + /// If we sink a COPY inst, some debug users of it's destination may no + /// longer be dominated by the COPY, and will eventually be dropped. + /// This is easily rectified by forwarding the non-dominated debug uses + /// to the copy source. + void SalvageUnsunkDebugUsersOfCopy(MachineInstr &, + MachineBasicBlock *TargetBlock); bool AllUsesDominatedByBlock(unsigned Reg, MachineBasicBlock *MBB, MachineBasicBlock *DefMBB, bool &BreakPHIEdge, bool &LocalUse) const; @@ -367,8 +377,11 @@ bool MachineSinking::ProcessBlock(MachineBasicBlock &MBB) { if (!ProcessedBegin) --I; - if (MI.isDebugInstr()) + if (MI.isDebugInstr()) { + if (MI.isDebugValue()) + ProcessDbgInst(MI); continue; + } bool Joined = PerformTrivialForwardCoalescing(MI, &MBB); if (Joined) { @@ -384,9 +397,23 @@ bool MachineSinking::ProcessBlock(MachineBasicBlock &MBB) { // If we just processed the first instruction in the block, we're done. } while (!ProcessedBegin); + SeenDbgUsers.clear(); + return MadeChange; } +void MachineSinking::ProcessDbgInst(MachineInstr &MI) { + // When we see DBG_VALUEs for registers, record any vreg it reads, so that + // we know what to sink if the vreg def sinks. + assert(MI.isDebugValue() && "Expected DBG_VALUE for processing"); + + MachineOperand &MO = MI.getOperand(0); + if (!MO.isReg() || !MO.getReg().isVirtual()) + return; + + SeenDbgUsers[MO.getReg()].push_back(&MI); +} + bool MachineSinking::isWorthBreakingCriticalEdge(MachineInstr &MI, MachineBasicBlock *From, MachineBasicBlock *To) { @@ -731,22 +758,13 @@ static bool SinkingPreventsImplicitNullCheck(MachineInstr &MI, MBP.LHS.getReg() == BaseOp->getReg(); } -/// Sink an instruction and its associated debug instructions. If the debug -/// instructions to be sunk are already known, they can be provided in DbgVals. +/// Sink an instruction and its associated debug instructions. static void performSink(MachineInstr &MI, MachineBasicBlock &SuccToSinkTo, MachineBasicBlock::iterator InsertPos, - SmallVectorImpl *DbgVals = nullptr) { + SmallVectorImpl &DbgValuesToSink) { const MachineRegisterInfo &MRI = MI.getMF()->getRegInfo(); const TargetInstrInfo &TII = *MI.getMF()->getSubtarget().getInstrInfo(); - // If debug values are provided use those, otherwise call collectDebugValues. - SmallVector DbgValuesToSink; - if (DbgVals) - DbgValuesToSink.insert(DbgValuesToSink.begin(), - DbgVals->begin(), DbgVals->end()); - else - MI.collectDebugValues(DbgValuesToSink); - // If we cannot find a location to use (merge with), then we erase the debug // location to prevent debug-info driven tools from potentially reporting // wrong location information. @@ -929,7 +947,25 @@ bool MachineSinking::SinkInstruction(MachineInstr &MI, bool &SawStore, while (InsertPos != SuccToSinkTo->end() && InsertPos->isPHI()) ++InsertPos; - performSink(MI, *SuccToSinkTo, InsertPos); + // Collect debug users of any vreg that this inst defines. + SmallVector DbgUsersToSink; + for (auto &MO : MI.operands()) { + if (!MO.isReg() || !MO.isDef() || !MO.getReg().isVirtual()) + continue; + if (!SeenDbgUsers.count(MO.getReg())) + continue; + + auto &Users = SeenDbgUsers[MO.getReg()]; + DbgUsersToSink.insert(DbgUsersToSink.end(), Users.begin(), Users.end()); + } + + // After sinking, some debug users may not be dominated any more. If possible, + // copy-propagate their operands. As it's expensive, don't do this if there's + // no debuginfo in the program. + if (MI.getMF()->getFunction().getSubprogram() && MI.isCopy()) + SalvageUnsunkDebugUsersOfCopy(MI, SuccToSinkTo); + + performSink(MI, *SuccToSinkTo, InsertPos, DbgUsersToSink); // Conservatively, clear any kill flags, since it's possible that they are no // longer correct. @@ -944,6 +980,41 @@ bool MachineSinking::SinkInstruction(MachineInstr &MI, bool &SawStore, return true; } +void MachineSinking::SalvageUnsunkDebugUsersOfCopy( + MachineInstr &MI, MachineBasicBlock *TargetBlock) { + assert(MI.isCopy()); + assert(MI.getOperand(1).isReg()); + + // Enumerate all users of vreg operands that are def'd. Skip those that will + // be sunk. For the rest, if they are not dominated by the block we will sink + // MI into, propagate the copy source to them. + SmallVector DbgDefUsers; + const MachineRegisterInfo &MRI = MI.getMF()->getRegInfo(); + for (auto &MO : MI.operands()) { + if (!MO.isReg() || !MO.isDef() || !MO.getReg().isVirtual()) + continue; + for (auto &User : MRI.use_instructions(MO.getReg())) { + if (!User.isDebugValue() || DT->dominates(TargetBlock, User.getParent())) + continue; + + // If is in same block, will either sink or be use-before-def. + if (User.getParent() == MI.getParent()) + continue; + + assert(User.getOperand(0).isReg() && + "DBG_VALUE user of vreg, but non reg operand?"); + DbgDefUsers.push_back(&User); + } + } + + // Point the users of this copy that are no longer dominated, at the source + // of the copy. + for (auto *User : DbgDefUsers) { + User->getOperand(0).setReg(MI.getOperand(1).getReg()); + User->getOperand(0).setSubReg(MI.getOperand(1).getSubReg()); + } +} + //===----------------------------------------------------------------------===// // This pass is not intended to be a replacement or a complete alternative // for the pre-ra machine sink pass. It is only designed to sink COPY @@ -1253,7 +1324,7 @@ bool PostRAMachineSinking::tryToSinkCopy(MachineBasicBlock &CurBB, // block. clearKillFlags(MI, CurBB, UsedOpsInCopy, UsedRegUnits, TRI); MachineBasicBlock::iterator InsertPos = SuccBB->getFirstNonPHI(); - performSink(*MI, *SuccBB, InsertPos, &DbgValsToSink); + performSink(*MI, *SuccBB, InsertPos, DbgValsToSink); updateLiveIn(MI, SuccBB, UsedOpsInCopy, DefedRegsInCopy); Changed = true; -- cgit v1.2.3