From d73275993bbc19b38b7818e96953f84decf0653b Mon Sep 17 00:00:00 2001 From: Jeremy Morse Date: Thu, 22 Oct 2020 12:48:57 +0100 Subject: [PATCH] [DebugInstrRef] Substitute debug value numbers to handle optimizations This patch touches two optimizations, TwoAddressInstruction and X86's FixupLEAs pass, both of which optimize by re-creating instructions. For LEAs, various bits of arithmetic are better represented as LEAs on X86, while TwoAddressInstruction sometimes converts instrs into three address instructions if it's profitable. For debug instruction referencing, both of these require substitutions to be created -- the old instruction number must be pointed to the new instruction number, as illustrated in the added test. If this isn't done, any variable locations based on the optimized instruction are conservatively dropped. Differential Revision: https://reviews.llvm.org/D85756 --- .../lib/CodeGen/TwoAddressInstructionPass.cpp | 18 +++++++++ llvm/lib/Target/X86/X86FixupLEAs.cpp | 6 +++ .../MIR/InstrRef/twoaddr-to-threeaddr-sub.mir | 40 +++++++++++++++++++ 3 files changed, 64 insertions(+) create mode 100644 llvm/test/DebugInfo/MIR/InstrRef/twoaddr-to-threeaddr-sub.mir diff --git a/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp b/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp index 4a727902bdd2..17ad51a5d6ed 100644 --- a/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp +++ b/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp @@ -606,6 +606,24 @@ TwoAddressInstructionPass::convertInstTo3Addr(MachineBasicBlock::iterator &mi, if (LIS) LIS->ReplaceMachineInstrInMaps(*mi, *NewMI); + // If the old instruction is debug value tracked, an update is required. + if (auto OldInstrNum = mi->peekDebugInstrNum()) { + // Sanity check. + assert(mi->getNumExplicitDefs() == 1); + assert(NewMI->getNumExplicitDefs() == 1); + + // Find the old and new def location. + auto OldIt = mi->defs().begin(); + auto NewIt = NewMI->defs().begin(); + unsigned OldIdx = mi->getOperandNo(OldIt); + unsigned NewIdx = NewMI->getOperandNo(NewIt); + + // Record that one def has been replaced by the other. + unsigned NewInstrNum = NewMI->getDebugInstrNum(); + MF->makeDebugValueSubstitution(std::make_pair(OldInstrNum, OldIdx), + std::make_pair(NewInstrNum, NewIdx)); + } + MBB->erase(mi); // Nuke the old inst. DistanceMap.insert(std::make_pair(NewMI, Dist)); diff --git a/llvm/lib/Target/X86/X86FixupLEAs.cpp b/llvm/lib/Target/X86/X86FixupLEAs.cpp index ca9d8a1a4ffd..0054d5818a96 100644 --- a/llvm/lib/Target/X86/X86FixupLEAs.cpp +++ b/llvm/lib/Target/X86/X86FixupLEAs.cpp @@ -450,6 +450,7 @@ bool FixupLEAPass::optTwoAddrLEA(MachineBasicBlock::iterator &I, } else return false; + MBB.getParent()->substituteDebugValuesForInst(*I, *NewMI, 1); MBB.erase(I); I = NewMI; return true; @@ -485,6 +486,7 @@ void FixupLEAPass::seekLEAFixup(MachineOperand &p, LLVM_DEBUG(dbgs() << "FixLEA: Candidate to replace:"; MBI->dump();); // now to replace with an equivalent LEA... LLVM_DEBUG(dbgs() << "FixLEA: Replaced by: "; NewMI->dump();); + MBB.getParent()->substituteDebugValuesForInst(*MBI, *NewMI, 1); MBB.erase(MBI); MachineBasicBlock::iterator J = static_cast(NewMI); @@ -538,6 +540,7 @@ void FixupLEAPass::processInstructionForSlowLEA(MachineBasicBlock::iterator &I, LLVM_DEBUG(NewMI->dump();); } if (NewMI) { + MBB.getParent()->substituteDebugValuesForInst(*I, *NewMI, 1); MBB.erase(I); I = NewMI; } @@ -644,6 +647,7 @@ void FixupLEAPass::processInstrForSlow3OpLEA(MachineBasicBlock::iterator &I, } } + MBB.getParent()->substituteDebugValuesForInst(*I, *NewMI, 1); MBB.erase(I); I = NewMI; return; @@ -669,6 +673,7 @@ void FixupLEAPass::processInstrForSlow3OpLEA(MachineBasicBlock::iterator &I, .add(Index); LLVM_DEBUG(NewMI->dump();); + MBB.getParent()->substituteDebugValuesForInst(*I, *NewMI, 1); MBB.erase(I); I = NewMI; return; @@ -691,6 +696,7 @@ void FixupLEAPass::processInstrForSlow3OpLEA(MachineBasicBlock::iterator &I, .add(Base); LLVM_DEBUG(NewMI->dump();); + MBB.getParent()->substituteDebugValuesForInst(*I, *NewMI, 1); MBB.erase(I); I = NewMI; } diff --git a/llvm/test/DebugInfo/MIR/InstrRef/twoaddr-to-threeaddr-sub.mir b/llvm/test/DebugInfo/MIR/InstrRef/twoaddr-to-threeaddr-sub.mir new file mode 100644 index 000000000000..f8fac31fc5a1 --- /dev/null +++ b/llvm/test/DebugInfo/MIR/InstrRef/twoaddr-to-threeaddr-sub.mir @@ -0,0 +1,40 @@ +# RUN: llc -run-pass=twoaddressinstruction -mtriple=x86_64-- -o - %s -experimental-debug-variable-locations | FileCheck %s +# +# Test that a new instruction (LEA) is created when the two-addr add below is +# converted to three address; and that an appropriate substitution is created. +# Maybe at some point we'll normalise DBG_INSTR_REFs on output, but until then, +# lets not. +# +# CHECK: debugValueSubstitutions: +# CHECK-NEXT: - { srcinst: 1, srcop: 0, dstinst: 2, dstop: 0 } +# +# CHECK: LEA64_32r +# CHECK-SAME: debug-instr-number 2 +# +# CHECK: DBG_INSTR_REF 1, 0 +--- +name: test1 +alignment: 16 +tracksRegLiveness: true +registers: + - { id: 0, class: gr32 } + - { id: 1, class: gr32 } + - { id: 2, class: gr32 } +liveins: + - { reg: '$edi', virtual-reg: '%0' } +frameInfo: + maxAlignment: 1 +machineFunctionInfo: {} +body: | + bb.0: + liveins: $edi + + %0:gr32 = COPY killed $edi + %1:gr32 = SHL32ri killed %0, 5, implicit-def dead $eflags + %2:gr32 = ADD32ri8_DB killed %1, 3, implicit-def dead $eflags, debug-instr-number 1 + DBG_INSTR_REF 1, 0 + $eax = COPY killed %2 + RET 0, killed $eax + +... +