Page MenuHomePhabricator

[DebugInfo] Switch to using constructor homing (-debug-info-kind=constructor) by default when debug info is enabled
ClosedPublic

Authored by akhuang on Jul 15 2021, 11:00 AM.

Details

Summary

Constructor homing reduces the amount of class type info that is emitted
by emitting conmplete type info for a class only when a constructor for
that class is emitted.

This will mainly reduce the amount of duplicate debug info in object
files. In Chrome enabling ctor homing decreased total build directory sizes
by about 30%.

It's also expected that some class types (such as unused classes)
will no longer be emitted in the debug info. This is fine, since we wouldn't
expect to need these types when debugging.

In some cases (e.g. libc++, https://reviews.llvm.org/D98750), classes
are used without calling the constructor. Since this is technically
undefined behavior, enabling constructor homing should be fine.
However Clang now has an attribute
__attribute__((standalone_debug)) that can be used on classes to
ignore ctor homing.

Bug: https://bugs.llvm.org/show_bug.cgi?id=46537

Diff Detail

Event Timeline

akhuang created this revision.Jul 15 2021, 11:00 AM
akhuang requested review of this revision.Jul 15 2021, 11:00 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 15 2021, 11:00 AM
dblaikie accepted this revision.Jul 15 2021, 11:12 AM
dblaikie added subscribers: probinson, aprantl.

Please wait for sign-off from @aprantl (or another appropriate Apple representative) & @probinson (or another appropriate Sony representative) as well - at least to check everyone's ready for this. (Apple I think still uses -fstandalone-debug, so won't be immediately relevant, but good to know it's coming in case some of their users are using -fno-standalone-debug or they have plans in motion, etc)

This revision is now accepted and ready to land.Jul 15 2021, 11:12 AM
JDevlieghere accepted this revision.EditedJul 16 2021, 12:07 AM

Adrian said "Let's do it!" on the mailing list so LGTM for Darwin.

probinson added a subscriber: jmorse.

+ @jmorse who is better placed than I am to say whether this is what Sony would prefer.

This is going to be excellent for linux targets and similar,

+ @jmorse who is better placed than I am to say whether this is what Sony would prefer.

Slightly trickier -- our debugger won't resolve symbols across module boundaries (similar to the Windows debugger), which will make it hard to debug when debug/no-debug code is mixed. Would it be possible to default to -debug-info-kind=limited if DebuggerTuning == llvm::DebuggerKind::SCE? This leads to the fewest surprises in a default configuration targeting us.

This is going to be excellent for linux targets and similar,

+ @jmorse who is better placed than I am to say whether this is what Sony would prefer.

Slightly trickier -- our debugger won't resolve symbols across module boundaries (similar to the Windows debugger), which will make it hard to debug when debug/no-debug code is mixed. Would it be possible to default to -debug-info-kind=limited if DebuggerTuning == llvm::DebuggerKind::SCE? This leads to the fewest surprises in a default configuration targeting us.

It'd be preferable not to split these two cases (current "limited" versus "ctor" homing) - because they rely on the same assumption, that the whole program is built with debug info (hence the renaming of "limited" a long time ago to "standalone-debug" to create a policy/philosophy around what goes in each category).

Wouldn't the current "limited" behavior have problems for this shared libraries situation too? Sounds like in that case -fstandalone-debug should be used.

(if it's a sliding scale and the problems caused by the current -fno-standalone-debug/-flimit-debug-info are not severe enough, but ctor-homing would be too severe... I'd probably be inclined to pushback on that being a distinction we should draw in-tree & that might be suitable to be kept downstream)

(hence the renaming of "limited" a long time ago to "standalone-debug" to create a policy/philosophy around what goes in each category).

Sorry, what? I thought -fstandalone-debug meant FullDebugInfo, and AFAICT that's still how the driver handles it?

Wouldn't the current "limited" behavior have problems for this shared libraries situation too? Sounds like in that case -fstandalone-debug should be used.

@jmorse am I remembering correctly, that we require dllimport-style annotations, so "limited" actually includes these types even if they aren't constructed locally? I am vague on the details here. But if ctor homing and limited both will consider a dllimport-ed type as requiring a full description, that's not a reason to pick one over the other.

jmorse accepted this revision.Jul 20 2021, 9:27 AM

It'd be preferable not to split these two cases (current "limited" versus "ctor" homing) - because they rely on the same assumption, that the whole program is built with debug info (hence the renaming of "limited" a long time ago to "standalone-debug" to create a policy/philosophy around what goes in each category).

Wouldn't the current "limited" behavior have problems for this shared libraries situation too? Sounds like in that case -fstandalone-debug should be used.

I had a dig into what "limited" currently does -- and I think Windows most closely matches our setup. Many portions don't have the source available, the debugger behaves similarly, and the default debug-info mode there is "Limited". I think what saves our bacon is the import/export clause [0, 1], if a class is used between libraries then neither Limited or Constructor debug-info will suppress its class definition.

With that in mind, I suppose we'll be alright with constructor homing on by default. The interfaces between libraries will still have type information emitted in Constructor mode, and it's not excessive for those who want to dig deeper to run with standalone-debug.

[0] https://github.com/llvm/llvm-project/blob/4a30a5c8d9f0fa8a4b6ebd7c82d5335ae7a77521/clang/lib/CodeGen/CGDebugInfo.cpp#L2391
[1] https://github.com/llvm/llvm-project/blob/4a30a5c8d9f0fa8a4b6ebd7c82d5335ae7a77521/clang/lib/CodeGen/CGDebugInfo.cpp#L2346

@jmorse am I remembering correctly, that we require dllimport-style annotations, so "limited" actually includes these types even if they aren't constructed locally? I am vague on the details here. But if ctor homing and limited both will consider a dllimport-ed type as requiring a full description, that's not a reason to pick one over the other.

For anything shared between modules, indeed it needs the annotations (we try to follow Windows here). I believe there can still be un-exported types from the other modules headers that qualify for inclusion under "Limited" mode but won't for "Constructor", and that's the benefit of Constructor mode / price if the other module isn't built with debug-info.

(hence the renaming of "limited" a long time ago to "standalone-debug" to create a policy/philosophy around what goes in each category).

Sorry, what? I thought -fstandalone-debug meant FullDebugInfo, and AFAICT that's still how the driver handles it?

Sorry, misspoke. -flimit-debug-info is -fno-standalone-debug. I meant the terminology changed, not the semantics - the terminology of "standalone" V "non-standalone" is the one that gives, in my opinion, the correct framing for when you would use one flag or the other. The core assumption of -fno-standalone-debug is that you've compiled the whole program with debug info enabled, so assumptions can be made about type debug info being carried by some other object file. If parts of the program (for instance, only one shared library has debug info and it interacts with others that don't) are built without debug info, then -fstandalone-debug is the right option to use.

