From 90c2794bfc337338b4e4ec8421c75735e7207e6e Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Wed, 14 Aug 2019 12:20:02 +0000 Subject: [PATCH] [DebugInfo] MCP: collect and update DBG_VALUEs encountered in local block MCP currently uses changeDebugValuesDefReg / collectDebugValues to find debug users of a register, however those functions assume that all DBG_VALUEs immediately follow the specified instruction, which isn't necessarily true. This is going to become very often untrue when we turn off CodeGenPrepare::placeDbgValues. Instead of calling changeDebugValuesDefReg on an instruction to change its debug users, in this patch we instead collect DBG_VALUEs of copies as we iterate over insns, and update the debug users of copies that are made dead. This isn't a non-functional change, because MCP will now update DBG_VALUEs that aren't immediately after a copy, but refer to the same register. I've hijacked the regression test for PR38773 to test for this new behaviour, an entirely new test seemed overkill. Differential Revision: https://reviews.llvm.org/D56265 llvm-svn: 368835 --- llvm/include/llvm/CodeGen/MachineRegisterInfo.h | 11 +++++++ llvm/lib/CodeGen/MachineCopyPropagation.cpp | 38 +++++++++++++++++-------- llvm/test/CodeGen/MIR/X86/pr38773.mir | 7 +++++ 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/llvm/include/llvm/CodeGen/MachineRegisterInfo.h b/llvm/include/llvm/CodeGen/MachineRegisterInfo.h index ca64ae6..488a5a5 100644 --- a/llvm/include/llvm/CodeGen/MachineRegisterInfo.h +++ b/llvm/include/llvm/CodeGen/MachineRegisterInfo.h @@ -818,6 +818,17 @@ public: /// deleted during LiveDebugVariables analysis. void markUsesInDebugValueAsUndef(unsigned Reg) const; + /// updateDbgUsersToReg - Update a collection of DBG_VALUE instructions + /// to refer to the designated register. + void updateDbgUsersToReg(unsigned Reg, + ArrayRef Users) const { + for (MachineInstr *MI : Users) { + assert(MI->isDebugInstr()); + assert(MI->getOperand(0).isReg()); + MI->getOperand(0).setReg(Reg); + } + } + /// Return true if the specified register is modified in this function. /// This checks that no defining machine operands exist for the register or /// any of its aliases. Definitions found on functions marked noreturn are diff --git a/llvm/lib/CodeGen/MachineCopyPropagation.cpp b/llvm/lib/CodeGen/MachineCopyPropagation.cpp index 1df98fc..f3946b8 100644 --- a/llvm/lib/CodeGen/MachineCopyPropagation.cpp +++ b/llvm/lib/CodeGen/MachineCopyPropagation.cpp @@ -205,8 +205,11 @@ public: } private: + typedef enum { DebugUse = false, RegularUse = true } DebugType; + void ClobberRegister(unsigned Reg); - void ReadRegister(unsigned Reg); + void ReadRegister(unsigned Reg, MachineInstr &Reader, + DebugType DT); void CopyPropagateBlock(MachineBasicBlock &MBB); bool eraseIfRedundant(MachineInstr &Copy, unsigned Src, unsigned Def); void forwardUses(MachineInstr &MI); @@ -217,6 +220,9 @@ private: /// Candidates for deletion. SmallSetVector MaybeDeadCopies; + /// Multimap tracking debug users in current BB + DenseMap> CopyDbgUsers; + CopyTracker Tracker; bool Changed; @@ -231,13 +237,19 @@ char &llvm::MachineCopyPropagationID = MachineCopyPropagation::ID; INITIALIZE_PASS(MachineCopyPropagation, DEBUG_TYPE, "Machine Copy Propagation Pass", false, false) -void MachineCopyPropagation::ReadRegister(unsigned Reg) { +void MachineCopyPropagation::ReadRegister(unsigned Reg, MachineInstr &Reader, + DebugType DT) { // If 'Reg' is defined by a copy, the copy is no longer a candidate - // for elimination. + // for elimination. If a copy is "read" by a debug user, record the user + // for propagation. for (MCRegUnitIterator RUI(Reg, TRI); RUI.isValid(); ++RUI) { if (MachineInstr *Copy = Tracker.findCopyForUnit(*RUI, *TRI)) { - LLVM_DEBUG(dbgs() << "MCP: Copy is used - not dead: "; Copy->dump()); - MaybeDeadCopies.remove(Copy); + if (DT == RegularUse) { + LLVM_DEBUG(dbgs() << "MCP: Copy is used - not dead: "; Copy->dump()); + MaybeDeadCopies.remove(Copy); + } else { + CopyDbgUsers[Copy].push_back(&Reader); + } } } } @@ -488,14 +500,14 @@ void MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) { // If Src is defined by a previous copy, the previous copy cannot be // eliminated. - ReadRegister(Src); + ReadRegister(Src, *MI, RegularUse); for (const MachineOperand &MO : MI->implicit_operands()) { if (!MO.isReg() || !MO.readsReg()) continue; unsigned Reg = MO.getReg(); if (!Reg) continue; - ReadRegister(Reg); + ReadRegister(Reg, *MI, RegularUse); } LLVM_DEBUG(dbgs() << "MCP: Copy is a deletion candidate: "; MI->dump()); @@ -534,7 +546,7 @@ void MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) { // instruction, so we need to make sure we don't remove it as dead // later. if (MO.isTied()) - ReadRegister(Reg); + ReadRegister(Reg, *MI, RegularUse); Tracker.clobberRegister(Reg, *TRI); } @@ -558,8 +570,8 @@ void MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) { if (MO.isDef() && !MO.isEarlyClobber()) { Defs.push_back(Reg); continue; - } else if (!MO.isDebug() && MO.readsReg()) - ReadRegister(Reg); + } else if (MO.readsReg()) + ReadRegister(Reg, *MI, MO.isDebug() ? DebugUse : RegularUse); } // The instruction has a register mask operand which means that it clobbers @@ -609,9 +621,10 @@ void MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) { MaybeDead->dump()); assert(!MRI->isReserved(MaybeDead->getOperand(0).getReg())); - // Update matching debug values. + // Update matching debug values, if any. assert(MaybeDead->isCopy()); - MaybeDead->changeDebugValuesDefReg(MaybeDead->getOperand(1).getReg()); + unsigned SrcReg = MaybeDead->getOperand(1).getReg(); + MRI->updateDbgUsersToReg(SrcReg, CopyDbgUsers[MaybeDead]); MaybeDead->eraseFromParent(); Changed = true; @@ -620,6 +633,7 @@ void MachineCopyPropagation::CopyPropagateBlock(MachineBasicBlock &MBB) { } MaybeDeadCopies.clear(); + CopyDbgUsers.clear(); Tracker.clear(); } diff --git a/llvm/test/CodeGen/MIR/X86/pr38773.mir b/llvm/test/CodeGen/MIR/X86/pr38773.mir index 19b0deb..9af152b 100644 --- a/llvm/test/CodeGen/MIR/X86/pr38773.mir +++ b/llvm/test/CodeGen/MIR/X86/pr38773.mir @@ -99,6 +99,13 @@ body: | ; CHECK: IDIV32r killed renamable $ecx ; CHECK-NEXT: DBG_VALUE $eax, $noreg, !12, !DIExpression(), debug-location !13 DBG_VALUE $ecx, $noreg, !12, !DIExpression(), debug-location !13 + ; The following mov and DBG_VALUE have been inserted after the PR was + ; resolved to check that MCP will update debug users that are not + ; immediately after the dead copy. + ; CHECK-NEXT: $edx = MOV32r0 + $edx = MOV32r0 implicit-def dead $eflags + ; CHECK-NEXT: DBG_VALUE $eax, $noreg, !12, !DIExpression(), debug-location !13 + DBG_VALUE $ecx, $noreg, !12, !DIExpression(), debug-location !13 $eax = COPY killed renamable $ecx RET 0, $eax -- 2.7.4