[llvm] 854b1bc - [DebugInfo] getMergedLocation: Maintain the line number if they match
Juan Manuel MARTINEZ CAAMAÑO via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 25 07:13:50 PDT 2022
Author: Juan Manuel MARTINEZ CAAMAÑO
Date: 2022-10-25T14:10:21Z
New Revision: 854b1bca60ebb784e507e905885f4ba664bb65af
URL: https://github.com/llvm/llvm-project/commit/854b1bca60ebb784e507e905885f4ba664bb65af
DIFF: https://github.com/llvm/llvm-project/commit/854b1bca60ebb784e507e905885f4ba664bb65af.diff
LOG: [DebugInfo] getMergedLocation: Maintain the line number if they match
getMergedLocation returns a 'line 0' DILocaiton if the two locations
being merged don't perfecly match, even if they are in the same line but
a different column.
This commit adds support to keep the line number if it matches (but only
the column differs). The merged column number is the leftmost between the
two.
Reviewed By: dblaikie, orlando
Differential Revision: https://reviews.llvm.org/D135166
Added:
llvm/test/DebugInfo/return-same-line-merge.ll
Modified:
llvm/include/llvm/IR/DebugInfoMetadata.h
llvm/lib/IR/DebugInfoMetadata.cpp
llvm/unittests/IR/MetadataTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index 8fc3f54bf2813..5b20bf3ade99a 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -1710,18 +1710,18 @@ class DILocation : public MDNode {
/// When two instructions are combined into a single instruction we also
/// need to combine the original locations into a single location.
+ /// When the locations are the same we can use either location.
+ /// When they
diff er, we need a third location which is distinct from either.
+ /// If they share a common scope, use this scope and compare the line/column
+ /// pair of the locations with the common scope:
+ /// * if both match, keep the line and column;
+ /// * if only the line number matches, keep the line and set the column as 0;
+ /// * otherwise set line and column as 0.
+ /// If they do not share a common scope the location is ambiguous and can't be
+ /// represented in a line entry. In this case, set line and column as 0 and
+ /// use the scope of any location.
///
- /// When the locations are the same we can use either location. When they
- ///
diff er, we need a third location which is distinct from either. If they
- /// have the same file/line but have a
diff erent discriminator we could
- /// create a location with a new discriminator. If they are from
diff erent
- /// files/lines the location is ambiguous and can't be represented in a line
- /// entry. In this case, if \p GenerateLocation is true, we will set the
- /// merged debug location as line 0 of the nearest common scope where the two
- /// locations are inlined from.
- ///
- /// \p GenerateLocation: Whether the merged location can be generated when
- /// \p LocA and \p LocB
diff er.
+ /// \p LocA \p LocB: The locations to be merged.
static const DILocation *getMergedLocation(const DILocation *LocA,
const DILocation *LocB);
diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index b29daeb653f0b..99c445fc2842a 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -112,34 +112,64 @@ const DILocation *DILocation::getMergedLocation(const DILocation *LocA,
if (LocA == LocB)
return LocA;
- SmallSet<std::pair<DIScope *, DILocation *>, 5> Locations;
+ LLVMContext &C = LocA->getContext();
+ SmallDenseMap<std::pair<DILocalScope *, DILocation *>,
+ std::pair<unsigned, unsigned>, 4>
+ Locations;
+
DIScope *S = LocA->getScope();
DILocation *L = LocA->getInlinedAt();
- while (S) {
- Locations.insert(std::make_pair(S, L));
+ unsigned Line = LocA->getLine();
+ unsigned Col = LocA->getColumn();
+
+ // Walk from the current source locaiton until the file scope;
+ // then, do the same for the inlined-at locations.
+ auto AdvanceToParentLoc = [&S, &L, &Line, &Col]() {
S = S->getScope();
if (!S && L) {
+ Line = L->getLine();
+ Col = L->getColumn();
S = L->getScope();
L = L->getInlinedAt();
}
+ };
+
+ while (S) {
+ if (auto *LS = dyn_cast<DILocalScope>(S))
+ Locations.try_emplace(std::make_pair(LS, L), std::make_pair(Line, Col));
+ AdvanceToParentLoc();
}
+
+ // Walk the source locations of LocB until a match with LocA is found.
S = LocB->getScope();
L = LocB->getInlinedAt();
+ Line = LocB->getLine();
+ Col = LocB->getColumn();
while (S) {
- if (Locations.count(std::make_pair(S, L)))
- break;
- S = S->getScope();
- if (!S && L) {
- S = L->getScope();
- L = L->getInlinedAt();
+ if (auto *LS = dyn_cast<DILocalScope>(S)) {
+ auto MatchLoc = Locations.find(std::make_pair(LS, L));
+ if (MatchLoc != Locations.end()) {
+ // If the lines match, keep the line, but set the column to '0'
+ // If the lines don't match, pick a "line 0" location but keep
+ // the current scope and inlined-at.
+ bool SameLine = Line == MatchLoc->second.first;
+ bool SameCol = Col == MatchLoc->second.second;
+ Line = SameLine ? Line : 0;
+ Col = SameLine && SameCol ? Col : 0;
+ break;
+ }
}
+ AdvanceToParentLoc();
}
- // If the two locations are irreconsilable, just pick one. This is misleading,
- // but on the other hand, it's a "line 0" location.
- if (!S || !isa<DILocalScope>(S))
+ if (!S) {
+ // If the two locations are irreconsilable, pick any scope,
+ // and return a "line 0" location.
+ Line = Col = 0;
S = LocA->getScope();
- return DILocation::get(LocA->getContext(), 0, 0, S, L);
+ }
+
+ return DILocation::get(C, Line, Col, S, L);
}
Optional<unsigned> DILocation::encodeDiscriminator(unsigned BD, unsigned DF,
diff --git a/llvm/test/DebugInfo/return-same-line-merge.ll b/llvm/test/DebugInfo/return-same-line-merge.ll
new file mode 100644
index 0000000000000..5c9965b6447f9
--- /dev/null
+++ b/llvm/test/DebugInfo/return-same-line-merge.ll
@@ -0,0 +1,39 @@
+; RUN: opt -simplifycfg -S < %s | FileCheck %s
+;
+; Simplified from the following code:
+; int foo() {
+; if(c) { return a; } else { return b; }
+; }
+define i32 @foo(i32 %c, i32 %a, i32 %b) !dbg !4 {
+; CHECK: define i32 @foo({{.*}}) !dbg [[FOO_SUBPROGRAM:![0-9]+]]
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i32 [[C:%.*]], 0, !dbg [[DBG_CMP:![0-9]+]]
+; CHECK-NEXT: [[A_B:%.*]] = select i1 [[TOBOOL]], i32 [[A:%.*]], i32 [[B:%.*]]
+; CHECK-NEXT: ret i32 [[A_B]], !dbg [[DBG_RET:![0-9]+]]
+; CHECK: [[DBG_CMP]] = !DILocation(line: 2, column: 1, scope: [[FOO_SUBPROGRAM]])
+; CHECK: [[DBG_RET]] = !DILocation(line: 4, scope: [[FOO_SUBPROGRAM]])
+entry:
+ %tobool = icmp ne i32 %c, 0, !dbg !7
+ br i1 %tobool, label %cond.true, label %cond.false, !dbg !8
+
+cond.true: ; preds = %entry
+ ret i32 %a, !dbg !9
+
+cond.false: ; preds = %entry
+ ret i32 %b, !dbg !10
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 16.0.0)", isOptimized: false, runtimeVersion: 0, emissionKind: LineTablesOnly, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "test.c", directory: "/")
+!2 = !{i32 7, !"Dwarf Version", i32 5}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 1, type: !5, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !6)
+!5 = !DISubroutineType(types: !6)
+!6 = !{}
+!7 = !DILocation(line: 2, column: 1, scope: !4)
+!8 = !DILocation(line: 3, column: 1, scope: !4)
+!9 = !DILocation(line: 4, column: 1, scope: !4)
+!10 = !DILocation(line: 4, column: 2, scope: !4)
diff --git a/llvm/unittests/IR/MetadataTest.cpp b/llvm/unittests/IR/MetadataTest.cpp
index 9dd49d04405f5..126ae03ff9877 100644
--- a/llvm/unittests/IR/MetadataTest.cpp
+++ b/llvm/unittests/IR/MetadataTest.cpp
@@ -917,31 +917,6 @@ TEST_F(MDNodeTest, deleteTemporaryWithTrackingRef) {
typedef MetadataTest DILocationTest;
-TEST_F(DILocationTest, Overflow) {
- DISubprogram *N = getSubprogram();
- {
- DILocation *L = DILocation::get(Context, 2, 7, N);
- EXPECT_EQ(2u, L->getLine());
- EXPECT_EQ(7u, L->getColumn());
- }
- unsigned U16 = 1u << 16;
- {
- DILocation *L = DILocation::get(Context, UINT32_MAX, U16 - 1, N);
- EXPECT_EQ(UINT32_MAX, L->getLine());
- EXPECT_EQ(U16 - 1, L->getColumn());
- }
- {
- DILocation *L = DILocation::get(Context, UINT32_MAX, U16, N);
- EXPECT_EQ(UINT32_MAX, L->getLine());
- EXPECT_EQ(0u, L->getColumn());
- }
- {
- DILocation *L = DILocation::get(Context, UINT32_MAX, U16 + 1, N);
- EXPECT_EQ(UINT32_MAX, L->getLine());
- EXPECT_EQ(0u, L->getColumn());
- }
-}
-
TEST_F(DILocationTest, Merge) {
DISubprogram *N = getSubprogram();
DIScope *S = DILexicalBlock::get(Context, N, getFile(), 3, 4);
@@ -961,11 +936,24 @@ TEST_F(DILocationTest, Merge) {
auto *A = DILocation::get(Context, 2, 7, N);
auto *B = DILocation::get(Context, 2, 7, S);
auto *M = DILocation::getMergedLocation(A, B);
- EXPECT_EQ(0u, M->getLine()); // FIXME: Should this be 2?
- EXPECT_EQ(0u, M->getColumn()); // FIXME: Should this be 7?
+ EXPECT_EQ(2u, M->getLine());
+ EXPECT_EQ(7u, M->getColumn());
EXPECT_EQ(N, M->getScope());
}
+ {
+ // Same line,
diff erent column.
+ auto *A = DILocation::get(Context, 2, 7, N);
+ auto *B = DILocation::get(Context, 2, 10, S);
+ auto *M0 = DILocation::getMergedLocation(A, B);
+ auto *M1 = DILocation::getMergedLocation(B, A);
+ for (auto *M : {M0, M1}) {
+ EXPECT_EQ(2u, M->getLine());
+ EXPECT_EQ(0u, M->getColumn());
+ EXPECT_EQ(N, M->getScope());
+ }
+ }
+
{
// Different lines, same scopes.
auto *A = DILocation::get(Context, 1, 6, N);
@@ -998,15 +986,36 @@ TEST_F(DILocationTest, Merge) {
auto *I = DILocation::get(Context, 2, 7, N);
auto *A = DILocation::get(Context, 1, 6, SP1, I);
- auto *B = DILocation::get(Context, 2, 7, SP2, I);
+ auto *B = DILocation::get(Context, 3, 8, SP2, I);
auto *M = DILocation::getMergedLocation(A, B);
- EXPECT_EQ(0u, M->getLine());
+ EXPECT_EQ(2u, M->getLine());
+ EXPECT_EQ(7u, M->getColumn());
+ EXPECT_EQ(N, M->getScope());
+ EXPECT_EQ(nullptr, M->getInlinedAt());
+ }
+
+ {
+ // Different function, inlined-at same line, but
diff erent column.
+ auto *F = getFile();
+ auto *SP1 = DISubprogram::getDistinct(Context, F, "a", "a", F, 0, nullptr,
+ 0, nullptr, 0, 0, DINode::FlagZero,
+ DISubprogram::SPFlagZero, nullptr);
+ auto *SP2 = DISubprogram::getDistinct(Context, F, "b", "b", F, 0, nullptr,
+ 0, nullptr, 0, 0, DINode::FlagZero,
+ DISubprogram::SPFlagZero, nullptr);
+
+ auto *IA = DILocation::get(Context, 2, 7, N);
+ auto *IB = DILocation::get(Context, 2, 8, N);
+ auto *A = DILocation::get(Context, 1, 6, SP1, IA);
+ auto *B = DILocation::get(Context, 3, 8, SP2, IB);
+ auto *M = DILocation::getMergedLocation(A, B);
+ EXPECT_EQ(2u, M->getLine());
EXPECT_EQ(0u, M->getColumn());
- EXPECT_TRUE(isa<DILocalScope>(M->getScope()));
- EXPECT_EQ(I, M->getInlinedAt());
+ EXPECT_EQ(N, M->getScope());
+ EXPECT_EQ(nullptr, M->getInlinedAt());
}
- {
+ {
// Completely
diff erent.
auto *I = DILocation::get(Context, 2, 7, N);
auto *A = DILocation::get(Context, 1, 6, S, I);
@@ -1018,6 +1027,67 @@ TEST_F(DILocationTest, Merge) {
EXPECT_EQ(S, M->getScope());
EXPECT_EQ(nullptr, M->getInlinedAt());
}
+
+ // Two locations, same line/column
diff erent file, inlined at the same place.
+ {
+ auto *FA = getFile();
+ auto *FB = getFile();
+ auto *FI = getFile();
+
+ auto *SPA = DISubprogram::getDistinct(Context, FA, "a", "a", FA, 0, nullptr,
+ 0, nullptr, 0, 0, DINode::FlagZero,
+ DISubprogram::SPFlagZero, nullptr);
+
+ auto *SPB = DISubprogram::getDistinct(Context, FB, "b", "b", FB, 0, nullptr,
+ 0, nullptr, 0, 0, DINode::FlagZero,
+ DISubprogram::SPFlagZero, nullptr);
+
+ auto *SPI = DISubprogram::getDistinct(Context, FI, "i", "i", FI, 0, nullptr,
+ 0, nullptr, 0, 0, DINode::FlagZero,
+ DISubprogram::SPFlagZero, nullptr);
+
+ auto *I = DILocation::get(Context, 3, 8, SPI);
+ auto *A = DILocation::get(Context, 2, 7, SPA, I);
+ auto *B = DILocation::get(Context, 2, 7, SPB, I);
+ auto *M = DILocation::getMergedLocation(A, B);
+ EXPECT_EQ(3u, M->getLine());
+ EXPECT_EQ(8u, M->getColumn());
+ EXPECT_TRUE(isa<DILocalScope>(M->getScope()));
+ EXPECT_EQ(SPI, M->getScope());
+ EXPECT_EQ(nullptr, M->getInlinedAt());
+ }
+
+ // Two locations, same line/column
diff erent file, one location with 2 scopes,
+ // inlined at the same place.
+ {
+ auto *FA = getFile();
+ auto *FB = getFile();
+ auto *FI = getFile();
+
+ auto *SPA = DISubprogram::getDistinct(Context, FA, "a", "a", FA, 0, nullptr,
+ 0, nullptr, 0, 0, DINode::FlagZero,
+ DISubprogram::SPFlagZero, nullptr);
+
+ auto *SPB = DISubprogram::getDistinct(Context, FB, "b", "b", FB, 0, nullptr,
+ 0, nullptr, 0, 0, DINode::FlagZero,
+ DISubprogram::SPFlagZero, nullptr);
+
+ auto *SPI = DISubprogram::getDistinct(Context, FI, "i", "i", FI, 0, nullptr,
+ 0, nullptr, 0, 0, DINode::FlagZero,
+ DISubprogram::SPFlagZero, nullptr);
+
+ auto *SPAScope = DILexicalBlock::getDistinct(Context, SPA, FA, 4, 9);
+
+ auto *I = DILocation::get(Context, 3, 8, SPI);
+ auto *A = DILocation::get(Context, 2, 7, SPAScope, I);
+ auto *B = DILocation::get(Context, 2, 7, SPB, I);
+ auto *M = DILocation::getMergedLocation(A, B);
+ EXPECT_EQ(3u, M->getLine());
+ EXPECT_EQ(8u, M->getColumn());
+ EXPECT_TRUE(isa<DILocalScope>(M->getScope()));
+ EXPECT_EQ(SPI, M->getScope());
+ EXPECT_EQ(nullptr, M->getInlinedAt());
+ }
}
TEST_F(DILocationTest, getDistinct) {
More information about the llvm-commits
mailing list