Skip to content

Commit 854b1bc

Browse files
jmmartinezjmmartinez
committed
[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
1 parent 191d70f commit 854b1bc

File tree

4 files changed

+195
-56
lines changed

4 files changed

+195
-56
lines changed

‎llvm/include/llvm/IR/DebugInfoMetadata.h‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1710,18 +1710,18 @@ class DILocation : public MDNode {
17101710

17111711
/// When two instructions are combined into a single instruction we also
17121712
/// need to combine the original locations into a single location.
1713+
/// When the locations are the same we can use either location.
1714+
/// When they differ, we need a third location which is distinct from either.
1715+
/// If they share a common scope, use this scope and compare the line/column
1716+
/// pair of the locations with the common scope:
1717+
/// * if both match, keep the line and column;
1718+
/// * if only the line number matches, keep the line and set the column as 0;
1719+
/// * otherwise set line and column as 0.
1720+
/// If they do not share a common scope the location is ambiguous and can't be
1721+
/// represented in a line entry. In this case, set line and column as 0 and
1722+
/// use the scope of any location.
17131723
///
1714-
/// When the locations are the same we can use either location. When they
1715-
/// differ, we need a third location which is distinct from either. If they
1716-
/// have the same file/line but have a different discriminator we could
1717-
/// create a location with a new discriminator. If they are from different
1718-
/// files/lines the location is ambiguous and can't be represented in a line
1719-
/// entry. In this case, if \p GenerateLocation is true, we will set the
1720-
/// merged debug location as line 0 of the nearest common scope where the two
1721-
/// locations are inlined from.
1722-
///
1723-
/// \p GenerateLocation: Whether the merged location can be generated when
1724-
/// \p LocA and \p LocB differ.
1724+
/// \p LocA \p LocB: The locations to be merged.
17251725
static const DILocation *getMergedLocation(const DILocation *LocA,
17261726
const DILocation *LocB);
17271727

‎llvm/lib/IR/DebugInfoMetadata.cpp‎

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -112,34 +112,64 @@ const DILocation *DILocation::getMergedLocation(const DILocation *LocA,
112112
if (LocA == LocB)
113113
return LocA;
114114

115-
SmallSet<std::pair<DIScope *, DILocation *>, 5> Locations;
115+
LLVMContext &C = LocA->getContext();
116+
SmallDenseMap<std::pair<DILocalScope *, DILocation *>,
117+
std::pair<unsigned, unsigned>, 4>
118+
Locations;
119+
116120
DIScope *S = LocA->getScope();
117121
DILocation *L = LocA->getInlinedAt();
118-
while (S) {
119-
Locations.insert(std::make_pair(S, L));
122+
unsigned Line = LocA->getLine();
123+
unsigned Col = LocA->getColumn();
124+
125+
// Walk from the current source locaiton until the file scope;
126+
// then, do the same for the inlined-at locations.
127+
auto AdvanceToParentLoc = [&S, &L, &Line, &Col]() {
120128
S = S->getScope();
121129
if (!S && L) {
130+
Line = L->getLine();
131+
Col = L->getColumn();
122132
S = L->getScope();
123133
L = L->getInlinedAt();
124134
}
135+
};
136+
137+
while (S) {
138+
if (auto *LS = dyn_cast<DILocalScope>(S))
139+
Locations.try_emplace(std::make_pair(LS, L), std::make_pair(Line, Col));
140+
AdvanceToParentLoc();
125141
}
142+
143+
// Walk the source locations of LocB until a match with LocA is found.
126144
S = LocB->getScope();
127145
L = LocB->getInlinedAt();
146+
Line = LocB->getLine();
147+
Col = LocB->getColumn();
128148
while (S) {
129-
if (Locations.count(std::make_pair(S, L)))
130-
break;
131-
S = S->getScope();
132-
if (!S && L) {
133-
S = L->getScope();
134-
L = L->getInlinedAt();
149+
if (auto *LS = dyn_cast<DILocalScope>(S)) {
150+
auto MatchLoc = Locations.find(std::make_pair(LS, L));
151+
if (MatchLoc != Locations.end()) {
152+
// If the lines match, keep the line, but set the column to '0'
153+
// If the lines don't match, pick a "line 0" location but keep
154+
// the current scope and inlined-at.
155+
bool SameLine = Line == MatchLoc->second.first;
156+
bool SameCol = Col == MatchLoc->second.second;
157+
Line = SameLine ? Line : 0;
158+
Col = SameLine && SameCol ? Col : 0;
159+
break;
160+
}
135161
}
162+
AdvanceToParentLoc();
136163
}
137164

138-
// If the two locations are irreconsilable, just pick one. This is misleading,
139-
// but on the other hand, it's a "line 0" location.
140-
if (!S || !isa<DILocalScope>(S))
165+
if (!S) {
166+
// If the two locations are irreconsilable, pick any scope,
167+
// and return a "line 0" location.
168+
Line = Col = 0;
141169
S = LocA->getScope();
142-
return DILocation::get(LocA->getContext(), 0, 0, S, L);
170+
}
171+
172+
return DILocation::get(C, Line, Col, S, L);
143173
}
144174

145175
Optional<unsigned> DILocation::encodeDiscriminator(unsigned BD, unsigned DF,

‎llvm/test/DebugInfo/return-same-line-merge.ll‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
; RUN: opt -simplifycfg -S < %s | FileCheck %s
2+
;
3+
; Simplified from the following code:
4+
; int foo() {
5+
; if(c) { return a; } else { return b; }
6+
; }
7+
define i32 @foo(i32 %c, i32 %a, i32 %b) !dbg !4 {
8+
; CHECK: define i32 @foo({{.*}}) !dbg [[FOO_SUBPROGRAM:![0-9]+]]
9+
; CHECK-NEXT: entry:
10+
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp ne i32 [[C:%.*]], 0, !dbg [[DBG_CMP:![0-9]+]]
11+
; CHECK-NEXT: [[A_B:%.*]] = select i1 [[TOBOOL]], i32 [[A:%.*]], i32 [[B:%.*]]
12+
; CHECK-NEXT: ret i32 [[A_B]], !dbg [[DBG_RET:![0-9]+]]
13+
; CHECK: [[DBG_CMP]] = !DILocation(line: 2, column: 1, scope: [[FOO_SUBPROGRAM]])
14+
; CHECK: [[DBG_RET]] = !DILocation(line: 4, scope: [[FOO_SUBPROGRAM]])
15+
entry:
16+
%tobool = icmp ne i32 %c, 0, !dbg !7
17+
br i1 %tobool, label %cond.true, label %cond.false, !dbg !8
18+
19+
cond.true: ; preds = %entry
20+
ret i32 %a, !dbg !9
21+
22+
cond.false: ; preds = %entry
23+
ret i32 %b, !dbg !10
24+
}
25+
26+
!llvm.dbg.cu = !{!0}
27+
!llvm.module.flags = !{!2, !3}
28+
29+
!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)
30+
!1 = !DIFile(filename: "test.c", directory: "/")
31+
!2 = !{i32 7, !"Dwarf Version", i32 5}
32+
!3 = !{i32 2, !"Debug Info Version", i32 3}
33+
!4 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 1, type: !5, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !6)
34+
!5 = !DISubroutineType(types: !6)
35+
!6 = !{}
36+
!7 = !DILocation(line: 2, column: 1, scope: !4)
37+
!8 = !DILocation(line: 3, column: 1, scope: !4)
38+
!9 = !DILocation(line: 4, column: 1, scope: !4)
39+
!10 = !DILocation(line: 4, column: 2, scope: !4)

‎llvm/unittests/IR/MetadataTest.cpp‎

Lines changed: 102 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -917,31 +917,6 @@ TEST_F(MDNodeTest, deleteTemporaryWithTrackingRef) {
917917

918918
typedef MetadataTest DILocationTest;
919919

920-
TEST_F(DILocationTest, Overflow) {
921-
DISubprogram *N = getSubprogram();
922-
{
923-
DILocation *L = DILocation::get(Context, 2, 7, N);
924-
EXPECT_EQ(2u, L->getLine());
925-
EXPECT_EQ(7u, L->getColumn());
926-
}
927-
unsigned U16 = 1u << 16;
928-
{
929-
DILocation *L = DILocation::get(Context, UINT32_MAX, U16 - 1, N);
930-
EXPECT_EQ(UINT32_MAX, L->getLine());
931-
EXPECT_EQ(U16 - 1, L->getColumn());
932-
}
933-
{
934-
DILocation *L = DILocation::get(Context, UINT32_MAX, U16, N);
935-
EXPECT_EQ(UINT32_MAX, L->getLine());
936-
EXPECT_EQ(0u, L->getColumn());
937-
}
938-
{
939-
DILocation *L = DILocation::get(Context, UINT32_MAX, U16 + 1, N);
940-
EXPECT_EQ(UINT32_MAX, L->getLine());
941-
EXPECT_EQ(0u, L->getColumn());
942-
}
943-
}
944-
945920
TEST_F(DILocationTest, Merge) {
946921
DISubprogram *N = getSubprogram();
947922
DIScope *S = DILexicalBlock::get(Context, N, getFile(), 3, 4);
@@ -961,11 +936,24 @@ TEST_F(DILocationTest, Merge) {
961936
auto *A = DILocation::get(Context, 2, 7, N);
962937
auto *B = DILocation::get(Context, 2, 7, S);
963938
auto *M = DILocation::getMergedLocation(A, B);
964-
EXPECT_EQ(0u, M->getLine()); // FIXME: Should this be 2?
965-
EXPECT_EQ(0u, M->getColumn()); // FIXME: Should this be 7?
939+
EXPECT_EQ(2u, M->getLine());
940+
EXPECT_EQ(7u, M->getColumn());
966941
EXPECT_EQ(N, M->getScope());
967942
}
968943

944+
{
945+
// Same line, different column.
946+
auto *A = DILocation::get(Context, 2, 7, N);
947+
auto *B = DILocation::get(Context, 2, 10, S);
948+
auto *M0 = DILocation::getMergedLocation(A, B);
949+
auto *M1 = DILocation::getMergedLocation(B, A);
950+
for (auto *M : {M0, M1}) {
951+
EXPECT_EQ(2u, M->getLine());
952+
EXPECT_EQ(0u, M->getColumn());
953+
EXPECT_EQ(N, M->getScope());
954+
}
955+
}
956+
969957
{
970958
// Different lines, same scopes.
971959
auto *A = DILocation::get(Context, 1, 6, N);
@@ -998,15 +986,36 @@ TEST_F(DILocationTest, Merge) {
998986

999987
auto *I = DILocation::get(Context, 2, 7, N);
1000988
auto *A = DILocation::get(Context, 1, 6, SP1, I);
1001-
auto *B = DILocation::get(Context, 2, 7, SP2, I);
989+
auto *B = DILocation::get(Context, 3, 8, SP2, I);
1002990
auto *M = DILocation::getMergedLocation(A, B);
1003-
EXPECT_EQ(0u, M->getLine());
991+
EXPECT_EQ(2u, M->getLine());
992+
EXPECT_EQ(7u, M->getColumn());
993+
EXPECT_EQ(N, M->getScope());
994+
EXPECT_EQ(nullptr, M->getInlinedAt());
995+
}
996+
997+
{
998+
// Different function, inlined-at same line, but different column.
999+
auto *F = getFile();
1000+
auto *SP1 = DISubprogram::getDistinct(Context, F, "a", "a", F, 0, nullptr,
1001+
0, nullptr, 0, 0, DINode::FlagZero,
1002+
DISubprogram::SPFlagZero, nullptr);
1003+
auto *SP2 = DISubprogram::getDistinct(Context, F, "b", "b", F, 0, nullptr,
1004+
0, nullptr, 0, 0, DINode::FlagZero,
1005+
DISubprogram::SPFlagZero, nullptr);
1006+
1007+
auto *IA = DILocation::get(Context, 2, 7, N);
1008+
auto *IB = DILocation::get(Context, 2, 8, N);
1009+
auto *A = DILocation::get(Context, 1, 6, SP1, IA);
1010+
auto *B = DILocation::get(Context, 3, 8, SP2, IB);
1011+
auto *M = DILocation::getMergedLocation(A, B);
1012+
EXPECT_EQ(2u, M->getLine());
10041013
EXPECT_EQ(0u, M->getColumn());
1005-
EXPECT_TRUE(isa<DILocalScope>(M->getScope()));
1006-
EXPECT_EQ(I, M->getInlinedAt());
1014+
EXPECT_EQ(N, M->getScope());
1015+
EXPECT_EQ(nullptr, M->getInlinedAt());
10071016
}
10081017

1009-
{
1018+
{
10101019
// Completely different.
10111020
auto *I = DILocation::get(Context, 2, 7, N);
10121021
auto *A = DILocation::get(Context, 1, 6, S, I);
@@ -1018,6 +1027,67 @@ TEST_F(DILocationTest, Merge) {
10181027
EXPECT_EQ(S, M->getScope());
10191028
EXPECT_EQ(nullptr, M->getInlinedAt());
10201029
}
1030+
1031+
// Two locations, same line/column different file, inlined at the same place.
1032+
{
1033+
auto *FA = getFile();
1034+
auto *FB = getFile();
1035+
auto *FI = getFile();
1036+
1037+
auto *SPA = DISubprogram::getDistinct(Context, FA, "a", "a", FA, 0, nullptr,
1038+
0, nullptr, 0, 0, DINode::FlagZero,
1039+
DISubprogram::SPFlagZero, nullptr);
1040+
1041+
auto *SPB = DISubprogram::getDistinct(Context, FB, "b", "b", FB, 0, nullptr,
1042+
0, nullptr, 0, 0, DINode::FlagZero,
1043+
DISubprogram::SPFlagZero, nullptr);
1044+
1045+
auto *SPI = DISubprogram::getDistinct(Context, FI, "i", "i", FI, 0, nullptr,
1046+
0, nullptr, 0, 0, DINode::FlagZero,
1047+
DISubprogram::SPFlagZero, nullptr);
1048+
1049+
auto *I = DILocation::get(Context, 3, 8, SPI);
1050+
auto *A = DILocation::get(Context, 2, 7, SPA, I);
1051+
auto *B = DILocation::get(Context, 2, 7, SPB, I);
1052+
auto *M = DILocation::getMergedLocation(A, B);
1053+
EXPECT_EQ(3u, M->getLine());
1054+
EXPECT_EQ(8u, M->getColumn());
1055+
EXPECT_TRUE(isa<DILocalScope>(M->getScope()));
1056+
EXPECT_EQ(SPI, M->getScope());
1057+
EXPECT_EQ(nullptr, M->getInlinedAt());
1058+
}
1059+
1060+
// Two locations, same line/column different file, one location with 2 scopes,
1061+
// inlined at the same place.
1062+
{
1063+
auto *FA = getFile();
1064+
auto *FB = getFile();
1065+
auto *FI = getFile();
1066+
1067+
auto *SPA = DISubprogram::getDistinct(Context, FA, "a", "a", FA, 0, nullptr,
1068+
0, nullptr, 0, 0, DINode::FlagZero,
1069+
DISubprogram::SPFlagZero, nullptr);
1070+
1071+
auto *SPB = DISubprogram::getDistinct(Context, FB, "b", "b", FB, 0, nullptr,
1072+
0, nullptr, 0, 0, DINode::FlagZero,
1073+
DISubprogram::SPFlagZero, nullptr);
1074+
1075+
auto *SPI = DISubprogram::getDistinct(Context, FI, "i", "i", FI, 0, nullptr,
1076+
0, nullptr, 0, 0, DINode::FlagZero,
1077+
DISubprogram::SPFlagZero, nullptr);
1078+
1079+
auto *SPAScope = DILexicalBlock::getDistinct(Context, SPA, FA, 4, 9);
1080+
1081+
auto *I = DILocation::get(Context, 3, 8, SPI);
1082+
auto *A = DILocation::get(Context, 2, 7, SPAScope, I);
1083+
auto *B = DILocation::get(Context, 2, 7, SPB, I);
1084+
auto *M = DILocation::getMergedLocation(A, B);
1085+
EXPECT_EQ(3u, M->getLine());
1086+
EXPECT_EQ(8u, M->getColumn());
1087+
EXPECT_TRUE(isa<DILocalScope>(M->getScope()));
1088+
EXPECT_EQ(SPI, M->getScope());
1089+
EXPECT_EQ(nullptr, M->getInlinedAt());
1090+
}
10211091
}
10221092

10231093
TEST_F(DILocationTest, getDistinct) {

0 commit comments

Comments
 (0)