[DebugInfo] Make DILocation::getMergedLocation to return a dummy location instead of nullptr

[twoh]

Summary

Currently, DILocation::getMergedLocation returns nullptr if LocA and LocB come from different locations so that they can be discriminated. However, dropping metadata by returning nullptr may cause a problem when handling inlinable callsites, as discussed in PR31891. Since inlinable calls are required to have a scope information, if two inlinable calls are hoisted and merged, and the merged instruction does not have a debug location info because debug locations of original instructions are different, module verifier fails. For this reason r294250 has been reverted.

This patch makes getMergedLocation returning nullptr only when either LocA or LocB is a nullptr. Otherwise, if LocA and LocB can be discriminated, the function returns a dummy location of (line: 0, column: 0). It asserts that LocA and LocB are in the same scope considering inlining context, and use that scope for the dummy location info to return. This assertion might be unnecessary considering that the return value is a dummy location anyway, but I think performing the merge between two DILocation in different scope should not be permitted.

[andreadb]

FYI. An alternative fix for this same issue has been already uploaded by Adrian as review D29833.

[twoh]

Oops, I missed it. Thanks!

include/llvm/IR/DebugInfoMetadata.h

public:
   /// 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 single line entry.  In this case, no location
   /// should be set.
   ///
-  /// Currently the function does not create a new location. If the locations
-  /// are the same, or cannot be discriminated, the first location is returned.
-  /// Otherwise an empty location will be used.
+  /// Currently the function does not create a new 'valid' location. If the
+  /// locations are the same, or cannot be discriminated, the first location is
+  /// returned. Otherwise a dummy location of (line: 0, column: 0) is returned
+  /// with a scope of LocA, considering inlining context.
   static const DILocation *getMergedLocation(const DILocation *LocA,
                                              const DILocation *LocB) {
-    if (LocA && LocB && (LocA == LocB || !LocA->canDiscriminate(*LocB)))
-      return LocA;
+    if (!LocA || !LocB)
       return nullptr;
+
+    if (LocA == LocB || !LocA->canDiscriminate(*LocB))
+      return LocA;
+
+    // If the locations can be discriminated, returns a dummy location.
+    while (auto *InlinedLoc = LocA->getInlinedAt())
+      LocA = InlinedLoc;
+    while (auto *InlinedLoc = LocB->getInlinedAt())
+      LocB = InlinedLoc;
+    assert(LocA->getScope()->getSubprogram() ==
+               LocB->getScope()->getSubprogram() &&
+           "Cannot merge debug locations in different subprograms.");
+    assert(&LocA->getContext() == &LocB->getContext() &&
+           "Cannot merge debug locations in different contexts.");
+
+    return DILocation::get(LocA->getContext(), 0, 0, LocA->getScope());
   }
 
   Metadata *getRawScope() const { return getOperand(0); }
   Metadata *getRawInlinedAt() const {
     if (getNumOperands() == 2)
       return getOperand(1);
     return nullptr;
   }

test/DebugInfo/Generic/instcombine-phi.ll

 ;     b -= bar();
 ;   return b;
 ; }
 
 ; CHECK: define i32 @binop
 ; CHECK-LABEL: if.end:
 ; CHECK: %[[PHI:.*]] = phi i32 [ %call, %if.then ], [ %call1, %if.else ]
 ; CHECK: sub nsw i32 %b, %[[PHI]]
-; CHECK-NOT: !dbg
+; CHECK: !dbg [[DBG1:![0-9]+]]
 ; CHECK: ret i32
 
 define i32 @binop(i32 %a, i32 %b) !dbg !6 {
 entry:
   %tobool = icmp ne i32 %a, 0, !dbg !8
   br i1 %tobool, label %if.then, label %if.else, !dbg !8
 
 if.then:                                          ; preds = %entry

 ;     r = bar() < b;
 ;   return r;
 ; }
 
 ; CHECK: define i32 @cmp
 ; CHECK-LABEL: if.end:
 ; CHECK: %[[PHI:.*]] = phi i32 [ %call, %if.then ], [ %call1, %if.else ]
 ; CHECK: icmp slt i32 %[[PHI]], %b
