[llvm] [DebugInfo][RemoveDIs] Cope with instructions moving after themselves (PR #74113)
Jeremy Morse via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 5 06:04:51 PST 2023
https://github.com/jmorse updated https://github.com/llvm/llvm-project/pull/74113
>From f82d3962ee4144b86f4eeb1a78c25eddefcc0c3a Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Fri, 1 Dec 2023 14:21:42 +0000
Subject: [PATCH 1/3] [DebugInfo][RemoveDIs] Cope with instructions moving
after themselves
We occasionally move instructions to their own positions, which is not an
error, and has no effect on the program. However, if dbg.value intrinsics
are present then they can effectively be moved without any other effect on
the program.
This is a problem if we're using non-instruction debug-info: a moveAfter
that appears to be a no-op in RemoveDIs mode can jump in front of
intrinsics in dbg.value mode. Thus: if an instruction is moved to itself
and the head bit is set, force attached debug-info to shift down one
instruction, which replicates the dbg.value behaviour.
---
llvm/lib/IR/Instruction.cpp | 5 +-
llvm/unittests/IR/BasicBlockDbgInfoTest.cpp | 73 +++++++++++++++++++++
2 files changed, 76 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 4b5349856b8d7..7144924c34570 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -200,8 +200,9 @@ void Instruction::moveBeforeImpl(BasicBlock &BB, InstListType::iterator I,
// If we've been given the "Preserve" flag, then just move the DPValues with
// the instruction, no more special handling needed.
if (BB.IsNewDbgInfoFormat && DbgMarker && !Preserve) {
- if (I != this->getIterator()) {
- // "this" is definitely moving; detach any existing DPValues.
+ if (I != this->getIterator() || InsertAtHead) {
+ // "this" is definitely moving in the list, or it's moving ahead of it's
+ // attached DPValues. Detach any existing DPValues.
handleMarkerRemoval();
}
}
diff --git a/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp b/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
index a8ca706cc95b9..c914a5b6f2394 100644
--- a/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
+++ b/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
@@ -42,6 +42,79 @@ static std::unique_ptr<Module> parseIR(LLVMContext &C, const char *IR) {
namespace {
+// We can occasionally moveAfter an instruction so that it moves to the
+// position that it already resides at. This is fine -- but gets complicated
+// with dbg.value intrinsics. By moving an instruction, we can end up changing
+// nothing but the location of debug-info intrinsics. That has to be modelled
+// by DPValues, the dbg.value replacement.
+TEST(BasicBlockDbgInfoTest, InsertAfterSelf) {
+ LLVMContext C;
+ UseNewDbgInfoFormat = true;
+
+ std::unique_ptr<Module> M = parseIR(C, R"(
+ define i16 @f(i16 %a) !dbg !6 {
+ call void @llvm.dbg.value(metadata i16 %a, metadata !9, metadata !DIExpression()), !dbg !11
+ %b = add i16 %a, 1, !dbg !11
+ call void @llvm.dbg.value(metadata i16 %b, metadata !9, metadata !DIExpression()), !dbg !11
+ %c = add i16 %b, 1, !dbg !11
+ ret i16 0, !dbg !11
+ }
+ declare void @llvm.dbg.value(metadata, metadata, metadata) #0
+ attributes #0 = { nounwind readnone speculatable willreturn }
+
+ !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)
+)");
+
+ // Convert the module to "new" form debug-info.
+ M->convertToNewDbgValues();
+ // Fetch the entry block,
+ BasicBlock &BB = M->getFunction("f")->getEntryBlock();
+
+ Instruction *Inst1 = &*BB.begin();
+ Instruction *Inst2 = &*std::next(BB.begin());
+ Instruction *RetInst = &*std::next(Inst2->getIterator());
+ EXPECT_TRUE(Inst1->hasDbgValues());
+ EXPECT_TRUE(Inst2->hasDbgValues());
+ EXPECT_FALSE(RetInst->hasDbgValues());
+
+ // If we move Inst2 to be after Inst1, then it comes _immediately_ after. Were
+ // we in dbg.value form we would then have:
+ // dbg.value
+ // %b = add
+ // %c = add
+ // dbg.value
+ // Check that this is replicated by DPValues.
+ Inst2->moveAfter(Inst1);
+
+ // Inst1 should only have one DPValue on it.
+ EXPECT_TRUE(Inst1->hasDbgValues());
+ auto Range1 = Inst1->getDbgValueRange();
+ EXPECT_EQ(std::distance(Range1.begin(), Range1.end()), 1u);
+ // Inst2 should have none.
+ EXPECT_FALSE(Inst2->hasDbgValues());
+ // While the return inst should now have one on it.
+ EXPECT_TRUE(RetInst->hasDbgValues());
+ auto Range2 = RetInst->getDbgValueRange();
+ EXPECT_EQ(std::distance(Range2.begin(), Range2.end()), 1u);
+
+ M->convertFromNewDbgValues();
+
+ UseNewDbgInfoFormat = false;
+}
+
+
TEST(BasicBlockDbgInfoTest, MarkerOperations) {
LLVMContext C;
UseNewDbgInfoFormat = true;
>From 53c8fab79014edce34a33dbf5a633e60bebc5637 Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Tue, 5 Dec 2023 13:59:17 +0000
Subject: [PATCH 2/3] clang-format
---
llvm/unittests/IR/BasicBlockDbgInfoTest.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp b/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
index c914a5b6f2394..ea6be947bd152 100644
--- a/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
+++ b/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
@@ -114,7 +114,6 @@ TEST(BasicBlockDbgInfoTest, InsertAfterSelf) {
UseNewDbgInfoFormat = false;
}
-
TEST(BasicBlockDbgInfoTest, MarkerOperations) {
LLVMContext C;
UseNewDbgInfoFormat = true;
>From c37fd615d1ad3ba4ae1190b31d211742c7eca0be Mon Sep 17 00:00:00 2001
From: Jeremy Morse <jeremy.morse at sony.com>
Date: Tue, 5 Dec 2023 14:04:01 +0000
Subject: [PATCH 3/3] Comment fixes
---
llvm/lib/IR/Instruction.cpp | 2 +-
llvm/unittests/IR/BasicBlockDbgInfoTest.cpp | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp
index 7144924c34570..7c29555eb84ba 100644
--- a/llvm/lib/IR/Instruction.cpp
+++ b/llvm/lib/IR/Instruction.cpp
@@ -201,7 +201,7 @@ void Instruction::moveBeforeImpl(BasicBlock &BB, InstListType::iterator I,
// the instruction, no more special handling needed.
if (BB.IsNewDbgInfoFormat && DbgMarker && !Preserve) {
if (I != this->getIterator() || InsertAtHead) {
- // "this" is definitely moving in the list, or it's moving ahead of it's
+ // "this" is definitely moving in the list, or it's moving ahead of its
// attached DPValues. Detach any existing DPValues.
handleMarkerRemoval();
}
diff --git a/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp b/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
index ea6be947bd152..cf0a82de34698 100644
--- a/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
+++ b/llvm/unittests/IR/BasicBlockDbgInfoTest.cpp
@@ -79,7 +79,7 @@ TEST(BasicBlockDbgInfoTest, InsertAfterSelf) {
// Convert the module to "new" form debug-info.
M->convertToNewDbgValues();
- // Fetch the entry block,
+ // Fetch the entry block.
BasicBlock &BB = M->getFunction("f")->getEntryBlock();
Instruction *Inst1 = &*BB.begin();
More information about the llvm-commits
mailing list