[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

Change-Id: Ia3be858f1587722d8cb9ab7046f869ea2063424f

llvm/include/llvm/IR/DebugInfoMetadata.h

@@ -1737,18 +1737,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 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);
 

llvm/lib/IR/DebugInfoMetadata.cpp

@@ -108,34 +108,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,

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)

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) {