diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h --- a/llvm/include/llvm/IR/DebugInfoMetadata.h +++ b/llvm/include/llvm/IR/DebugInfoMetadata.h @@ -1710,18 +1710,18 @@ /// 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 --- a/llvm/lib/IR/DebugInfoMetadata.cpp +++ b/llvm/lib/IR/DebugInfoMetadata.cpp @@ -112,34 +112,64 @@ 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 --- /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 --- a/llvm/unittests/IR/MetadataTest.cpp +++ b/llvm/unittests/IR/MetadataTest.cpp @@ -917,31 +917,6 @@ 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 @@ 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); @@ -998,15 +986,36 @@ 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 @@ 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) {