[llvm] 47c3fe2 - [DebugInfo][InstrRef][1/4] Support transformations that widen values
Jeremy Morse via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 1 04:04:19 PDT 2021
Author: Jeremy Morse
Date: 2021-07-01T11:19:27+01:00
New Revision: 47c3fe2a22cf753fd55d08d367fbd817b4dd4a1c
URL: https://github.com/llvm/llvm-project/commit/47c3fe2a22cf753fd55d08d367fbd817b4dd4a1c
DIFF: https://github.com/llvm/llvm-project/commit/47c3fe2a22cf753fd55d08d367fbd817b4dd4a1c.diff
LOG: [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
Added:
llvm/test/DebugInfo/MIR/InstrRef/x86-fixup-bw-inst-subreb.mir
Modified:
llvm/include/llvm/CodeGen/MIRYamlMapping.h
llvm/include/llvm/CodeGen/MachineFunction.h
llvm/include/llvm/CodeGen/MachineInstr.h
llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
llvm/lib/CodeGen/MIRParser/MIRParser.cpp
llvm/lib/CodeGen/MIRPrinter.cpp
llvm/lib/CodeGen/MachineFunction.cpp
llvm/lib/CodeGen/MachineInstr.cpp
llvm/lib/Target/X86/X86FixupBWInsts.cpp
llvm/test/DebugInfo/MIR/InstrRef/livedebugvalues_instrref_tolocs.mir
llvm/test/DebugInfo/MIR/InstrRef/substitusions-roundtrip.mir
llvm/test/DebugInfo/MIR/InstrRef/twoaddr-to-threeaddr-sub.mir
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/MIRYamlMapping.h b/llvm/include/llvm/CodeGen/MIRYamlMapping.h
index 75f2ff86c29e7..e7428e7ad260a 100644
--- a/llvm/include/llvm/CodeGen/MIRYamlMapping.h
+++ b/llvm/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;
diff --git a/llvm/include/llvm/CodeGen/MachineFunction.h b/llvm/include/llvm/CodeGen/MachineFunction.h
index e9ce813428dc4..1d0a2a7deb761 100644
--- a/llvm/include/llvm/CodeGen/MachineFunction.h
+++ b/llvm/include/llvm/CodeGen/MachineFunction.h
@@ -451,11 +451,24 @@ class MachineFunction {
/// 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 @@ class MachineFunction {
/// Create a substitution between one <instr,operand> value to a
diff erent,
/// 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,
diff --git a/llvm/include/llvm/CodeGen/MachineInstr.h b/llvm/include/llvm/CodeGen/MachineInstr.h
index 12da008a7bf44..7fc1576fe5a09 100644
--- a/llvm/include/llvm/CodeGen/MachineInstr.h
+++ b/llvm/include/llvm/CodeGen/MachineInstr.h
@@ -455,6 +455,11 @@ class MachineInstr
/// 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; }
diff --git a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp b/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
index 1085564fa5d61..b92614ee124d0 100644
--- a/llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
+++ b/llvm/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));
}
diff --git a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp b/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
index 58ce95aaf023c..d77104752880a 100644
--- a/llvm/lib/CodeGen/MIRParser/MIRParser.cpp
+++ b/llvm/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);
}
}
diff --git a/llvm/lib/CodeGen/MIRPrinter.cpp b/llvm/lib/CodeGen/MIRPrinter.cpp
index c7dc73191889a..0c8da19e3f41f 100644
--- a/llvm/lib/CodeGen/MIRPrinter.cpp
+++ b/llvm/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())
diff --git a/llvm/lib/CodeGen/MachineFunction.cpp b/llvm/lib/CodeGen/MachineFunction.cpp
index 39feb92a9752f..8b1d05d252de9 100644
--- a/llvm/lib/CodeGen/MachineFunction.cpp
+++ b/llvm/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?");
}
diff --git a/llvm/lib/CodeGen/MachineInstr.cpp b/llvm/lib/CodeGen/MachineInstr.cpp
index 6a3bb0e78ff11..20d6ab88fac2f 100644
--- a/llvm/lib/CodeGen/MachineInstr.cpp
+++ b/llvm/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;
+}
diff --git a/llvm/lib/Target/X86/X86FixupBWInsts.cpp b/llvm/lib/Target/X86/X86FixupBWInsts.cpp
index f8d822aebc5b6..e1d4b4c347721 100644
--- a/llvm/lib/Target/X86/X86FixupBWInsts.cpp
+++ b/llvm/lib/Target/X86/X86FixupBWInsts.cpp
@@ -137,6 +137,8 @@ class FixupBWInstPass : public MachineFunctionPass {
/// 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;
}
diff --git a/llvm/test/DebugInfo/MIR/InstrRef/livedebugvalues_instrref_tolocs.mir b/llvm/test/DebugInfo/MIR/InstrRef/livedebugvalues_instrref_tolocs.mir
index 578cac9dc0ec4..13d9295ad656a 100644
--- a/llvm/test/DebugInfo/MIR/InstrRef/livedebugvalues_instrref_tolocs.mir
+++ b/llvm/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
diff --git a/llvm/test/DebugInfo/MIR/InstrRef/substitusions-roundtrip.mir b/llvm/test/DebugInfo/MIR/InstrRef/substitusions-roundtrip.mir
index cf0ebe3502cf7..0a8fada169cf8 100644
--- a/llvm/test/DebugInfo/MIR/InstrRef/substitusions-roundtrip.mir
+++ b/llvm/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
diff --git a/llvm/test/DebugInfo/MIR/InstrRef/twoaddr-to-threeaddr-sub.mir b/llvm/test/DebugInfo/MIR/InstrRef/twoaddr-to-threeaddr-sub.mir
index 55a8d6be2f0f9..561f73524bd06 100644
--- a/llvm/test/DebugInfo/MIR/InstrRef/twoaddr-to-threeaddr-sub.mir
+++ b/llvm/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
diff --git a/llvm/test/DebugInfo/MIR/InstrRef/x86-fixup-bw-inst-subreb.mir b/llvm/test/DebugInfo/MIR/InstrRef/x86-fixup-bw-inst-subreb.mir
new file mode 100644
index 0000000000000..27b155639af5d
--- /dev/null
+++ b/llvm/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
diff erent 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
+
+...
More information about the llvm-commits
mailing list