-; CHECK-NOT: !dbg
+; CHECK: !dbg [[DBG2:![0-9]+]]
 ; CHECK: ret i32
 
 define i32 @cmp(i32 %a, i32 %b) !dbg !15 {
 entry:
   %tobool = icmp ne i32 %a, 0, !dbg !16
   br i1 %tobool, label %if.then, label %if.else, !dbg !16
 
 if.then:                                          ; preds = %entry

 ;     r = &b[bar2()];
 ;   return p;
 ; }
 
 ; CHECK: define i32* @gep
 ; CHECK-LABEL: if.end:
 ; CHECK: %[[PHI:.*]] = phi i64 [ %call, %if.then ], [ %call1, %if.else ]
 ; CHECK: getelementptr inbounds i32, i32* %b, i64 %[[PHI]]
-; CHECK-NOT: !dbg
+; CHECK: !dbg [[DBG3:![0-9]+]]
 ; CHECK: ret i32*
 
 define i32* @gep(i32 %a, i32* %b) !dbg !23 {
 entry:
   %tobool = icmp ne i32 %a, 0, !dbg !24
   br i1 %tobool, label %if.then, label %if.else, !dbg !24
 
 if.then:                                          ; preds = %entry

 ;     r = *bar3();
 ;   return r;
 ; }
 
 ; CHECK: define i32 @load
 ; CHECK-LABEL: if.end:
 ; CHECK: %[[PHI:.*]] = phi i32* [ %call, %if.then ], [ %call1, %if.else ]
 ; CHECK: load i32, i32* %[[PHI]]
-; CHECK-NOT: !dbg
+; CHECK: !dbg [[DBG4:![0-9]+]]
 ; CHECK: ret i32
 
 define i32 @load(i32 %a) !dbg !31 {
 entry:
   %tobool = icmp ne i32 %a, 0, !dbg !32
   br i1 %tobool, label %if.then, label %if.else, !dbg !32
 
 if.then:                                          ; preds = %entry

 ;     r = bar();
 ;   return r;
 ; }
 
 ; CHECK: define i64 @cast
 ; CHECK-LABEL: if.end:
 ; CHECK: %[[PHI:.*]] = phi i32 [ %call, %if.then ], [ %call1, %if.else ]
 ; CHECK: sext i32 %[[PHI]] to i64
-; CHECK-NOT: !dbg
+; CHECK: !dbg [[DBG5:![0-9]+]]
 ; CHECK: ret i64
 
 define i64 @cast(i32 %a) !dbg !39 {
 entry:
   %tobool = icmp ne i32 %a, 0, !dbg !40
   br i1 %tobool, label %if.then, label %if.else, !dbg !40
 
 if.then:                                          ; preds = %entry

 ;     r = bar() - 5;
 ;   return r;
 ; }
 
 ; CHECK: define i32 @binop_const
 ; CHECK-LABEL: if.end:
 ; CHECK: %[[PHI:.*]] = phi i32 [ %call, %if.then ], [ %call1, %if.else ]
 ; CHECK: add nsw i32 %[[PHI]], -5
-; CHECK-NOT: !dbg
+; CHECK: !dbg [[DBG6:![0-9]+]]
 ; CHECK: ret i32
 
 define i32 @binop_const(i32 %a) !dbg !45 {
 entry:
   %tobool = icmp ne i32 %a, 0, !dbg !46
   br i1 %tobool, label %if.then, label %if.else, !dbg !46
 
 if.then:                                          ; preds = %entry

 ;     r = bar() < 10;
 ;   return r;
 ; }
 
 ; CHECK: define i32 @cmp_const
 ; CHECK-LABEL: if.end:
 ; CHECK: %[[PHI:.*]] = phi i32 [ %call, %if.then ], [ %call1, %if.else ]
 ; CHECK: icmp slt i32 %[[PHI]], 10
-; CHECK-NOT: !dbg
+; CHECK: !dbg [[DBG7:![0-9]+]]
 ; CHECK: ret i32
 
 define i32 @cmp_const(i32 %a) !dbg !53 {
 entry:
   %tobool = icmp ne i32 %a, 0, !dbg !54
   br i1 %tobool, label %if.then, label %if.else, !dbg !54
 
 if.then:                                          ; preds = %entry

 declare i64 @foo2()
 declare i64 @bar2()
 declare i32* @foo3()
 declare i32* @bar3()
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!3, !4}
 
