This is an archive of the discontinued LLVM Phabricator instance.

[llvm-dwarfdump] Fix split-dwarf bug in stats for inlined var loc cov
ClosedPublic

Authored by djtodoro on Apr 21 2021, 6:36 AM.

Details

Summary

Initial (D96045) patch didn't handle split dwarf cases, so this fixes that bug.

In addition, before applying this patch, we had a slowdown that happened after the D96045. With this patch, the slowdown will be fixed.

Diff Detail

Event Timeline

djtodoro created this revision.Apr 21 2021, 6:36 AM
djtodoro requested review of this revision.Apr 21 2021, 6:36 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 21 2021, 6:36 AM
aprantl accepted this revision.Apr 21 2021, 9:29 AM

One comment inline.

llvm/tools/llvm-dwarfdump/Statistics.cpp
727

This comment is mostly repeats what's already in the name of the variable. It might more interesting to explain why these are being reset for each CU.

This revision is now accepted and ready to land.Apr 21 2021, 9:29 AM

Generally good for me - but could you try to see if the test could be simplified any further (maybe "main" can be in test1 or test2 (replacing f1 or f2) and could call the other function in the other file) - and see if the auxiliary/input files could be represented as assembly instead of checked in object files?

djtodoro updated this revision to Diff 339532.Apr 22 2021, 2:33 AM
  • Reduce test
  • Improve the comment
djtodoro marked an inline comment as done.Apr 22 2021, 2:35 AM

Generally good for me - but could you try to see if the test could be simplified any further (maybe "main" can be in test1 or test2 (replacing f1 or f2) and could call the other function in the other file) - and see if the auxiliary/input files could be represented as assembly instead of checked in object files?

I am not sure how to keep this as assembly code, since we'd need to create object files of it, and then to link it together...

Generally good for me - but could you try to see if the test could be simplified any further (maybe "main" can be in test1 or test2 (replacing f1 or f2) and could call the other function in the other file) - and see if the auxiliary/input files could be represented as assembly instead of checked in object files?

I am not sure how to keep this as assembly code, since we'd need to create object files of it, and then to link it together...

the dwo files should be able to be assembly without any particular restrictions/complications in authoring them, right? (taking the assembly of a split DWARF compilation and pulling out the .dwo parts should be adequate, I think)
The executable - it could probably actually be an object file that happens to have two skeleton units in it (like we generate under LTO, for instance - though without any of the LTO-specific cross-CU references) so that could be written in assembly relatively easily (to hand-craft it you could probably take the assembly of the two skeletons and copy/paste them into one file for instance)

Generally good for me - but could you try to see if the test could be simplified any further (maybe "main" can be in test1 or test2 (replacing f1 or f2) and could call the other function in the other file) - and see if the auxiliary/input files could be represented as assembly instead of checked in object files?

I am not sure how to keep this as assembly code, since we'd need to create object files of it, and then to link it together...

the dwo files should be able to be assembly without any particular restrictions/complications in authoring them, right? (taking the assembly of a split DWARF compilation and pulling out the .dwo parts should be adequate, I think)
The executable - it could probably actually be an object file that happens to have two skeleton units in it (like we generate under LTO, for instance - though without any of the LTO-specific cross-CU references) so that could be written in assembly relatively easily (to hand-craft it you could probably take the assembly of the two skeletons and copy/paste them into one file for instance)

I've accomplished to write these as asm files. Thanks!

djtodoro updated this revision to Diff 340017.Apr 23 2021, 7:11 AM
  • make the tests as assembly files
dblaikie added inline comments.Apr 23 2021, 1:44 PM
llvm/test/tools/llvm-dwarfdump/X86/inlined_variables_with_zero_cov.test
31

Output files should go in %t or a subdirectory of %t, so things like:

RUN: rm -r %t
RUN: mkdir %t
RUN: ... -o %t/test1.dwo
...
dblaikie accepted this revision.Apr 23 2021, 1:44 PM

Other than the test file tweak, looks good to me!

This revision was landed with ongoing or failed builds.Apr 26 2021, 2:01 AM
This revision was automatically updated to reflect the committed changes.