Page MenuHomePhabricator

[Utils] Salvage the debug info of DCE'ed 'xor' instructions
ClosedPublic

Authored by vsk on Feb 12 2018, 1:14 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

vsk created this revision.Feb 12 2018, 1:14 PM
vsk added a comment.Feb 12 2018, 1:15 PM

@djtodoro, I was hoping to rebase and land this after D43163 (taking care of test updates as needed). Wdyt?

aprantl accepted this revision.Feb 12 2018, 1:23 PM

I wonder if this actually happens in the wild, but it doesn't hurt.

This revision is now accepted and ready to land.Feb 12 2018, 1:23 PM
ibadawi accepted this revision.Feb 12 2018, 1:24 PM

LGTM

vsk added a comment.Feb 12 2018, 1:50 PM

It looks like this is able to salvage actual variables in the wild:

Assertion failed: (false && "hi! salvaged a xor!"), function salvageDebugInfo
...
Running pass 'CallGraph Pass Manager' on module '/Users/vsk/src/llvm.org-debugify/llvm/lib/Support/Regex.cpp'.

Assertion failed: (false && "hi! salvaged a xor!"), function salvageDebugInfo
...
Running pass 'CallGraph Pass Manager' on module '/Users/vsk/src/llvm.org-debugify/llvm/lib/Support/APInt.cpp'.
vsk added a comment.Feb 12 2018, 2:27 PM

Here are some better numbers from a stage2 build of clang:

12273 SALVAGE: ADD
14184 SALVAGE: MISSED
1064 SALVAGE: OR
  14 SALVAGE: SUB
 259 SALVAGE: XOR

The "MISSED" case refers to binary operators we simply haven't taught salvageDebugInfo() about.

Thanks! Might be worth also measuring the size of .debug_loc as we are doing this. We may need to start compressing locations by using DWARF procedures for common patterns (DW_OP_call).

vsk added a comment.Feb 12 2018, 4:52 PM

I extended this patch to salvage debug values from mul/sdiv/srem, shl/lhsr/ashr, and sub instructions (in addition to xor instructions). Next, I prepared dsyms for stage2 builds of clang (-O3 -g). Here are the results:

pre-patch
---------

Segment __DWARF: 814804992
        Section __debug_line: 36805639     (35.10 MB)
        Section __debug_pubnames: 8291810
        Section __debug_pubtypes: 62606366
        Section __debug_ranges: 43286544
        Section __debug_loc: 122491744     (116.81 MB)
        Section __debug_aranges: 986672
        Section __debug_info: 379630571    (362.04 MB)
        Section __debug_abbrev: 10685
        Section __debug_str: 106914677
        Section __apple_names: 35657500
        Section __apple_namespac: 50616
        Section __apple_types: 18069091
        Section __apple_objc: 36
        total 814801951

{"version":"1","file":"pre-patches-clang.dSYM/Contents/Resources/DWARF/clang-7.0","format":"Mach-O 64-bit x86-64","source functions":309766,"inlined functions":3351149,"unique source variables":1981705,"source variables":403779101,"variables with location":4127221,"scope bytes total":522119469,"scope bytes covered":305284275}

post-patch
----------

Salvaging dbg.values from:
mul/sdiv/srem, shl/lhsr/ashr, sub, xor

Segment __DWARF: 814891008
        Section __debug_line: 36806878     (35.10 MB, +0%)
        Section __debug_pubnames: 8291810
        Section __debug_pubtypes: 62606366
        Section __debug_ranges: 43287504
        Section __debug_loc: 122557034     (116.88 MB, +0.06%)
        Section __debug_aranges: 986672
        Section __debug_info: 379647347    (362.06 MB, +0%)
        Section __debug_abbrev: 10685
        Section __debug_str: 106914677
        Section __apple_names: 35658692
        Section __apple_namespac: 50616
        Section __apple_types: 18069091
        Section __apple_objc: 36
        total 814887408

{"version":"1","file":"post-patches-clang.dSYM/Contents/Resources/DWARF/clang-7.0","format":"Mach-O 64-bit x86-64","source functions":309766,"inlined functions":3351298,"unique source variables":1982115,"source variables":403824889,"variables with location":4128581,"scope bytes total":522201413,"scope bytes covered":305416207}

There is a modest increase of the debug_loc section, which indicates that users will notice a small increase in the number of available debug values, without seeing too much binary size bloat.

I also included statistics from dwarfdump: post-patch, we have 410 additional unique source variables available.

Those numbers look good!

This revision was automatically updated to reflect the committed changes.