[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

diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index 8fc3f54bf2813940c76bad06c178ae6716d47691..5b20bf3ade99a1b38e001b7bbcb0228b59053be4 100644 (file)
--- 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 b29daeb653f0b8b836bfeb18030a420ca8b382d4..99c445fc2842a3be10cca630105b0346bccf9cdc 100644 (file)
--- 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 (file)
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 9dd49d04405f5a1a84d88a471e10a0aed85c202b..126ae03ff987727a501df745cd03175e9e9cb211 100644 (file)
--- 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, 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 @@ 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<DILocalScope>(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<DILocalScope>(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<DILocalScope>(M->getScope()));
+    EXPECT_EQ(SPI, M->getScope());
+    EXPECT_EQ(nullptr, M->getInlinedAt());
+  }
 }
 
 TEST_F(DILocationTest, getDistinct) {