[PATCH] D27462: For functions with debug info, do not propagate the callsite debug location to inlined instructions.

Andrea Di Biagio via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 6 11:56:23 PST 2016


andreadb added a comment.

Hi David,



================
Comment at: lib/Transforms/Utils/InlineFunction.cpp:1377-1378
       if (!DL) {
+        if (CalleeHasDebugInfo)
+          continue;
         // If the inlined instruction has no line number, make it look as if it
----------------
probinson wrote:
> dblaikie wrote:
> > Leaving this instruction without a location at all may be problematic - it's still certainly inlined from this other function... - what can we do about that, if anything? (only thing that comes to mind is giving it a zero debug location, but that doesn't sound right)
> > 
> > Maybe no debug location is the right solution... - I wonder how that interacts with the inline range for the instruction, etc.
> If it had no location in the called function, seems like it's not inherently different for it to have no location when moved into the calling function.  An instruction with no location implicitly inherits the preceding location, no? That's no different in the caller than the callee.
About the interaction with inline ranges:
as an experiment, I built test `inline-debug-loc.ll` with/without my patch.
Here are my findings:

Before my patch:

Contents of the .debug_info section:

```
<1><26>: Abbrev Number: 2 (DW_TAG_subprogram)
    <27>   DW_AT_name        : (indirect string, offset: 0x8): bar
 <1><2b>: Abbrev Number: 3 (DW_TAG_subprogram)
    <2c>   DW_AT_low_pc      : 0x10
    <34>   DW_AT_high_pc     : 0xd
    <38>   DW_AT_name        : (indirect string, offset: 0xc): baz
 <2><3c>: Abbrev Number: 4 (DW_TAG_inlined_subroutine)
    <3d>   DW_AT_abstract_origin: <0x26>
    <41>   DW_AT_ranges      : 0x0
    <45>   DW_AT_call_file   : 1
    <46>   DW_AT_call_line   : 12
```

Where ranges are:
```
    Offset   Begin    End
    00000000 0000000000000010 0000000000000015
    00000000 0000000000000017 000000000000001a
    00000000 <End of list>
```

Assembly in the .text:

```
0000000000000010 <baz>:
  10:   89 f8                   mov    %edi,%eax
  12:   83 c0 01                add    $0x1,%eax
  15:   39 f0                   cmp    %esi,%eax
  17:   0f 4c f7                cmovl  %edi,%esi
  1a:   89 f0                   mov    %esi,%eax
  1c:   c3                      retq
```

So, we have two ranges because instruction `cmp    %esi,%eax` breaks the sequence. That is because it obtained its location from the callsite.

With my patch:

```
<2><3c>: Abbrev Number: 4 (DW_TAG_inlined_subroutine)
    <3d>   DW_AT_abstract_origin: <0x26>
    <41>   DW_AT_low_pc      : 0x10
    <49>   DW_AT_high_pc     : 0xa
    <4d>   DW_AT_call_file   : 1
    <4e>   DW_AT_call_line   : 12
```

I think this is more correct. Now we have a single uniform range `[0x10, 0x1a)`. The assembly is obviously still the same (with same offsets).

My opinion is that no debug location is probably better. As Paul mentioned in his last comment, an instruction would implicitly inherit the preceding location (whathever that is..). This is also demonstrated by these findings.



https://reviews.llvm.org/D27462





More information about the llvm-commits mailing list