[DebugInfo][InstrRef][1/4] Support transformations that widen values

Very late in compilation, backends like X86 will perform optimisations like
this:

    $cx = MOV16rm $rax, ...
    ->
    $rcx = MOV64rm $rax, ...

Widening the load from 16 bits to 64 bits. SEeing how the lower 16 bits
remain the same, this doesn't affect execution. However, any debug
instruction reference to the defined operand now refers to a 64 bit value,
nto a 16 bit one, which might be unexpected. Elsewhere in codegen, there's
often this pattern:

    CALL64pcrel32 @foo, implicit-def $rax
    %0:gr64 = COPY $rax
    %1:gr32 = COPY %0.sub_32bit

Where we want to refer to the definition of $eax by the call, but don't
want to refer the copies (they don't define values in the way
LiveDebugValues sees it). To solve this, add a subregister field to the
existing "substitutions" facility, so that we can describe a field within
a larger value definition. I would imagine that this would be used most
often when a value is widened, and we need to refer to the original,
narrower definition.

Differential Revision: https://reviews.llvm.org/D88891

include/llvm/CodeGen/MIRYamlMapping.h

@@ -492,6 +492,7 @@ struct DebugValueSubstitution {
   unsigned SrcOp;
   unsigned DstInst;
   unsigned DstOp;
+  unsigned Subreg;
 
   bool operator==(const DebugValueSubstitution &Other) const {
     return std::tie(SrcInst, SrcOp, DstInst, DstOp) ==
@@ -505,6 +506,7 @@ template <> struct MappingTraits<DebugValueSubstitution> {
     YamlIO.mapRequired("srcop", Sub.SrcOp);
     YamlIO.mapRequired("dstinst", Sub.DstInst);
     YamlIO.mapRequired("dstop", Sub.DstOp);
+    YamlIO.mapRequired("subreg", Sub.Subreg);
   }
 
   static const bool flow = true;

include/llvm/CodeGen/MachineFunction.h

@@ -451,11 +451,24 @@ public:
   /// Pair of instruction number and operand number.
   using DebugInstrOperandPair = std::pair<unsigned, unsigned>;
 
-  /// Substitution map: from one <inst,operand> pair to another. Used to
-  /// record changes in where a value is defined, so that debug variable
-  /// locations can find it later.
-  std::map<DebugInstrOperandPair, DebugInstrOperandPair>
-      DebugValueSubstitutions;
+  /// Replacement definition for a debug instruction reference. Made up of an
+  /// instruction / operand pair, and a qualifying subregister indicating what
+  /// bits in the operand make up the substitution. For example, a debug user
+  /// of %1:
+  ///    %0:gr32 = someinst, debug-instr-number 2
+  ///    %1:gr16 = %0.some_16_bit_subreg
+  /// Would receive the substitution {{2, 0}, $subreg}, where $subreg is the
+  /// subregister number for some_16_bit_subreg.
+  struct DebugSubstitution {
+    DebugInstrOperandPair Dest; ///< Replacement instruction / operand pair.
+    unsigned Subreg;            ///< Qualifier for which part of Dest is read.
+  };
+
+  /// Substitution map: from one <inst,operand> pair identifying a value,
+  /// to a DebugSubstitution identifying another. Used to record changes in
+  /// where a value is defined, so that debug variable locations can find it
+  /// later.
+  std::map<DebugInstrOperandPair, DebugSubstitution> DebugValueSubstitutions;
 
   /// Location of a PHI instruction that is also a debug-info variable value,
   /// for the duration of register allocation. Loaded by the PHI-elimination
@@ -477,7 +490,8 @@ public:
 
   /// Create a substitution between one <instr,operand> value to a different,
   /// new value.
-  void makeDebugValueSubstitution(DebugInstrOperandPair, DebugInstrOperandPair);
+  void makeDebugValueSubstitution(DebugInstrOperandPair, DebugInstrOperandPair,
+                                  unsigned SubReg = 0);
 
   /// Create substitutions for any tracked values in \p Old, to point at
   /// \p New. Needed when we re-create an instruction during optimization,

include/llvm/CodeGen/MachineInstr.h

@@ -455,6 +455,11 @@ public:
   /// one already, a new and unique number will be assigned.
   unsigned getDebugInstrNum();
 
+  /// Fetch instruction number of this MachineInstr -- but before it's inserted
+  /// into \p MF. Needed for transformations that create an instruction but
+  /// don't immediately insert them.
+  unsigned getDebugInstrNum(MachineFunction &MF);
+
   /// Examine the instruction number of this MachineInstr. May be zero if
   /// it hasn't been assigned a number yet.
   unsigned peekDebugInstrNum() const { return DebugInstrNum; }

lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp

@@ -1830,8 +1830,8 @@ bool InstrRefBasedLDV::transferDebugInstrRef(MachineInstr &MI,
   // the instruction / operand number in this DBG_INSTR_REF.
   auto Sub = MF.DebugValueSubstitutions.find(std::make_pair(InstNo, OpNo));
   while (Sub != MF.DebugValueSubstitutions.end()) {
-    InstNo = Sub->second.first;
-    OpNo = Sub->second.second;
+    InstNo = Sub->second.Dest.first;
+    OpNo = Sub->second.Dest.second;
     Sub = MF.DebugValueSubstitutions.find(std::make_pair(InstNo, OpNo));
   }
 

lib/CodeGen/MIRParser/MIRParser.cpp

@@ -425,8 +425,8 @@ void MIRParserImpl::setupDebugValueTracking(
 
   // Load any substitutions.
   for (auto &Sub : YamlMF.DebugValueSubstitutions) {
-    MF.makeDebugValueSubstitution(std::make_pair(Sub.SrcInst, Sub.SrcOp),
-                                  std::make_pair(Sub.DstInst, Sub.DstOp));
+    MF.makeDebugValueSubstitution({Sub.SrcInst, Sub.SrcOp},
+                                  {Sub.DstInst, Sub.DstOp}, Sub.Subreg);
   }
 }
 

lib/CodeGen/MIRPrinter.cpp

@@ -224,10 +224,14 @@ void MIRPrinter::print(const MachineFunction &MF) {
   convert(MST, YamlMF.FrameInfo, MF.getFrameInfo());
   convertStackObjects(YamlMF, MF, MST);
   convertCallSiteObjects(YamlMF, MF, MST);
-  for (auto &Sub : MF.DebugValueSubstitutions)
-    YamlMF.DebugValueSubstitutions.push_back({Sub.first.first, Sub.first.second,
-                                              Sub.second.first,
-                                              Sub.second.second});
+  for (const auto &Sub : MF.DebugValueSubstitutions) {
+    auto &SubSrc = Sub.first;
+    const MachineFunction::DebugSubstitution &SubDest = Sub.second;
+    YamlMF.DebugValueSubstitutions.push_back({SubSrc.first, SubSrc.second,
+                                              SubDest.Dest.first,
+                                              SubDest.Dest.second,
+                                              SubDest.Subreg});
+  }
   if (const auto *ConstantPool = MF.getConstantPool())
     convert(YamlMF, *ConstantPool);
   if (const auto *JumpTableInfo = MF.getJumpTableInfo())

lib/CodeGen/MachineFunction.cpp

@@ -969,8 +969,11 @@ void MachineFunction::setDebugInstrNumberingCount(unsigned Num) {
 }
 
 void MachineFunction::makeDebugValueSubstitution(DebugInstrOperandPair A,
-                                                 DebugInstrOperandPair B) {
-  auto Result = DebugValueSubstitutions.insert(std::make_pair(A, B));
+                                                 DebugInstrOperandPair B,
+                                                 unsigned Subreg) {
+  // Catch any accidental self-loops.
+  assert(A.first != B.first);
+  auto Result = DebugValueSubstitutions.insert({A, {B, Subreg}});
   (void)Result;
   assert(Result.second && "Substitution for an already substituted value?");
 }

lib/CodeGen/MachineInstr.cpp

@@ -2374,3 +2374,9 @@ unsigned MachineInstr::getDebugInstrNum() {
     DebugInstrNum = getParent()->getParent()->getNewDebugInstrNum();
   return DebugInstrNum;
 }
+
+unsigned MachineInstr::getDebugInstrNum(MachineFunction &MF) {
+  if (DebugInstrNum == 0)
+    DebugInstrNum = MF.getNewDebugInstrNum();
+  return DebugInstrNum;
+}

lib/Target/X86/X86FixupBWInsts.cpp

@@ -137,6 +137,8 @@ private:
   /// Machine instruction info used throughout the class.
   const X86InstrInfo *TII = nullptr;
 
+  const TargetRegisterInfo *TRI = nullptr;
+
   /// Local member for function's OptForSize attribute.
   bool OptForSize = false;
 
@@ -162,6 +164,7 @@ bool FixupBWInstPass::runOnMachineFunction(MachineFunction &MF) {
 
   this->MF = &MF;
   TII = MF.getSubtarget<X86Subtarget>().getInstrInfo();
+  TRI = MF.getRegInfo().getTargetRegisterInfo();
   MLI = &getAnalysis<MachineLoopInfo>();
   PSI = &getAnalysis<ProfileSummaryInfoWrapperPass>().getPSI();
   MBFI = (PSI && PSI->hasProfileSummary()) ?
@@ -303,6 +306,14 @@ MachineInstr *FixupBWInstPass::tryReplaceLoad(unsigned New32BitOpcode,
 
   MIB.setMemRefs(MI->memoperands());
 
+  // If it was debug tracked, record a substitution.
+  if (unsigned OldInstrNum = MI->peekDebugInstrNum()) {
+    unsigned Subreg = TRI->getSubRegIndex(MIB->getOperand(0).getReg(),
+                                          MI->getOperand(0).getReg());
+    unsigned NewInstrNum = MIB->getDebugInstrNum(*MF);
+    MF->makeDebugValueSubstitution({OldInstrNum, 0}, {NewInstrNum, 0}, Subreg);
+  }
+
   return MIB;
 }
 
@@ -366,6 +377,13 @@ MachineInstr *FixupBWInstPass::tryReplaceExtend(unsigned New32BitOpcode,
 
   MIB.setMemRefs(MI->memoperands());
 
+  if (unsigned OldInstrNum = MI->peekDebugInstrNum()) {
+    unsigned Subreg = TRI->getSubRegIndex(MIB->getOperand(0).getReg(),
+                                          MI->getOperand(0).getReg());
+    unsigned NewInstrNum = MIB->getDebugInstrNum(*MF);
+    MF->makeDebugValueSubstitution({OldInstrNum, 0}, {NewInstrNum, 0}, Subreg);
+  }
+
   return MIB;
 }
 

test/DebugInfo/MIR/InstrRef/livedebugvalues_instrref_tolocs.mir

@@ -32,7 +32,7 @@
 ---
 name: _Z8bb_to_bb
 debugValueSubstitutions:
-  - { srcinst: 4, srcop: 0, dstinst: 3, dstop: 0 }
+  - { srcinst: 4, srcop: 0, dstinst: 3, dstop: 0, subreg: 0 }
 body:  |
   bb.0.entry:
     $rax = MOV64ri 1, debug-instr-number 1, debug-location !17

test/DebugInfo/MIR/InstrRef/substitusions-roundtrip.mir

@@ -4,7 +4,7 @@
 # REQUIRES: x86-registered-target
 #
 # CHECK:      debugValueSubstitutions:
-# CHECK-NEXT: - { srcinst: 1, srcop: 0, dstinst: 2, dstop: 0 }
+# CHECK-NEXT: - { srcinst: 1, srcop: 0, dstinst: 2, dstop: 0, subreg: 0 }
 #
 # CHECK:      MOV64rr $rdi, debug-instr-number 2
 # CHECK-NEXT: DBG_INSTR_REF 1, 0
@@ -14,7 +14,7 @@ tracksRegLiveness: true
 liveins:
   - { reg: '$rdi', virtual-reg: '' }
 debugValueSubstitutions:
-  - { srcinst: 1, srcop: 0, dstinst: 2, dstop: 0 }
+  - { srcinst: 1, srcop: 0, dstinst: 2, dstop: 0, subreg: 0 }
 body:  |
   bb.0:
   liveins: $rdi, $rax

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

@@ -8,7 +8,7 @@
 # lets not.
 #
 # CHECK:      debugValueSubstitutions:
-# CHECK-NEXT:  - { srcinst: 1, srcop: 0, dstinst: 2, dstop: 0 }
+# CHECK-NEXT:  - { srcinst: 1, srcop: 0, dstinst: 2, dstop: 0, subreg: 0 }
 #
 # CHECK:      LEA64_32r
 # CHECK-SAME: debug-instr-number 2

test/DebugInfo/MIR/InstrRef/x86-fixup-bw-inst-subreb.mir

@@ -0,0 +1,65 @@
+# RUN: llc -mtriple=x86_64-unknown-linux-gnu -run-pass x86-fixup-bw-insts %s -o - -experimental-debug-variable-locations | FileCheck  %s
+#
+# This test is a copy of llvm/test/CodeGen/X86/fixup-bw-inst.mir, with a few
+# test bodies removed. The pass promotes certain register operations to be
+# wider operations (such as loads and sign extensions), which has an instruction
+# encoding benefit. New instructions are created, and so should have a debug 
+# instruction number substitution; but in addition a qualifiying subregister,
+# because the newly def'd register is a different size to the old one.
+#
+# Plain copies that get transformed are not tested for, as they should never
+# be instrumented. At a high level, copies do not define a value; they move
+# them.
+
+---
+# CHECK-LABEL: name: test1
+name:            test1
+alignment:       16
+tracksRegLiveness: true
+liveins:
+  - { reg: '$rax' }
+# CHECK:     debugValueSubstitutions:
+# CHECK-NEXT - { srcinst: 1, srcop: 0, dstinst: 2, dstop: 0, subreg: 4 }
+## Subreg 4 -> sub_16bit
+body:             |
+  bb.0:
+    liveins: $rax
+
+    $ax = MOV16rm killed $rax, 1, $noreg, 0, $noreg, debug-instr-number 1
+    ; CHECK: $eax = MOVZX32rm16 killed $rax, {{.*}} debug-instr-number 2
+
+    RETQ $ax
+
+...
+---
+# CHECK-LABEL: name: test3
+name:            test3
+alignment:       16
+tracksRegLiveness: true
+liveins:
+  - { reg: '$rdi' }
+# CHECK:     debugValueSubstitutions:
+# CHECK-NEXT - { srcinst: 1, srcop: 0, dstinst: 2, dstop: 0, subreg: 4 }
+## Subreg 4 -> sub_16bit
+body:             |
+  bb.0:
+    successors: %bb.1(0x30000000), %bb.2(0x50000000)
+    liveins: $rdi
+
+    TEST64rr $rdi, $rdi, implicit-def $eflags
+    JCC_1 %bb.1, 4, implicit $eflags
+
+  bb.2:
+    liveins: $rdi
+
+    $ax = MOV16rm killed $rdi, 1, $noreg, 0, $noreg, implicit-def $eax, debug-instr-number 1
+    ; CHECK: $eax = MOVZX32rm16 killed $rdi, {{.*}} debug-instr-number 2
+    $ax = KILL $ax, implicit killed $eax
+    RETQ $ax
+
+  bb.1:
+    $eax = XOR32rr undef $eax, undef $eax, implicit-def dead $eflags
+    $ax = KILL $ax, implicit killed $eax
+    RETQ $ax
+
+...