[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a subscriber: JDevlieghere.
dblaikie added a comment.

In D71451#1793828 <https://reviews.llvm.org/D71451#1793828>, @Jac1494 wrote:

> > Am I reading this right that the data would suggest that enabling this feature /reduces/ the size of debug info sections? That doesn't sound right - can you explain why that would be the case? (perhaps the data is incorrect/it wasn't built with -fstandalone-debug?)
>
> Hi @dblaikie  sorry for incorrect data.
>
> Updated result are as per below:
>
> Without "-fstandalone-debug" option :-
>  $size -A -d build_debug_withoutstandalone/bin/clang-10
>  ...
>  ...
>  .comment                     159         0
>  .debug_str               2655675         0
>  .debug_loc                   941         0
>  .debug_abbrev              10761         0
>  .debug_info              1526674         0
>  .debug_ranges              46672         0
>  .debug_line               149807         0
>  .note.gnu.gold-version        28         0
>  Total                    4786206
>
> With "-fstandalone-debug" option :-
>  $size -A -d build_debug_withstandalone/bin/clang-10
>  ...
>  ...
>  .comment                     159         0
>  .debug_str               3997839         0
>  .debug_loc                   941         0
>  .debug_abbrev              12746         0
>  .debug_info              2225392         0
>  .debug_ranges              46672         0
>  .debug_line               153779         0
>  .note.gnu.gold-version        28         0
>  Total                    6833045


Ah, that looks more like what I'd expect - so that's a 42% increase in the final linked binary size? Yeah, that seems somewhat unlikely to be what anyone would want to enable, though I could be wrong (@aprantl @JDevlieghere - you folks use standalone-debug by default, have any opinions on this proposed change to standalone-debug behavior?)

Could you audit some files & see what sort of things are going in there? It might be that a more nuanced definition of "used" is needed.

For instance, does this global variable get emitted?

  extern int i;
  inline int f1() {
    return i;
  }
  void f2() {
  }

'i' is used according to the AST, but it's not used even statically by this translation unit - f1 is never called here. So if this patch currently emits a declaration of 'i' then it shows one way there's some room for improvement - specifically I'd motivate the declaration emission by the IR usage itself - does the LLVM IR end up with a declaration of 'i' in it (for the purposes of loading/storing to it, etc) then add a declaration. This won't catch all cases (eg: "extern const int  i = 7; int f() { int x[i]; ... }" - there won't be any use of 'i' in the IR in that case), but would potentially be a workable tradeoff for now. Catching the "use of a global in a constant context" is part of a much broader challenge - to get that and some other existing cases like it right, we'd need some kind of reachability analysis of usage. That analysis could benefit other areas of debug info too, but is probably a lot more work.


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

https://reviews.llvm.org/D71451