diff --git a/llvm/include/llvm/IR/BasicBlock.h b/llvm/include/llvm/IR/BasicBlock.h index ec916acc25151..45e4b577c8a99 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 Pos); + private: void setParent(Function *parent); diff --git a/llvm/include/llvm/IR/DebugProgramInstruction.h b/llvm/include/llvm/IR/DebugProgramInstruction.h index cfee2a87b75c7..cd5d37a078015 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 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 58fc32237367d..bea21ba27ed61 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::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::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 6c08ca1efc652..82868fb69f300 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 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 581d77a26acb8..6a4ee9d610107 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 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::iterator> DPMarker::cloneDebugInfoFrom( DPMarker *From, std::optional::iterator> from_here, bool InsertAtHead) { diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp index 7449692f05d7b..e2c78c60a75fe 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 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 481cd181d3848..a8ca706cc95b9 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 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(SubInst)); + Instruction *AddInst = SubInst->getNextNode(); + ASSERT_TRUE(isa(AddInst)); + Instruction *RetInst = AddInst->getNextNode(); + ASSERT_TRUE(isa(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 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 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(SubInst)); + Instruction *AddInst = SubInst->getNextNode(); + ASSERT_TRUE(isa(AddInst)); + Instruction *RetInst = AddInst->getNextNode(); + ASSERT_TRUE(isa(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 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