[llvm] 2ec0283 - [DebugInfo][RemoveDIs] Emulate inserting insts in dbg.value sequences (#73350)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 30 04:41:58 PST 2023
Author: Jeremy Morse
Date: 2023-11-30T12:41:52Z
New Revision: 2ec0283c0405535594a46ea4244b09e63a797f0b
URL: https://github.com/llvm/llvm-project/commit/2ec0283c0405535594a46ea4244b09e63a797f0b
DIFF: https://github.com/llvm/llvm-project/commit/2ec0283c0405535594a46ea4244b09e63a797f0b.diff
LOG: [DebugInfo][RemoveDIs] Emulate inserting insts in dbg.value sequences (#73350)
Here's a problem for the RemoveDIs project to make debug-info not be
stored in instructions -- in the following sequence:
dbg.value(foo
%bar = add i32 ...
dbg.value(baz
It's possible for rare passes (only CodeGenPrepare) to remove the add
instruction, and then re-insert it back in the same place. When
debug-info is stored in instructions and there's a total order on "when"
things happen this is easy, but by moving that information out of the
instruction stream we start having to do manual maintenance.
This patch adds some utilities for re-inserting an instruction into a
sequence of DPValue objects. Someday we hope to design this away, but
for now it's necessary to support all the things you can do with
dbg.values. The two unit tests show how DPValues get shuffled around
using the relevant function calls. A follow-up patch adds
instrumentation to CodeGenPrepare.
Added:
Modified:
llvm/include/llvm/IR/BasicBlock.h
llvm/include/llvm/IR/DebugProgramInstruction.h
llvm/include/llvm/IR/Instruction.h
llvm/lib/IR/BasicBlock.cpp
llvm/lib/IR/DebugProgramInstruction.cpp
llvm/lib/IR/Instruction.cpp
llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h
index ec916acc25151c8..45e4b577c8a992d 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -141,6 +141,14 @@ class BasicBlock final : public Value, // Basic blocks are data objects also
/// move such DPValues back to the right place (ahead of the terminator).
void flushTerminatorDbgValues();
+ /// In rare circumstances instructions can be speculatively removed from
+ /// blocks, and then be re-inserted back into that position later. When this
+ /// happens in RemoveDIs debug-info mode, some special patching-up needs to
+ /// occur: inserting into the middle of a sequence of dbg.value intrinsics
+ /// does not have an equivalent with DPValues.
+ void reinsertInstInDPValues(Instruction *I,
+ std::optional<DPValue::self_iterator> Pos);
+
private:
void setParent(Function *parent);
diff --git a/llvm/include/llvm/IR/DebugProgramInstruction.h b/llvm/include/llvm/IR/DebugProgramInstruction.h
index cfee2a87b75c7b5..cd5d37a078015be 100644
--- a/llvm/include/llvm/IR/DebugProgramInstruction.h
+++ b/llvm/include/llvm/IR/DebugProgramInstruction.h
@@ -308,6 +308,11 @@ class DPMarker {
/// Transfer any DPValues from \p Src into this DPMarker. If \p InsertAtHead
/// is true, place them before existing DPValues, otherwise afterwards.
void absorbDebugValues(DPMarker &Src, bool InsertAtHead);
+ /// Transfer the DPValues in \p Range from \p Src into this DPMarker. If
+ /// \p InsertAtHead is true, place them before existing DPValues, otherwise
+ // afterwards.
+ void absorbDebugValues(iterator_range<DPValue::self_iterator> Range,
+ DPMarker &Src, bool InsertAtHead);
/// Insert a DPValue into this DPMarker, at the end of the list. If
/// \p InsertAtHead is true, at the start.
void insertDPValue(DPValue *New, bool InsertAtHead);
diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h
index a676e1d4dea49d8..0211b5076131ce6 100644
--- a/llvm/include/llvm/IR/Instruction.h
+++ b/llvm/include/llvm/IR/Instruction.h
@@ -78,6 +78,11 @@ class Instruction : public User,
/// Return a range over the DPValues attached to this instruction.
iterator_range<simple_ilist<DPValue>::iterator> getDbgValueRange() const;
+ /// Return an iterator to the position of the "Next" DPValue after this
+ /// instruction, or std::nullopt. This is the position to pass to
+ /// BasicBlock::reinsertInstInDPValues when re-inserting an instruction.
+ std::optional<simple_ilist<DPValue>::iterator> getDbgReinsertionPosition();
+
/// Returns true if any DPValues are attached to this instruction.
bool hasDbgValues() const;
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index 6c08ca1efc65288..82868fb69f30044 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -1013,6 +1013,58 @@ DPMarker *BasicBlock::getMarker(InstListType::iterator It) {
return It->DbgMarker;
}
+void BasicBlock::reinsertInstInDPValues(
+ Instruction *I, std::optional<DPValue::self_iterator> Pos) {
+ // "I" was originally removed from a position where it was
+ // immediately in front of Pos. Any DPValues on that position then "fell down"
+ // onto Pos. "I" has been re-inserted at the front of that wedge of DPValues,
+ // shuffle them around to represent the original positioning. To illustrate:
+ //
+ // Instructions: I1---I---I0
+ // DPValues: DDD DDD
+ //
+ // Instruction "I" removed,
+ //
+ // Instructions: I1------I0
+ // DPValues: DDDDDD
+ // ^Pos
+ //
+ // Instruction "I" re-inserted (now):
+ //
+ // Instructions: I1---I------I0
+ // DPValues: DDDDDD
+ // ^Pos
+ //
+ // After this method completes:
+ //
+ // Instructions: I1---I---I0
+ // DPValues: DDD DDD
+
+ // This happens if there were no DPValues on I0. Are there now DPValues there?
+ if (!Pos) {
+ DPMarker *NextMarker = getNextMarker(I);
+ if (!NextMarker)
+ return;
+ if (NextMarker->StoredDPValues.empty())
+ return;
+ // There are DPMarkers there now -- they fell down from "I".
+ DPMarker *ThisMarker = createMarker(I);
+ ThisMarker->absorbDebugValues(*NextMarker, false);
+ return;
+ }
+
+ // Is there even a range of DPValues to move?
+ DPMarker *DPM = (*Pos)->getMarker();
+ auto Range = make_range(DPM->StoredDPValues.begin(), (*Pos));
+ if (Range.begin() == Range.end())
+ return;
+
+ // Otherwise: splice.
+ DPMarker *ThisMarker = createMarker(I);
+ assert(ThisMarker->StoredDPValues.empty());
+ ThisMarker->absorbDebugValues(Range, *DPM, true);
+}
+
#ifndef NDEBUG
/// In asserts builds, this checks the numbering. In non-asserts builds, it
/// is defined as a no-op inline function in BasicBlock.h.
diff --git a/llvm/lib/IR/DebugProgramInstruction.cpp b/llvm/lib/IR/DebugProgramInstruction.cpp
index 581d77a26acb80a..6a4ee9d6101076f 100644
--- a/llvm/lib/IR/DebugProgramInstruction.cpp
+++ b/llvm/lib/IR/DebugProgramInstruction.cpp
@@ -334,6 +334,18 @@ void DPMarker::absorbDebugValues(DPMarker &Src, bool InsertAtHead) {
StoredDPValues.splice(It, Src.StoredDPValues);
}
+void DPMarker::absorbDebugValues(iterator_range<DPValue::self_iterator> Range,
+ DPMarker &Src, bool InsertAtHead) {
+ for (DPValue &DPV : Range)
+ DPV.setMarker(this);
+
+ auto InsertPos =
+ (InsertAtHead) ? StoredDPValues.begin() : StoredDPValues.end();
+
+ StoredDPValues.splice(InsertPos, Src.StoredDPValues, Range.begin(),
+ Range.end());
+}
+
iterator_range<simple_ilist<DPValue>::iterator> DPMarker::cloneDebugInfoFrom(
DPMarker *From, std::optional<simple_ilist<DPValue>::iterator> from_here,
bool InsertAtHead) {
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index b6a422477a672c6..4b5349856b8d7bf 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -254,6 +254,19 @@ Instruction::getDbgValueRange() const {
return DbgMarker->getDbgValueRange();
}
+std::optional<DPValue::self_iterator> Instruction::getDbgReinsertionPosition() {
+ // Is there a marker on the next instruction?
+ DPMarker *NextMarker = getParent()->getNextMarker(this);
+ if (!NextMarker)
+ return std::nullopt;
+
+ // Are there any DPValues in the next marker?
+ if (NextMarker->StoredDPValues.empty())
+ return std::nullopt;
+
+ return NextMarker->StoredDPValues.begin();
+}
+
bool Instruction::hasDbgValues() const { return !getDbgValueRange().empty(); }
void Instruction::dropDbgValues() {
diff --git a/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp b/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
index 481cd181d3848e7..a8ca706cc95b9b5 100644
--- a/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
+++ b/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
@@ -1110,5 +1110,160 @@ TEST(BasicBlockDbgInfoTest, DbgSpliceTrailing) {
UseNewDbgInfoFormat = false;
}
+// When we remove instructions from the program, adjacent DPValues coalesce
+// together into one DPMarker. In "old" dbg.value mode you could re-insert
+// the removed instruction back into the middle of a sequence of dbg.values.
+// Test that this can be replicated correctly by DPValues
+TEST(BasicBlockDbgInfoTest, RemoveInstAndReinsert) {
+ LLVMContext C;
+ UseNewDbgInfoFormat = true;
+
+ std::unique_ptr<Module> M = parseIR(C, R"(
+ define i16 @f(i16 %a) !dbg !6 {
+ entry:
+ %qux = sub i16 %a, 0
+ call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+ %foo = add i16 %a, %a
+ call void @llvm.dbg.value(metadata i16 0, metadata !9, metadata !DIExpression()), !dbg !11
+ ret i16 1
+ }
+ declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+ !llvm.dbg.cu = !{!0}
+ !llvm.module.flags = !{!5}
+
+ !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+ !1 = !DIFile(filename: "t.ll", directory: "/")
+ !2 = !{}
+ !5 = !{i32 2, !"Debug Info Version", i32 3}
+ !6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+ !7 = !DISubroutineType(types: !2)
+ !8 = !{!9}
+ !9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
+ !10 = !DIBasicType(name: "ty16", size: 16, encoding: DW_ATE_unsigned)
+ !11 = !DILocation(line: 1, column: 1, scope: !6)
+)");
+
+ BasicBlock &Entry = M->getFunction("f")->getEntryBlock();
+ M->convertToNewDbgValues();
+
+ // Fetch the relevant instructions from the converted function.
+ Instruction *SubInst = &*Entry.begin();
+ ASSERT_TRUE(isa<BinaryOperator>(SubInst));
+ Instruction *AddInst = SubInst->getNextNode();
+ ASSERT_TRUE(isa<BinaryOperator>(AddInst));
+ Instruction *RetInst = AddInst->getNextNode();
+ ASSERT_TRUE(isa<ReturnInst>(RetInst));
+
+ // add and sub should both have one DPValue on add and ret.
+ EXPECT_FALSE(SubInst->hasDbgValues());
+ EXPECT_TRUE(AddInst->hasDbgValues());
+ EXPECT_TRUE(RetInst->hasDbgValues());
+ auto R1 = AddInst->getDbgValueRange();
+ EXPECT_EQ(std::distance(R1.begin(), R1.end()), 1u);
+ auto R2 = RetInst->getDbgValueRange();
+ EXPECT_EQ(std::distance(R2.begin(), R2.end()), 1u);
+
+ // The Supported (TM) code sequence for removing then reinserting insts
+ // after another instruction:
+ std::optional<DPValue::self_iterator> Pos =
+ AddInst->getDbgReinsertionPosition();
+ AddInst->removeFromParent();
+
+ // We should have a re-insertion position.
+ ASSERT_TRUE(Pos);
+ // Both DPValues should now be attached to the ret inst.
+ auto R3 = RetInst->getDbgValueRange();
+ EXPECT_EQ(std::distance(R3.begin(), R3.end()), 2u);
+
+ // Re-insert and re-insert.
+ AddInst->insertAfter(SubInst);
+ Entry.reinsertInstInDPValues(AddInst, Pos);
+ // We should be back into a position of having one DPValue on add and ret.
+ EXPECT_FALSE(SubInst->hasDbgValues());
+ EXPECT_TRUE(AddInst->hasDbgValues());
+ EXPECT_TRUE(RetInst->hasDbgValues());
+ auto R4 = AddInst->getDbgValueRange();
+ EXPECT_EQ(std::distance(R4.begin(), R4.end()), 1u);
+ auto R5 = RetInst->getDbgValueRange();
+ EXPECT_EQ(std::distance(R5.begin(), R5.end()), 1u);
+
+ UseNewDbgInfoFormat = false;
+}
+
+// Test instruction removal and re-insertion, this time with one DPValue that
+// should hop up one instruction.
+TEST(BasicBlockDbgInfoTest, RemoveInstAndReinsertForOneDPValue) {
+ LLVMContext C;
+ UseNewDbgInfoFormat = true;
+
+ std::unique_ptr<Module> M = parseIR(C, R"(
+ define i16 @f(i16 %a) !dbg !6 {
+ entry:
+ %qux = sub i16 %a, 0
+ call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+ %foo = add i16 %a, %a
+ ret i16 1
+ }
+ declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+ !llvm.dbg.cu = !{!0}
+ !llvm.module.flags = !{!5}
+
+ !0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+ !1 = !DIFile(filename: "t.ll", directory: "/")
+ !2 = !{}
+ !5 = !{i32 2, !"Debug Info Version", i32 3}
+ !6 = distinct !DISubprogram(name: "foo", linkageName: "foo", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !8)
+ !7 = !DISubroutineType(types: !2)
+ !8 = !{!9}
+ !9 = !DILocalVariable(name: "1", scope: !6, file: !1, line: 1, type: !10)
+ !10 = !DIBasicType(name: "ty16", size: 16, encoding: DW_ATE_unsigned)
+ !11 = !DILocation(line: 1, column: 1, scope: !6)
+)");
+
+ BasicBlock &Entry = M->getFunction("f")->getEntryBlock();
+ M->convertToNewDbgValues();
+
+ // Fetch the relevant instructions from the converted function.
+ Instruction *SubInst = &*Entry.begin();
+ ASSERT_TRUE(isa<BinaryOperator>(SubInst));
+ Instruction *AddInst = SubInst->getNextNode();
+ ASSERT_TRUE(isa<BinaryOperator>(AddInst));
+ Instruction *RetInst = AddInst->getNextNode();
+ ASSERT_TRUE(isa<ReturnInst>(RetInst));
+
+ // There should be one DPValue.
+ EXPECT_FALSE(SubInst->hasDbgValues());
+ EXPECT_TRUE(AddInst->hasDbgValues());
+ EXPECT_FALSE(RetInst->hasDbgValues());
+ auto R1 = AddInst->getDbgValueRange();
+ EXPECT_EQ(std::distance(R1.begin(), R1.end()), 1u);
+
+ // The Supported (TM) code sequence for removing then reinserting insts:
+ std::optional<DPValue::self_iterator> Pos =
+ AddInst->getDbgReinsertionPosition();
+ AddInst->removeFromParent();
+
+ // No re-insertion position as there were no DPValues on the ret.
+ ASSERT_FALSE(Pos);
+ // The single DPValue should now be attached to the ret inst.
+ EXPECT_TRUE(RetInst->hasDbgValues());
+ auto R2 = RetInst->getDbgValueRange();
+ EXPECT_EQ(std::distance(R2.begin(), R2.end()), 1u);
+
+ // Re-insert and re-insert.
+ AddInst->insertAfter(SubInst);
+ Entry.reinsertInstInDPValues(AddInst, Pos);
+ // We should be back into a position of having one DPValue on the AddInst.
+ EXPECT_FALSE(SubInst->hasDbgValues());
+ EXPECT_TRUE(AddInst->hasDbgValues());
+ EXPECT_FALSE(RetInst->hasDbgValues());
+ auto R3 = AddInst->getDbgValueRange();
+ EXPECT_EQ(std::distance(R3.begin(), R3.end()), 1u);
+
+ UseNewDbgInfoFormat = false;
+}
+
} // End anonymous namespace.
#endif // EXPERIMENTAL_DEBUGINFO_ITERATORS
More information about the llvm-commits
mailing list