+; CHECK: [[BINOP:![0-9]+]] = distinct !DISubprogram(name: "binop"{{.+}})
+; CHECK: [[DBG1]] = !DILocation(line: 0, scope: [[BINOP]])
+; CHECK: [[CMP:![0-9]+]] = distinct !DISubprogram(name: "cmp"{{.+}})
+; CHECK: [[DBG2]] = !DILocation(line: 0, scope: [[CMP]])
+; CHECK: [[GEP:![0-9]+]] = distinct !DISubprogram(name: "gep"{{.+}})
+; CHECK: [[DBG3]] = !DILocation(line: 0, scope: [[GEP]])
+; CHECK: [[LOAD:![0-9]+]] = distinct !DISubprogram(name: "load"{{.+}})
+; CHECK: [[DBG4]] = !DILocation(line: 0, scope: [[LOAD]])
+; CHECK: [[CAST:![0-9]+]] = distinct !DISubprogram(name: "cast"{{.+}})
+; CHECK: [[DBG5]] = !DILocation(line: 0, scope: [[CAST]])
+; CHECK: [[BINOP_CONST:![0-9]+]] = distinct !DISubprogram(name: "binop_const"{{.+}})
+; CHECK: [[DBG6]] = !DILocation(line: 0, scope: [[BINOP_CONST]])
+; CHECK: [[CMP_CONST:![0-9]+]] = distinct !DISubprogram(name: "cmp_const"{{.+}})
+; CHECK: [[DBG7]] = !DILocation(line: 0, scope: [[CMP_CONST]])
+
 !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "", isOptimized: false, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)
 !1 = !DIFile(filename: "test.c", directory: "")
 !2 = !{}
 !3 = !{i32 2, !"Dwarf Version", i32 4}
 !4 = !{i32 2, !"Debug Info Version", i32 3}
 !6 = distinct !DISubprogram(name: "binop", scope: !1, file: !1, line: 8, type: !7, isLocal: false, isDefinition: true, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2)
 !7 = !DISubroutineType(types: !2)
 !8 = !DILocation(line: 9, column: 6, scope: !6)

test/DebugInfo/Generic/simplifycfg_sink_last_inst.ll

 ;     b -= bar();
 ;   return b;
 ; }
 
 ; CHECK: define i32 @test
 ; CHECK-LABEL: if.end:
 ; CHECK: %[[PHI:.*]] = phi i32 [ %call1, %if.else ], [ %call, %if.then ]
 ; CHECK: sub nsw i32 %b, %[[PHI]]
-; CHECK-NOT: !dbg
+; CHECK: !dbg [[DBG1:![0-9]+]]
 ; CHECK: ret i32
 
 define i32 @test(i32 %a, i32 %b) !dbg !6 {
 entry:
   %tobool = icmp ne i32 %a, 0, !dbg !8
   br i1 %tobool, label %if.then, label %if.else, !dbg !8
 
 if.then:                                          ; preds = %entry

 ; int test2(int a, int b) {
 ;   if(a) b -= foo(); else b -= bar();
 ;   return b;
 ; }
 
 ; CHECK: define i32 @test2
 ; CHECK-LABEL: if.end:
 ; CHECK: %[[PHI:.*]] = phi i32 [ %call1, %if.else ], [ %call, %if.then ]
-; CHECK: sub nsw i32 %b, %[[PHI]], !dbg ![[DBG:.*]]
+; CHECK: sub nsw i32 %b, %[[PHI]], !dbg ![[DBG2:.*]]
 ; CHECK: ret i32
-; CHECK: ![[DBG]] = !DILocation(line: 17, scope: !{{.*}})
 
 define i32 @test2(i32 %a, i32 %b) !dbg !15 {
 entry:
   %tobool = icmp ne i32 %a, 0, !dbg !16
   br i1 %tobool, label %if.then, label %if.else, !dbg !16
 
 if.then:                                          ; preds = %entry
   %call = call i32 @foo(), !dbg !16

 }
 
 declare i32 @foo()
 declare i32 @bar()
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!3, !4}
 
+; CHECK: [[TEST:![0-9]+]] = distinct !DISubprogram(name: "test"{{.+}})
+; CHECK: [[DBG1]] = !DILocation(line: 0, scope: [[TEST]])
+; CHECK: ![[DBG2]] = !DILocation(line: 17, scope: !{{.*}})
+
 !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "", isOptimized: false, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)
 !1 = !DIFile(filename: "test.c", directory: "")
 !2 = !{}
 !3 = !{i32 2, !"Dwarf Version", i32 4}
 !4 = !{i32 2, !"Debug Info Version", i32 3}
 !6 = distinct !DISubprogram(name: "test", scope: !1, file: !1, line: 8, type: !7, isLocal: false, isDefinition: true, scopeLine: 8, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2)
 !7 = !DISubroutineType(types: !2)
 !8 = !DILocation(line: 9, column: 6, scope: !6)