@jmorse am I remembering correctly, that we require dllimport-style annotations, so "limited" actually includes these types even if they aren't constructed locally? I am vague on the details here. But if ctor homing and limited both will consider a dllimport-ed type as requiring a full description, that's not a reason to pick one over the other.

For anything shared between modules, indeed it needs the annotations (we try to follow Windows here). I believe there can still be un-exported types from the other modules headers that qualify for inclusion under "Limited" mode but won't for "Constructor", and that's the benefit of Constructor mode / price if the other module isn't built with debug-info.

I think what I'm missing here: If -fno-standalone-debug is already in use/the default and is causing missing types because parts of the program are bulit without debug info, then I'm not sure what the rationale is for slicing -fstandalone-debug into a "has ctor homing" and a "doesn't have ctor homing" strategy. The same general design philosophy applies - that in both cases (about 4 cases in total now: types that aren't required to be complete, explicitly instantiated templates, vtables, and now ctor homing) the whole program must be compiled with debug info enabled for these DWARF size optimizations to be sound.

Is there some set of cases where the current set of homing strategies is sound, but ctor homing is not sound?

akhuang updated this revision to Diff 360621.Jul 21 2021, 3:27 PM

Add an opt out flag: fno-use-ctor-homing

Realized it's probably a good idea to add an opt out flag (counterpart to fuse-ctor-homing). Also, maybe in a separate patch, maybe should make these clang flags instead of cc1 flags-

probinson added inline comments.Jul 22 2021, 6:03 AM
clang/lib/Frontend/CompilerInvocation.cpp
1636 ↗(On Diff #360621)

No... you want to check both options in one call, otherwise -fno-use-ctor-homing -fuse-ctor-homing will prefer the no version instead of last-one-wins.

I suggest fiddling the options should be done separately.
Also if you want to make it a clang option, the option name should have debug in it; pretty sure all the -f options related to debug info do that, and in any case it's a good idea.

akhuang added inline comments.Jul 22 2021, 12:04 PM
clang/lib/Frontend/CompilerInvocation.cpp
1636 ↗(On Diff #360621)

ah, right.

I'll move this to a separate patch.

akhuang updated this revision to Diff 360918.Jul 22 2021, 12:05 PM

Remove fno-use-ctor-homing flag

David wrote:

think what I'm missing here: If -fno-standalone-debug is already in use/the default and is causing missing types because parts of the program are bulit without debug info, then I'm not sure what the rationale is for slicing -fstandalone-debug into a "has ctor homing" and a "doesn't have ctor homing" strategy. The same general design philosophy applies - that in both cases (about 4 cases in total now: types that aren't required to be complete, explicitly instantiated templates, vtables, and now ctor homing) the whole program must be compiled with debug info enabled for these DWARF size optimizations to be sound.

It's a matter of degree:

  • Some types go missing, but due to the dllimport mechanism mentioned, all the important ones are retained,
  • Switching to -fstandalone-debug increases the amount of DWARF by ~50% in experiments I've run, which for a {single-file-recompile, link, load-into-debugger} round trip will translate to an almost 50% increase in round-trip-time. (What with DWARF being the major part of linking and debugger-loading).

Thus the status quo (-fno-standalone-debug and closed-source libraries) hasn't been conceptually sounds, but it's given a "good enough" debugging experience without major round-trip-time penalty.

David wrote:

think what I'm missing here: If -fno-standalone-debug is already in use/the default and is causing missing types because parts of the program are bulit without debug info, then I'm not sure what the rationale is for slicing -fstandalone-debug into a "has ctor homing" and a "doesn't have ctor homing" strategy. The same general design philosophy applies - that in both cases (about 4 cases in total now: types that aren't required to be complete, explicitly instantiated templates, vtables, and now ctor homing) the whole program must be compiled with debug info enabled for these DWARF size optimizations to be sound.

It's a matter of degree:

  • Some types go missing, but due to the dllimport mechanism mentioned, all the important ones are retained,
  • Switching to -fstandalone-debug increases the amount of DWARF by ~50% in experiments I've run, which for a {single-file-recompile, link, load-into-debugger} round trip will translate to an almost 50% increase in round-trip-time. (What with DWARF being the major part of linking and debugger-loading).

Thus the status quo (-fno-standalone-debug and closed-source libraries) hasn't been conceptually sounds, but it's given a "good enough" debugging experience without major round-trip-time penalty.

Right, I think chromium on mac does the same thing (build with -fno-standalone-debug even though it drops some types, because -fstandalone-debug is too large).

David wrote:

think what I'm missing here: If -fno-standalone-debug is already in use/the default and is causing missing types because parts of the program are bulit without debug info, then I'm not sure what the rationale is for slicing -fstandalone-debug into a "has ctor homing" and a "doesn't have ctor homing" strategy. The same general design philosophy applies - that in both cases (about 4 cases in total now: types that aren't required to be complete, explicitly instantiated templates, vtables, and now ctor homing) the whole program must be compiled with debug info enabled for these DWARF size optimizations to be sound.

It's a matter of degree:

  • Some types go missing, but due to the dllimport mechanism mentioned, all the important ones are retained,
  • Switching to -fstandalone-debug increases the amount of DWARF by ~50% in experiments I've run, which for a {single-file-recompile, link, load-into-debugger} round trip will translate to an almost 50% increase in round-trip-time. (What with DWARF being the major part of linking and debugger-loading).

Thus the status quo (-fno-standalone-debug and closed-source libraries) hasn't been conceptually sounds, but it's given a "good enough" debugging experience without major round-trip-time penalty.

I think it'd be unfortunate to split this hair in LLVM/Clang proper & feel like that sort of value tradeoff might be better suited in a downstream patch. (mostly because eventually it'd be good to get rid of the distinction between other type homing and ctor type homing entirely - there's already 3 categories of type homing under the existing category (type completeness, explicit template instantiations, and vtable based homing) & I don't think there's a good line to draw between those and ctor type homing - and module type homing (which I think I've already implemented &the is under -fno-standalone-debug, but no one's using modular code generation right now so it's not super interesting to anyone), maybe ThinLTO type homing (which would be a bit more robust than the others, since it has whole program-ish view)

dblaikie accepted this revision.Jul 26 2021, 2:28 PM

Sounds like this is good for Google and Sony, and Apple doesn't use -fno-standalone-debug/-flimit-debug-info anyway, so probably about ready to move forward, then?

yep, should be good now - I'll commit it soon

This revision was landed with ongoing or failed builds.Jul 26 2021, 5:25 PM
This revision was automatically updated to reflect the committed changes.