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

[David Blaikie via llvm-commits]

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> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170214/df4e4975/attachment.html>