test/DebugInfo/Generic/store-tail-merge.ll

 ;
 ;   return a;
 ; }
 ;
 ; CHECK:       define {{.*}}@foo
 ; CHECK:       if.end:
 ; CHECK-NEXT:  %storemerge = phi
 ; This final check is the "real" test, verify no !dbg on the store.
-; CHECK-NEXT:  store i32 %storemerge{{.*}}, align 4{{$}}
+; CHECK-NEXT:  store i32 %storemerge{{.*}}, align 4
+; CHECK: !dbg [[DBG:![0-9]+]]
 ;
 ; ModuleID = 'test1.ll'
 source_filename = "test.c"
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 ; Function Attrs: noinline nounwind uwtable
 define i32 @foo(i32 %a, i32* %d) !dbg !6 {

 
 declare i32 @bar(i32)
 
 declare i32 @bar2(i32)
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!3, !4}
 
+; CHECK: [[FOO:![0-9]+]] = distinct !DISubprogram(name: "foo"{{.+}})
+; CHECK: [[DBG]] = !DILocation(line: 0, scope: [[FOO]])
+
 !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "", isOptimized: false, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)
 !1 = !DIFile(filename: "test.c", directory: "/home/probinson/projects/scratch")
 !2 = !{}
 !3 = !{i32 2, !"Dwarf Version", i32 4}
 !4 = !{i32 2, !"Debug Info Version", i32 3}
 !6 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 4, type: !7, isLocal: false, isDefinition: true, scopeLine: 4, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2)
 !7 = !DISubroutineType(types: !2)
 !8 = !DILocation(line: 5, column: 6, scope: !6)
 !9 = !DILocation(line: 6, column: 12, scope: !6)
 !10 = !DILocation(line: 6, column: 10, scope: !6)
 !11 = !DILocation(line: 7, column: 3, scope: !6)
 !12 = !DILocation(line: 8, column: 12, scope: !6)
 !13 = !DILocation(line: 8, column: 10, scope: !6)
 !14 = !DILocation(line: 10, column: 3, scope: !6)

test/Transforms/SimplifyCFG/remove-debug-2.ll

 ; RUN: opt < %s -simplifycfg -S | FileCheck %s
 
 ; Check if the debug info for hoisted store for "ret = 0" is removed
 ;
 ; int foo(int x) {
 ;   int ret = 1;
 ;   if (x)
 ;     ret = 0;
 ;   return ret;
 ; }
 ;
 ; CHECK: store i32 1,{{.+}}!dbg ![[DLOC1:[0-9]+]]
 ; CHECK: icmp ne {{.+}}!dbg ![[DLOC2:[0-9]+]]
 ; CHECK: [[VREG:%[^ ]+]] = select
 ; CHECK: store i32 [[VREG]]
-; CHECK-NOT: !dbg
-; CHECK-SAME: {{$}}
-; CHECK: ret {{.+}}!dbg ![[DLOC3:[0-9]+]]
-; CHECK: ![[DLOC1]] = !DILocation(line: 2
-; CHECK: ![[DLOC2]] = !DILocation(line: 3
-; CHECK: ![[DLOC3]] = !DILocation(line: 5
+; CHECK-SAME: !dbg ![[DLOC3:[0-9]+]]
+; CHECK: ret {{.+}}!dbg ![[DLOC4:[0-9]+]]
+; CHECK-DAG: [[FOO:![0-9]+]] = distinct !DISubprogram(name: "foo"{{.+}})
+; CHECK-DAG: ![[DLOC1]] = !DILocation(line: 2
+; CHECK-DAG: ![[DLOC2]] = !DILocation(line: 3
+; CHECK-DAG: ![[DLOC3]] = !DILocation(line: 0, scope: [[FOO]])
+; CHECK-DAG: ![[DLOC4]] = !DILocation(line: 5
 
 target triple = "x86_64-unknown-linux-gnu"
 
 ; Function Attrs: noinline nounwind uwtable
 define i32 @foo(i32) !dbg !6 {
   %2 = alloca i32, align 4
   %3 = alloca i32, align 4
   store i32 %0, i32* %2, align 4