[llvm] d732759 - [DebugInstrRef] Substitute debug value numbers to handle optimizations

[Jeremy Morse via llvm-commits]

Author: Jeremy Morse
Date: 2020-10-22T13:01:03+01:00
New Revision: d73275993bbc19b38b7818e96953f84decf0653b

URL: https://github.com/llvm/llvm-project/commit/d73275993bbc19b38b7818e96953f84decf0653b
DIFF: https://github.com/llvm/llvm-project/commit/d73275993bbc19b38b7818e96953f84decf0653b.diff

LOG: [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

Added: 
    llvm/test/DebugInfo/MIR/InstrRef/twoaddr-to-threeaddr-sub.mir

Modified: 
    llvm/lib/CodeGen/TwoAddressInstructionPass.cpp
    llvm/lib/Target/X86/X86FixupLEAs.cpp

Removed: 
    


################################################################################
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<MachineBasicBlock::iterator>(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
+
+...
+