[PATCH] D96045: [llvm-dwarfdump][locstats] Unify handling of inlined vars with no loc

Wed Apr 14 00:33:27 PDT 2021

dblaikie added a comment.

In D96045#2685000 <https://reviews.llvm.org/D96045#2685000>, @djtodoro wrote:

> In D96045#2684570 <https://reviews.llvm.org/D96045#2684570>, @dblaikie wrote:
>
>> In D96045#2678117 <https://reviews.llvm.org/D96045#2678117>, @rdhindsa wrote:
>>
>>> After moving the variable InlinedFnsToBeProcessed to inside the for loop, I noticed a difference in two dwarfdump statistics for a quite large binary with fission=yes:
>>>
>>> #variables processed by location statistics
>>> #variables with 0% of parent scope covered by DW_AT_location
>>>
>>> Since I am not very familiar with this code, I would request if you could investigate and apply the patch.
>>
>> Hmm, looking at this trying to reproduce the behavior makes me think this wouldn't be the right fix & the change in numbers is probably reflective of a bug in the fix - specifically in LTO situations (where the abstract origin of the function may be in a different CU than the one it's referenced from/inlined into).
>>
>> But there might be a reasonable alternative fix/improvement - pruning the contents of the list once it has been handled. Possibly sorting the list so it's easy to just visit the elements that need to.
>
> I have taken a look into this and come up with the similar conclusion.
> This could be reproduced with the Split DWARF -- when building e.g. GDB project with -gsplit-dwarf option enabled, it triggers the issue. I was struggling to make a simple test case (from a high level) that meets all the requirements, but it is very hard. That is why I've sent the email: https://lists.llvm.org/pipermail/llvm-dev/2021-April/149735.html.

Ah, hmm - why would this come up in Split DWARF, do you think? Split DWARF doesn't have any cross-CU references (the format has no way to encode them) so I'd expect no DIEs to be added to the InlinedFnsToBeProcessed list?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96045/new/

https://reviews.llvm.org/D96045