[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
dwblaikie wrote: > Looking forward to trying this out! > > Should the new flag have some docs and maybe be mentioned in the release > notes, or do we think it's not ready for prime time yet for some reason? I'm /pretty/ neutral on that - it's got pretty clear behavior, etc (& in fact some class members already had this handling -implicit special members, member templates, and nested types) - but I expect the ecosystem (debuggers - gdb and lldb) won't work well with this. It's not too hard to observe GDB showing a version of a different type depending on the context you're in - so name resolution will fail depending on which copy of the type the debugger is using at the time. LLDB I expect will fail even harder, because it creates fewer copies of a type - -so it'll pick one seemingly at random and only give you that view, even in some other context where GDB would give the type view local to that context at least, I think. https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
joker-eph wrote: Reverted in https://github.com/llvm/llvm-project/pull/93767 (maybe it's just a missing explicit triple in the test?) https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
joker-eph wrote: It also fails on Windows: https://lab.llvm.org/buildbot/#/builders/271/builds/8095 https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
dyung wrote: Hi @dwblaikie the test debug-options.c is failing on the macOS build bot. Can you take a look? https://lab.llvm.org/buildbot/#/builders/280/builds/4510 https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
https://github.com/dwblaikie closed https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
pogo59 wrote: What @SLTozer said. I don't want "members" to mean "some but not all members" and "methods" was shorter than "member-functions" (but I'm okay with "member-functions"). https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
https://github.com/SLTozer approved this pull request. > Yeah, seems I'm outvoted here. I'm a bit of a pedant for the C++ standard > language, which doesn't talk about "methods", only "member functions". All I'd say is that if we went with members, it ought to be `-gomit-unreferenced-member-functions` rather than `-gomit-unreferenced-members`; I'm not strongly attached to "methods" over "member functions", but just "members" is misleading imo. In any case, the patch functionally LGTM. https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
https://github.com/SLTozer edited https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
@@ -4260,6 +4260,13 @@ defm strict_dwarf : BoolOption<"g", "strict-dwarf", "the specified version, avoiding features from later versions.">, NegFlag, BothFlags<[], [ClangOption, CLOption, DXCOption]>>, Group; +defm omit_unreferenced_members : BoolOption<"g", "omit-unreferenced-members", + CodeGenOpts<"DebugOmitUnreferencedMembers">, DefaultFalse, + PosFlag, + NegFlag, BothFlags<[], [ClangOption, CLOption, DXCOption]>>, + Group; SLTozer wrote: It seems you're right - the marshalling stuff allows us to omit boilerplate in a few places, but `renderDebugOptions` isn't one of them; I'd missed that `BoolOption` was already covering that base as well, so carry on I think (though the `BoolGOption` change does seem sensible). https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
@@ -4260,6 +4260,13 @@ defm strict_dwarf : BoolOption<"g", "strict-dwarf", "the specified version, avoiding features from later versions.">, NegFlag, BothFlags<[], [ClangOption, CLOption, DXCOption]>>, Group; +defm omit_unreferenced_members : BoolOption<"g", "omit-unreferenced-members", + CodeGenOpts<"DebugOmitUnreferencedMembers">, DefaultFalse, + PosFlag, + NegFlag, BothFlags<[], [ClangOption, CLOption, DXCOption]>>, + Group; dwblaikie wrote: I looked into this - and I couldn't find any example where MarshallingInfoFlag would allow us to omit the renderDebugOptions code - do you have an example in mind? And it seems like BoolOption could even be replaced with BoolGOption, and that either of those does use Marshalling stuff? (the BoolOption< def in clang Options.td has some `MarshalledFlagRec` stuff at the end?) https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
dwblaikie wrote: > I think the comment about `s/members/methods/` is still outstanding - I agree > that methods is more descriptive than members. Yeah, seems I'm outvoted here. I'm a bit of a pedant for the C++ standard language, which doesn't talk about "methods", only "member functions". But anyway, since you're likeyl the first folks to use this, I've made that change toward "methods". > I'm +1 on having this be non-default; adding it to SCE tuning is also not > necessary (or desired) for now, because this is more aggressive than our > private option (we keep entries for member functions that are called), and > we're still working out whether we can/should adopt this instead. Yeah, no plans to make this the default any time soon - will start with it entirely off by default, and figured you Sony folks can see if it can be your default in the future - save you carrying the downstream patches, gives you some hope other folks might adopt the same strategy (& folks considering adopting it would have some reassurance that other people are living with it/interested in caring for it already) https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
@@ -4260,6 +4260,13 @@ defm strict_dwarf : BoolOption<"g", "strict-dwarf", "the specified version, avoiding features from later versions.">, NegFlag, BothFlags<[], [ClangOption, CLOption, DXCOption]>>, Group; +defm omit_unreferenced_members : BoolOption<"g", "omit-unreferenced-members", + CodeGenOpts<"DebugOmitUnreferencedMembers">, DefaultFalse, + PosFlag, + NegFlag, BothFlags<[], [ClangOption, CLOption, DXCOption]>>, + Group; dwblaikie wrote: Wasn't able to get this to avoid the `renderDebugOptions` code - even other uses of marshalling (and it looks like BoolOption, and even BoolGOption, have some marshalling details in their implementation - so maybe they're getting the same functionality as MarshallingInfoFlag already?) seem to still have to handle/repeat the flag from the driver to the frontend. Oh, and now that I'm checking compatible flags in the driver (disabling this feature if -fstandalone-debug or -fdebug-types-section are enabled) then there's probably no avoiding having some code there anyway. Switching to BoolGOption does make it a bit tidier, though. https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
@@ -2755,7 +2755,7 @@ CGDebugInfo::CreateTypeDefinition(const RecordType *Ty) {
// Collect data fields (including static variables and any initializers).
CollectRecordFields(RD, DefUnit, EltTys, FwdDecl);
- if (CXXDecl)
+ if (CXXDecl && !CGM.getCodeGenOpts().DebugOmitUnreferencedMembers)
dwblaikie wrote:
Added that at the driver level - and similarly for type units (since they use
an mllvm flag, clang frontend/codegen doesn't actually know if type units are
enabled). (though this doesn't work out /that/ badly for type units, in some
sense - the type units are still consistent, they just consistently contain no
member functions (in the same way that even without the flag, for things like
member function template instantiations we never put them in the member list,
so they never end up in type units - only attached as declarations to the
skeleton type DIE in the CU))
https://github.com/llvm/llvm-project/pull/87018
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
@@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -debug-info-kind=limited -gomit-unreferenced-members %s -emit-llvm -o - | FileCheck %s dwblaikie wrote: Done https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
https://github.com/dwblaikie updated
https://github.com/llvm/llvm-project/pull/87018
>From 6834c245205d1e38a615e97217dada3cd941ed03 Mon Sep 17 00:00:00 2001
From: David Blaikie
Date: Fri, 2 Jun 2023 15:04:14 +
Subject: [PATCH 1/3] [DebugInfo] Add flag to only emit referenced member
functions
Complete C++ type information can be quite expensive - and there's
limited value in representing every member function, even those that
can't be called (we don't do similarly for every non-member function
anyway). So add a flag to opt out of this behavior for experimenting
with this more terse behavior.
I think Sony already does this by default, so perhaps with a change to
the defaults, Sony can migrate to this rather than a downstream patch.
This breaks current debuggers in some expected ways - but those
breakages are visible without this feature too. Consider member function
template instantiations - they can't be consistently enumerated in every
translation unit:
a.h:
```
struct t1 {
template
static int f1() {
return i;
}
};
namespace ns {
template
int f1() {
return i;
}
} // namespace ns
```
a.cpp:
```
void f1() {
t1::f1<0>();
ns::f1<0>();
}
```
b.cpp:
```
void f1();
int main() {
f1();
t1::f1<1>();
ns::f1<1>();
}
```
```
(gdb) p ns::f1<0>()
$1 = 0
(gdb) p ns::f1<1>()
$2 = 1
(gdb) p t1::f1<0>()
Couldn't find method t1::f1<0>
(gdb) p t1::f1<1>()
$3 = 1
(gdb) s
f1 () at a.cpp:3
3 t1::f1<0>();
(gdb) p t1::f1<0>()
$4 = 0
(gdb) p t1::f1<1>()
Couldn't find method t1::f1<1>
(gdb)
```
(other similar non-canonical features are implicit special members
(copy/move ctor/assignment operator, default ctor) and nested types (eg:
pimpl idiom, where the nested type is declared-but-not-defined in one
TU, and defined in another TU))
lldb can't parse the template expressions above, so I'm not sure how to
test it there, but I'd guess it has similar problems. (
https://stackoverflow.com/questions/64602475/how-to-print-value-returned-by-template-member-function-in-gdb-lldb-debugging
so... I guess that's just totally not supported in lldb, how
unfortunate. And implicit special members are instantiated implicitly by
lldb, so missing those doesn't tickle the same issue)
Some very rudimentary numbers for a clang debug build:
.debug_info section size:
-g: 476MiB
-g -fdebug-types-section: 357MiB
-g -gomit-unreferenced-members: 340MiB
Though it also means a major reduction in .debug_str size,
-fdebug-types-section doesn't reduce string usage (so the first two
examples have the same .debug_str size, 247MiB), down to 175MiB.
So for total clang binary size (I don't have a quick "debug section size
reduction" on-hand): 1.45 (no type units) GiB -> 1.34 -> 1.22, so it
saves about 120MiB of binary size.
Also open to any riffing on the flag name for sure.
@probinson - would this be an accurate upstreaming of your internal
handling/would you use this functionality? If it wouldn't be useful to you,
it's maybe not worth adding upstream yet - not sure we'll use it at Google, but
if it was useful to you folks and meant other folks could test with it it
seemed maybe useful.
Original Differential Revision: https://reviews.llvm.org/D152017
---
clang/include/clang/Basic/DebugOptions.def | 2 ++
clang/include/clang/Driver/Options.td| 7 +++
clang/lib/CodeGen/CGDebugInfo.cpp| 2 +-
clang/lib/Driver/ToolChains/Clang.cpp| 6 ++
.../test/CodeGenCXX/debug-info-incomplete-types.cpp | 12
clang/test/Driver/debug-options.c| 6 ++
6 files changed, 34 insertions(+), 1 deletion(-)
create mode 100644 clang/test/CodeGenCXX/debug-info-incomplete-types.cpp
diff --git a/clang/include/clang/Basic/DebugOptions.def
b/clang/include/clang/Basic/DebugOptions.def
index 7cd3edf08a17e..fe2adaa20f4e3 100644
--- a/clang/include/clang/Basic/DebugOptions.def
+++ b/clang/include/clang/Basic/DebugOptions.def
@@ -68,6 +68,8 @@ BENIGN_DEBUGOPT(NoInlineLineTables, 1, 0) ///< Whether debug
info should contain
///< inline line tables.
DEBUGOPT(DebugStrictDwarf, 1, 1) ///< Whether or not to use strict DWARF info.
+DEBUGOPT(DebugOmitUnreferencedMembers, 1, 0) ///< Omit unreferenced member
+///< functions in type debug info.
/// Control the Assignment Tracking debug info feature.
BENIGN_ENUM_DEBUGOPT(AssignmentTrackingMode, AssignmentTrackingOpts, 2,
diff --git a/clang/include/clang/Driver/Options.td
b/clang/include/clang/Driver/Options.td
index 29066ea14280c..4000403a1991d 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4260,6 +4260,13 @@ defm strict_dwarf : BoolOption<"g", "strict-dwarf",
"the specified version, avoiding features from later versions.">,
NegFlag, BothFlags<[], [ClangOption, CLOption, DXCOption]>>,
Group;
+defm omit_unreferenced_members : BoolOption<"g
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
@@ -4260,6 +4260,13 @@ defm strict_dwarf : BoolOption<"g", "strict-dwarf", "the specified version, avoiding features from later versions.">, NegFlag, BothFlags<[], [ClangOption, CLOption, DXCOption]>>, Group; +defm omit_unreferenced_members : BoolOption<"g", "omit-unreferenced-members", + CodeGenOpts<"DebugOmitUnreferencedMembers">, DefaultFalse, + PosFlag, + NegFlag, BothFlags<[], [ClangOption, CLOption, DXCOption]>>, + Group; SLTozer wrote: As a small matter of convenience, since this is just being passed straight through the driver I think this could be a marshalling flag, which iirc would allow us to omit the `renderDebugOptions` code? ```suggestion defm omit_unreferenced_members : Flag<["-"], "gomit-unreferenced-members">, Group, Visibility<[ClangOption, CC1Option]>, HelpText<"Omit member function declarations from type descriptions if the " "member is unreferenced.">, MarshallingInfoFlag>; ``` https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
https://github.com/SLTozer commented: I think the comment about `s/members/methods/` is still outstanding - I agree that methods is more descriptive than members. I'm +1 on having this be non-default; adding it to SCE tuning is also not necessary (or desired) for now, because this is more aggressive than our private option (we keep entries for member functions that are called), and we're still working out whether we can/should adopt this instead. Otherwise, existing comments aside, this looks good: the size reduction is considerable, and likely becoming necessary for some. https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
@@ -2755,7 +2755,7 @@ CGDebugInfo::CreateTypeDefinition(const RecordType *Ty) {
// Collect data fields (including static variables and any initializers).
CollectRecordFields(RD, DefUnit, EltTys, FwdDecl);
- if (CXXDecl)
+ if (CXXDecl && !CGM.getCodeGenOpts().DebugOmitUnreferencedMembers)
SLTozer wrote:
>From the old review, there was a question about whether this should be
>disabled by `-fstandalone-debug` - it seemed like there was a reasonable case
>for (being able to call member functions from a different TU without debug
>info), and a reasonable case against (inconsistency with non-member
>functions), and it's not totally clear whether that was resolved.
https://github.com/llvm/llvm-project/pull/87018
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
https://github.com/SLTozer edited https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
@@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -debug-info-kind=limited -gomit-unreferenced-members %s -emit-llvm -o - | FileCheck %s SLTozer wrote: Test needs renaming for the different flag name? https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
chrdavis wrote: Building Chromium with this change shows a decrease of 35% for the PDB TPI size. The TPI size is capped at 2GB due to a signed int limitation. Since Chromium to approaching this limitation having this flag would be extremely beneficial. Can we get this PR completed soon? https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
adrian-prantl wrote: Seems to be a reasonable tuning option to have available. I probably wouldn't want this to be on by default, but I can see the appeal. https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
rnk wrote: To restate the finding, 29% of .debug_info is describing class methods, at least in Clang. I think this is a useful mode, and we should land it as is. There are many users up against the scaling limits of debug info size, and it's helpful to have this as an option for experimentation in the field. We should document somewhere that this flat is known to be incompatible with some debugger features. https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
pogo59 wrote: Thanks for the link back to the Phab review, that was helpful. I didn't offhand recall the previous round of this. I'll trust the comments I made on that review. :) s/members/methods/ in the option name to avoid the incorrect implication of suppressing data members? I suggest _not_ omitting methods when generating type sections. If there's only one description (in the final executable) it ought to be complete. If you omit undefined methods, and the linker picks the section from a CU with no method definitions, you end up with a type description that has no methods at all, which is not especially useful. https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
dwblaikie wrote: Cleaning up some old branches - @pogo59 @rnk who commented on the original https://reviews.llvm.org/D152017 I think the only outstanding thing was the flag name, I've renamed it from `-gincomplete-types` to `-gomit-unreferenced-members` to try to address the feedback. It's not totally precise (it's only member functions that are affected, not member variables, for instance) but seems close enough. https://github.com/llvm/llvm-project/pull/87018 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
llvmbot wrote:
@llvm/pr-subscribers-clang-driver
Author: David Blaikie (dwblaikie)
Changes
Complete C++ type information can be quite expensive - and there's
limited value in representing every member function, even those that
can't be called (we don't do similarly for every non-member function
anyway). So add a flag to opt out of this behavior for experimenting
with this more terse behavior.
I think Sony already does this by default, so perhaps with a change to
the defaults, Sony can migrate to this rather than a downstream patch.
This breaks current debuggers in some expected ways - but those
breakages are visible without this feature too. Consider member function
template instantiations - they can't be consistently enumerated in every
translation unit:
a.h:
```
struct t1 {
template
static int f1() {
return i;
}
};
namespace ns {
template
int f1() {
return i;
}
} // namespace ns
```
a.cpp:
```
void f1() {
t1::f1<0>();
ns::f1<0>();
}
```
b.cpp:
```
void f1();
int main() {
f1();
t1::f1<1>();
ns::f1<1>();
}
```
```
(gdb) p ns::f1<0>()
$1 = 0
(gdb) p ns::f1<1>()
$2 = 1
(gdb) p t1::f1<0>()
Couldn't find method t1::f1<0>
(gdb) p t1::f1<1>()
$3 = 1
(gdb) s
f1 () at a.cpp:3
3 t1::f1<0>();
(gdb) p t1::f1<0>()
$4 = 0
(gdb) p t1::f1<1>()
Couldn't find method t1::f1<1>
(gdb)
```
(other similar non-canonical features are implicit special members
(copy/move ctor/assignment operator, default ctor) and nested types (eg:
pimpl idiom, where the nested type is declared-but-not-defined in one
TU, and defined in another TU))
lldb can't parse the template expressions above, so I'm not sure how to
test it there, but I'd guess it has similar problems. (
https://stackoverflow.com/questions/64602475/how-to-print-value-returned-by-template-member-function-in-gdb-lldb-debugging
so... I guess that's just totally not supported in lldb, how
unfortunate. And implicit special members are instantiated implicitly by
lldb, so missing those doesn't tickle the same issue)
Some very rudimentary numbers for a clang debug build:
.debug_info section size:
-g: 476MiB
-g -fdebug-types-section: 357MiB
-g -gomit-unreferenced-members: 340MiB
Though it also means a major reduction in .debug_str size,
-fdebug-types-section doesn't reduce string usage (so the first two
examples have the same .debug_str size, 247MiB), down to 175MiB.
So for total clang binary size (I don't have a quick "debug section size
reduction" on-hand): 1.45 (no type units) GiB -> 1.34 -> 1.22, so it
saves about 120MiB of binary size.
Also open to any riffing on the flag name for sure.
@probinson - would this be an accurate upstreaming of your internal
handling/would you use this functionality? If it wouldn't be useful to you,
it's maybe not worth adding upstream yet - not sure we'll use it at Google, but
if it was useful to you folks and meant other folks could test with it it
seemed maybe useful.
Original Differential Revision: https://reviews.llvm.org/D152017
---
Full diff: https://github.com/llvm/llvm-project/pull/87018.diff
6 Files Affected:
- (modified) clang/include/clang/Basic/DebugOptions.def (+2)
- (modified) clang/include/clang/Driver/Options.td (+7)
- (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+1-1)
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+6)
- (added) clang/test/CodeGenCXX/debug-info-incomplete-types.cpp (+12)
- (modified) clang/test/Driver/debug-options.c (+6)
``diff
diff --git a/clang/include/clang/Basic/DebugOptions.def
b/clang/include/clang/Basic/DebugOptions.def
index 7cd3edf08a17ea..fe2adaa20f4e3a 100644
--- a/clang/include/clang/Basic/DebugOptions.def
+++ b/clang/include/clang/Basic/DebugOptions.def
@@ -68,6 +68,8 @@ BENIGN_DEBUGOPT(NoInlineLineTables, 1, 0) ///< Whether debug
info should contain
///< inline line tables.
DEBUGOPT(DebugStrictDwarf, 1, 1) ///< Whether or not to use strict DWARF info.
+DEBUGOPT(DebugOmitUnreferencedMembers, 1, 0) ///< Omit unreferenced member
+///< functions in type debug info.
/// Control the Assignment Tracking debug info feature.
BENIGN_ENUM_DEBUGOPT(AssignmentTrackingMode, AssignmentTrackingOpts, 2,
diff --git a/clang/include/clang/Driver/Options.td
b/clang/include/clang/Driver/Options.td
index 29066ea14280c2..4000403a1991d2 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4260,6 +4260,13 @@ defm strict_dwarf : BoolOption<"g", "strict-dwarf",
"the specified version, avoiding features from later versions.">,
NegFlag, BothFlags<[], [ClangOption, CLOption, DXCOption]>>,
Group;
+defm omit_unreferenced_members : BoolOption<"g", "omit-unreferenced-members",
+ CodeGenOpts<"DebugOmitUnreferencedMembers">, DefaultFalse,
+ PosFlag,
+ NegFlag, BothFlags<[], [ClangOption, CLOption, DXCOption]>>,
+ Group;
defm column_info : BoolOption<"g", "column-info",
CodeGenOpts<
[clang] [DebugInfo] Add flag to only emit referenced member functions (PR #87018)
https://github.com/dwblaikie created
https://github.com/llvm/llvm-project/pull/87018
Complete C++ type information can be quite expensive - and there's
limited value in representing every member function, even those that
can't be called (we don't do similarly for every non-member function
anyway). So add a flag to opt out of this behavior for experimenting
with this more terse behavior.
I think Sony already does this by default, so perhaps with a change to
the defaults, Sony can migrate to this rather than a downstream patch.
This breaks current debuggers in some expected ways - but those
breakages are visible without this feature too. Consider member function
template instantiations - they can't be consistently enumerated in every
translation unit:
a.h:
```
struct t1 {
template
static int f1() {
return i;
}
};
namespace ns {
template
int f1() {
return i;
}
} // namespace ns
```
a.cpp:
```
void f1() {
t1::f1<0>();
ns::f1<0>();
}
```
b.cpp:
```
void f1();
int main() {
f1();
t1::f1<1>();
ns::f1<1>();
}
```
```
(gdb) p ns::f1<0>()
$1 = 0
(gdb) p ns::f1<1>()
$2 = 1
(gdb) p t1::f1<0>()
Couldn't find method t1::f1<0>
(gdb) p t1::f1<1>()
$3 = 1
(gdb) s
f1 () at a.cpp:3
3 t1::f1<0>();
(gdb) p t1::f1<0>()
$4 = 0
(gdb) p t1::f1<1>()
Couldn't find method t1::f1<1>
(gdb)
```
(other similar non-canonical features are implicit special members
(copy/move ctor/assignment operator, default ctor) and nested types (eg:
pimpl idiom, where the nested type is declared-but-not-defined in one
TU, and defined in another TU))
lldb can't parse the template expressions above, so I'm not sure how to
test it there, but I'd guess it has similar problems. (
https://stackoverflow.com/questions/64602475/how-to-print-value-returned-by-template-member-function-in-gdb-lldb-debugging
so... I guess that's just totally not supported in lldb, how
unfortunate. And implicit special members are instantiated implicitly by
lldb, so missing those doesn't tickle the same issue)
Some very rudimentary numbers for a clang debug build:
.debug_info section size:
-g: 476MiB
-g -fdebug-types-section: 357MiB
-g -gomit-unreferenced-members: 340MiB
Though it also means a major reduction in .debug_str size,
-fdebug-types-section doesn't reduce string usage (so the first two
examples have the same .debug_str size, 247MiB), down to 175MiB.
So for total clang binary size (I don't have a quick "debug section size
reduction" on-hand): 1.45 (no type units) GiB -> 1.34 -> 1.22, so it
saves about 120MiB of binary size.
Also open to any riffing on the flag name for sure.
@probinson - would this be an accurate upstreaming of your internal
handling/would you use this functionality? If it wouldn't be useful to you,
it's maybe not worth adding upstream yet - not sure we'll use it at Google, but
if it was useful to you folks and meant other folks could test with it it
seemed maybe useful.
Original Differential Revision: https://reviews.llvm.org/D152017
>From 6834c245205d1e38a615e97217dada3cd941ed03 Mon Sep 17 00:00:00 2001
From: David Blaikie
Date: Fri, 2 Jun 2023 15:04:14 +
Subject: [PATCH] [DebugInfo] Add flag to only emit referenced member functions
Complete C++ type information can be quite expensive - and there's
limited value in representing every member function, even those that
can't be called (we don't do similarly for every non-member function
anyway). So add a flag to opt out of this behavior for experimenting
with this more terse behavior.
I think Sony already does this by default, so perhaps with a change to
the defaults, Sony can migrate to this rather than a downstream patch.
This breaks current debuggers in some expected ways - but those
breakages are visible without this feature too. Consider member function
template instantiations - they can't be consistently enumerated in every
translation unit:
a.h:
```
struct t1 {
template
static int f1() {
return i;
}
};
namespace ns {
template
int f1() {
return i;
}
} // namespace ns
```
a.cpp:
```
void f1() {
t1::f1<0>();
ns::f1<0>();
}
```
b.cpp:
```
void f1();
int main() {
f1();
t1::f1<1>();
ns::f1<1>();
}
```
```
(gdb) p ns::f1<0>()
$1 = 0
(gdb) p ns::f1<1>()
$2 = 1
(gdb) p t1::f1<0>()
Couldn't find method t1::f1<0>
(gdb) p t1::f1<1>()
$3 = 1
(gdb) s
f1 () at a.cpp:3
3 t1::f1<0>();
(gdb) p t1::f1<0>()
$4 = 0
(gdb) p t1::f1<1>()
Couldn't find method t1::f1<1>
(gdb)
```
(other similar non-canonical features are implicit special members
(copy/move ctor/assignment operator, default ctor) and nested types (eg:
pimpl idiom, where the nested type is declared-but-not-defined in one
TU, and defined in another TU))
lldb can't parse the template expressions above, so I'm not sure how to
test it there, but I'd guess it has similar problems. (
https://stackoverflow.com/questions/64602475/how-to-print-value-returned-by-template-member-function-in-gdb-lldb-debugging
so... I guess that's just totally not supported in lldb, how
unfortunate
