[PATCH] D29833: Improve the API of DILocation::getMergedLocation()

[Taewook Oh via Phabricator via llvm-commits]

twoh added inline comments.


================
Comment at: lib/IR/Instruction.cpp:664-668
+  if (Loc && isa<CallInst>(this)) {
+    DbgLoc = DILocation::get(Loc->getContext(), 0, 0, Loc->getScope(),
+                             Loc->getInlinedAt());
+    return;
+  }
----------------
dblaikie wrote:
> I'm not sure we can get the scope right, though - since we're potentially moving this location across scopes. It's going to be jumpy/create a difficult range in the scope no matter what, really... 
> 
> Any ideas? Take the scope from a nearby location at the destination, if possible? (doesn't really reflect reality, but nothing will - this would at least mean a chance of not punching holes in/making islands in scopes that would necessitate a DW_AT_ranges/more verbose description/etc)
It would be hard to get the precise scope anyway, but I think it might be worth to consider the inlining context. Please take a look at the following test case:

```
define i32 @bar() !dbg !6 {
entry:
  %call = tail call i32 @foo(), !dbg !8
  ret i32 %call, !dbg !9
}

define i32 @test(i32 %a, i32 %b) !dbg !10 {
entry:
  %tobool = icmp ne i32 %a, 0, !dbg !11
  br i1 %tobool, label %if.then, label %if.else, !dbg !11

if.then:                                          ; preds = %entry
  %call.i = call i32 @foo(), !dbg !12
  %sub = sub nsw i32 %b, %call.i, !dbg !14
  br label %if.end, !dbg !15

if.else:                                          ; preds = %entry
  %call1 = call i32 @foo(), !dbg !16
  %sub2 = sub nsw i32 %b, %call1, !dbg !17
  br label %if.end

if.end:                                           ; preds = %if.else, %if.then
  %b.addr.0 = phi i32 [ %sub, %if.then ], [ %sub2, %if.else ]
  ret i32 %b.addr.0, !dbg !18
}
```

where

```
!6 = distinct !DISubprogram(name: "bar", ...)
!10 = distinct !DISubprogram(name: "test", ...)
!12 = !DILocation(line: 4, column: 10, scope: !6, inlinedAt: !13)
!16 = !DILocation(line: 11, column: 10, scope: !10)
```

Here the call instruction in %if.then block is actually inlined from @bar, and if we pickup the scope from that instruction, the final code would be look something like below:

```
define i32 @test(i32 %a, i32 %b) !dbg !10 {
entry:
  %call.i = call i32 @foo(), !dbg !11
  %sub = sub nsw i32 %b, %call.i, !dbg !13
  ret i32 %sub, !dbg !14
}

!6 = distinct !DISubprogram(name: "bar", ...)
!11 = !DILocation(line: 0, column: 0, scope: !6, inlinedAt: !12)
```

As the call instructions are merged and hoisted inside test not bar, it would be awkward if the merged call instruction has a scope of bar, though the debug location is still just (line: 0, column: 0).

I missed the discussion in llvm-dev and implemented a patch for the same issue considering inline scope (D29927). Maybe worth take a look. 


Repository:
  rL LLVM

https://reviews.llvm.org/D29833