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

[Taewook Oh via llvm-commits]

I think I was unclear. What I meant was that it would be better if the merged and hoisted call instruction has a scope of “@test, line: 0, column: 0”, not “@bar, line: 0, column: 0”.

Thanks,
Taewook

From: David Blaikie <dblaikie at gmail.com>
Date: Tuesday, February 14, 2017 at 10:49 AM
To: "reviews+D29833+public+d0b46dd83e56e03f at reviews.llvm.org" <reviews+D29833+public+d0b46dd83e56e03f at reviews.llvm.org>, "aprantl at apple.com" <aprantl at apple.com>, "paul.robinson at sony.com" <paul.robinson at sony.com>, "danielcdh at gmail.com" <danielcdh at gmail.com>, "echristo at gmail.com" <echristo at gmail.com>, "rnk at google.com" <rnk at google.com>, "rob.lougher at gmail.com" <rob.lougher at gmail.com>
Cc: Taewook Oh <twoh at fb.com>, "andrea.dibiagio at gmail.com" <andrea.dibiagio at gmail.com>, "llvm-commits at lists.llvm.org" <llvm-commits at lists.llvm.org>, Mehdi AMINI <mehdi.amini at apple.com>
Subject: Re: [PATCH] D29833: Improve the API of DILocation::getMergedLocation()

Ah, sure - we can't/shouldn't remove any inline scope information (if it's unambiguous - if we hoisted from two different function calls we should probably drop the inlining scope information in the same way that we drop the ambiguous locations, unfortunately), but block scopes may or may not be helpful to preserve. I'm not sure what the right answer is there.

On Tue, Feb 14, 2017 at 10:44 AM Taewook Oh via Phabricator <reviews at reviews.llvm.org<mailto:reviews at reviews.llvm.org>> wrote:
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<https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D29833&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=kOsLCgQzH7N8ptZ7diJD9g&m=9Leqrn73ABLnKnbnINVXAvQ8dotwlC-Yd0CdOm037Mo&s=_IcWlv6Hg_yo5fDdIiU0jfSUExVmbDNqlChJLK--7Y4&e=>


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170214/aa00df66/attachment.html>