From 854b1bca60ebb784e507e905885f4ba664bb65af Mon Sep 17 00:00:00 2001 From: =?utf8?q?Juan=20Manuel=20MARTINEZ=20CAAMA=C3=91O?= Date: Tue, 25 Oct 2022 07:29:30 +0000 Subject: [PATCH] [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 --- llvm/include/llvm/IR/DebugInfoMetadata.h | 22 ++--- llvm/lib/IR/DebugInfoMetadata.cpp | 56 ++++++++--- llvm/test/DebugInfo/return-same-line-merge.ll | 39 ++++++++ llvm/unittests/IR/MetadataTest.cpp | 134 ++++++++++++++++++++------ 4 files changed, 195 insertions(+), 56 deletions(-) create mode 100644 llvm/test/DebugInfo/return-same-line-merge.ll diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h index 8fc3f54..5b20bf3 100644 --- a/llvm/include/llvm/IR/DebugInfoMetadata.h +++ b/llvm/include/llvm/IR/DebugInfoMetadata.h @@ -1710,18 +1710,18 @@ public: /// 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 differ, 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 - /// differ, we need a third location which is distinct from either. If they - /// have the same file/line but have a different discriminator we could - /// create a location with a new discriminator. If they are from different - /// 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 differ. + /// \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 b29daeb..99c445f 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, 5> Locations; + LLVMContext &C = LocA->getContext(); + SmallDenseMap, + std::pair, 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(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(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(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 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 0000000..5c9965b --- /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 9dd49d0..126ae03 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,12 +936,25 @@ 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, different 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); auto *B = DILocation::get(Context, 2, 7, 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 different 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(M->getScope())); - EXPECT_EQ(I, M->getInlinedAt()); + EXPECT_EQ(N, M->getScope()); + EXPECT_EQ(nullptr, M->getInlinedAt()); } - { + { // Completely different. 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 different 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(M->getScope())); + EXPECT_EQ(SPI, M->getScope()); + EXPECT_EQ(nullptr, M->getInlinedAt()); + } + + // Two locations, same line/column different 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(M->getScope())); + EXPECT_EQ(SPI, M->getScope()); + EXPECT_EQ(nullptr, M->getInlinedAt()); + } } TEST_F(DILocationTest, getDistinct) { -- 2.7.4