[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:335
 
+  /// A key method to reduce duplicate type info from Sema.
+  virtual void anchor();

rnk wrote:
> erichkeane wrote:
> > rnk wrote:
> > > hans wrote:
> > > > I worry that this is going to look obscure to most readers passing 
> > > > through. Maybe it could be expanded to more explicitly spell out that 
> > > > it reduces the size of the debug info?
> > > I want to keep it concise, most readers shouldn't need to know what this 
> > > is, and they can look up technical terms like "key method". I'll say 
> > > "debug info" instead of "type info", though, that should be more obvious.
> > FWIW, I just ran into this and did a double/triple take, as it didn't make 
> > sense for me to see a 'virtual' function in a 'final' type that didn't 
> > inherit to anything looked like nonsense.
> > 
> > The only way I found out what this meant (googling "key method" did very 
> > little for me here) was to do a 'git-blame' then found this review.  The 
> > ONLY place that explained what is happening here is the comment you made 
> > here: https://reviews.llvm.org/D70340#1752192
> Sorry, I went ahead and wrote better comments in 
> rG6d78c38986fa0974ea0b37e66f8cb89b256f4e0d.
> 
> Re: key functions, this is where the idea is documented:
> https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-vtable
> They control where the vtable is emitted. We have this style rule to take 
> advantage of them:
> https://llvm.org/docs/CodingStandards.html#provide-a-virtual-method-anchor-for-classes-in-headers
> However, the existing rule has to do with RTTI and vtables, which doesn't 
> make any sense for Sema.
> 
> The idea that class debug info is tied to the vtable "known", but not well 
> documented. It is mentioned maybe once in the user manual:
> https://clang.llvm.org/docs/UsersManual.html#cmdoption-fstandalone-debug
> I couldn't find any GCC documentation about this behavior, so we're doing 
> better. :)
> 
> @akhuang has been working on the constructor homing feature announced here:
> https://blog.llvm.org/posts/2021-04-05-constructor-homing-for-debug-info/
> So maybe in the near future we won't need this hack.
Thanks!  That is at least more descriptive that a virtual function in a 
non-inheriting final type is intentional and not just silliness.   I appreciate 
the change!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: akhuang.
rnk added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:335
 
+  /// A key method to reduce duplicate type info from Sema.
+  virtual void anchor();

erichkeane wrote:
> rnk wrote:
> > hans wrote:
> > > I worry that this is going to look obscure to most readers passing 
> > > through. Maybe it could be expanded to more explicitly spell out that it 
> > > reduces the size of the debug info?
> > I want to keep it concise, most readers shouldn't need to know what this 
> > is, and they can look up technical terms like "key method". I'll say "debug 
> > info" instead of "type info", though, that should be more obvious.
> FWIW, I just ran into this and did a double/triple take, as it didn't make 
> sense for me to see a 'virtual' function in a 'final' type that didn't 
> inherit to anything looked like nonsense.
> 
> The only way I found out what this meant (googling "key method" did very 
> little for me here) was to do a 'git-blame' then found this review.  The ONLY 
> place that explained what is happening here is the comment you made here: 
> https://reviews.llvm.org/D70340#1752192
Sorry, I went ahead and wrote better comments in 
rG6d78c38986fa0974ea0b37e66f8cb89b256f4e0d.

Re: key functions, this is where the idea is documented:
https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-vtable
They control where the vtable is emitted. We have this style rule to take 
advantage of them:
https://llvm.org/docs/CodingStandards.html#provide-a-virtual-method-anchor-for-classes-in-headers
However, the existing rule has to do with RTTI and vtables, which doesn't make 
any sense for Sema.

The idea that class debug info is tied to the vtable "known", but not well 
documented. It is mentioned maybe once in the user manual:
https://clang.llvm.org/docs/UsersManual.html#cmdoption-fstandalone-debug
I couldn't find any GCC documentation about this behavior, so we're doing 
better. :)

@akhuang has been working on the constructor homing feature announced here:
https://blog.llvm.org/posts/2021-04-05-constructor-homing-for-debug-info/
So maybe in the near future we won't need this hack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I guess my point is: a better comment would have saved me some time.  Basically 
point out that the 'debug' info for the whole type is emitted with a virtual 
method, and that non-virtual types have it emitted in every TU.  Also that this 
causes it to be emitted in only 1 place, since now there is only a single 
virtual method definition in a single TU.




Comment at: clang/include/clang/Sema/Sema.h:335
 
+  /// A key method to reduce duplicate type info from Sema.
+  virtual void anchor();

rnk wrote:
> hans wrote:
> > I worry that this is going to look obscure to most readers passing through. 
> > Maybe it could be expanded to more explicitly spell out that it reduces the 
> > size of the debug info?
> I want to keep it concise, most readers shouldn't need to know what this is, 
> and they can look up technical terms like "key method". I'll say "debug info" 
> instead of "type info", though, that should be more obvious.
FWIW, I just ran into this and did a double/triple take, as it didn't make 
sense for me to see a 'virtual' function in a 'final' type that didn't inherit 
to anything looked like nonsense.

The only way I found out what this meant (googling "key method" did very little 
for me here) was to do a 'git-blame' then found this review.  The ONLY place 
that explained what is happening here is the comment you made here: 
https://reviews.llvm.org/D70340#1752192


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG586f65d31f32: Add a key method to Sema to optimize debug 
info size (authored by rnk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -189,6 +189,9 @@
   SemaPPCallbackHandler->set(*this);
 }
 
+// Anchor Sema's type info to this TU.
+void Sema::anchor() {}
+
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
   DeclarationName DN = &Context.Idents.get(Name);
   if (IdResolver.begin(DN) == IdResolver.end())
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -328,10 +328,13 @@
 };
 
 /// Sema - This implements semantic analysis and AST building for C.
-class Sema {
+class Sema final {
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 
+  /// A key method to reduce duplicate debug info from Sema.
+  virtual void anchor();
+
   ///Source of additional semantic information.
   ExternalSemaSource *ExternalSource;
 


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -189,6 +189,9 @@
   SemaPPCallbackHandler->set(*this);
 }
 
+// Anchor Sema's type info to this TU.
+void Sema::anchor() {}
+
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
   DeclarationName DN = &Context.Idents.get(Name);
   if (IdResolver.begin(DN) == IdResolver.end())
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -328,10 +328,13 @@
 };
 
 /// Sema - This implements semantic analysis and AST building for C.
-class Sema {
+class Sema final {
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 
+  /// A key method to reduce duplicate debug info from Sema.
+  virtual void anchor();
+
   ///Source of additional semantic information.
   ExternalSemaSource *ExternalSource;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Reid Kleckner via Phabricator via cfe-commits]

rnk marked an inline comment as done.
rnk added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:335
 
+  /// A key method to reduce duplicate type info from Sema.
+  virtual void anchor();

hans wrote:
> I worry that this is going to look obscure to most readers passing through. 
> Maybe it could be expanded to more explicitly spell out that it reduces the 
> size of the debug info?
I want to keep it concise, most readers shouldn't need to know what this is, 
and they can look up technical terms like "key method". I'll say "debug info" 
instead of "type info", though, that should be more obvious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Reid Kleckner via Phabricator via cfe-commits]

rnk updated this revision to Diff 230130.
rnk added a comment.

- add final, tweak comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -189,6 +189,9 @@
   SemaPPCallbackHandler->set(*this);
 }
 
+// Anchor Sema's type info to this TU.
+void Sema::anchor() {}
+
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
   DeclarationName DN = &Context.Idents.get(Name);
   if (IdResolver.begin(DN) == IdResolver.end())
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -328,10 +328,13 @@
 };
 
 /// Sema - This implements semantic analysis and AST building for C.
-class Sema {
+class Sema final {
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 
+  /// A key method to reduce duplicate debug info from Sema.
+  virtual void anchor();
+
   ///Source of additional semantic information.
   ExternalSemaSource *ExternalSource;
 


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -189,6 +189,9 @@
   SemaPPCallbackHandler->set(*this);
 }
 
+// Anchor Sema's type info to this TU.
+void Sema::anchor() {}
+
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
   DeclarationName DN = &Context.Idents.get(Name);
   if (IdResolver.begin(DN) == IdResolver.end())
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -328,10 +328,13 @@
 };
 
 /// Sema - This implements semantic analysis and AST building for C.
-class Sema {
+class Sema final {
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 
+  /// A key method to reduce duplicate debug info from Sema.
+  virtual void anchor();
+
   ///Source of additional semantic information.
   ExternalSemaSource *ExternalSource;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Oh, yeah, I forgot this causes tons of -Wdelete-non-virtual-dtor warnings, so 
I'll have to look into that before landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D70340#1751148 , @hans wrote:

> Nice!
>
> Silly questions, but for my own education: I thought the key function concept 
> only existed in the Itanium ABI, but from your numbers it sounds like it's a 
> concept, at least for debug info, also on Windows?


There's sort of two things going on:

- -flimit-debug-info: if a type has a vtable, debug info for the class is only 
emitted where the vtable is emitted, on the assumption that we believe the 
vtable will be in the program somewhere.
- key functions in the ABI: these optimize object file size by avoiding the 
need to emit the vtable in as many places.

The -flimit-debug-info behavior is cross-platform and happens regardless of 
whether the class has a key function. So, clang only emits a forward 
declaration of Foo in the debug info for this program, regardless of target:

  struct Foo {
Foo();
~Foo();
virtual void f() {}
  };
  Foo *makeFoo() { return new Foo(); }

-flimit-debug-info would emit complete type info if the constructor (which 
touches the vtable) was inline.

---

I'll try to land this today, I think it's worth doing. If anyone thinks it's 
too much of a hack, let me know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Nice!

Silly questions, but for my own education: I thought the key function concept 
only existed in the Itanium ABI, but from your numbers it sounds like it's a 
concept, at least for debug info, also on Windows?




Comment at: clang/include/clang/Sema/Sema.h:335
 
+  /// A key method to reduce duplicate type info from Sema.
+  virtual void anchor();

I worry that this is going to look obscure to most readers passing through. 
Maybe it could be expanded to more explicitly spell out that it reduces the 
size of the debug info?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Reid Kleckner via Phabricator via cfe-commits]

rnk updated this revision to Diff 229943.
rnk added a comment.

- comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -189,6 +189,9 @@
   SemaPPCallbackHandler->set(*this);
 }
 
+// Anchor Sema's type info to this TU.
+void Sema::anchor() {}
+
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
   DeclarationName DN = &Context.Idents.get(Name);
   if (IdResolver.begin(DN) == IdResolver.end())
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -332,6 +332,9 @@
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 
+  /// A key method to reduce duplicate type info from Sema.
+  virtual void anchor();
+
   ///Source of additional semantic information.
   ExternalSemaSource *ExternalSource;
 


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -189,6 +189,9 @@
   SemaPPCallbackHandler->set(*this);
 }
 
+// Anchor Sema's type info to this TU.
+void Sema::anchor() {}
+
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
   DeclarationName DN = &Context.Idents.get(Name);
   if (IdResolver.begin(DN) == IdResolver.end())
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -332,6 +332,9 @@
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 
+  /// A key method to reduce duplicate type info from Sema.
+  virtual void anchor();
+
   ///Source of additional semantic information.
   ExternalSemaSource *ExternalSource;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D70340#1749073 , @thakis wrote:

> With the overhead being the cost of a single vtable with one entry? Or is 
> there more?


I guess I worry about the extra dead vtable pointer in Sema. But, I don't think 
it matters. I think we should do this. I'll re-upload with comments and update 
the description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D70340#1748975 , @rnk wrote:

> In D70340#1748712 , @
>
> >
>
>
> I guess I was thinking about enabling this only in +asserts builds, so we pay 
> zero overhead in release builds. I was also thinking that if we do implement 
> the "constructor is key for class debug info" flag in the near term, this 
> becomes obsolete. But it's not that much code churn, and it reduces DWARF 
> size with GCC. I guess we could land it after all. :)


With the overhead being the cost of a single vtable with one entry? Or is there 
more?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D70340#1748712 , @thakis wrote:

> I don't see any reason not to do this. What's there to discuss? I'm probably 
> missing something obvious.


I guess I was thinking about enabling this only in +asserts builds, so we pay 
zero overhead in release builds. I was also thinking that if we do implement 
the "constructor is key for class debug info" flag in the near term, this 
becomes obsolete. But it's not that much code churn, and it reduces DWARF size 
with GCC. I guess we could land it after all. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D70340#1748712 , @thakis wrote:

> I don't see any reason not to do this. What's there to discuss? I'm probably 
> missing something obvious.


Eh, it's a bit quirky - adds production code (albeit a very small amount) only 
to improve debug build properties. I'm not super averse to it - though would 
like @rsmith to weigh in before committing to it.

> dblaikie added anchor functions in many places a while ago (but iirc for 
> vtables, not debug info).

Yeah, that was just following the rules (& a little pedantry/boredom): 
https://llvm.org/docs/CodingStandards.html#provide-a-virtual-method-anchor-for-classes-in-headers
 - it'd be interesting to see how much those are actually worth in object size 
with and without debug info.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

PS: nice find!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Nico Weber via Phabricator via cfe-commits]

thakis accepted this revision.
thakis added a comment.
This revision is now accepted and ready to land.

I don't see any reason not to do this. What's there to discuss? I'm probably 
missing something obvious.

dblaikie added anchor functions in many places a while ago (but iirc for 
vtables, not debug info).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70340



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70340: Add a key method to Sema to optimize debug info size

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added reviewers: dblaikie, hans, thakis, rsmith.
Herald added a subscriber: aprantl.
Herald added a project: clang.

DONOTSUBMIT, this patch is just for the purpose of discussion.

It turns out that the debug info describing the Sema class is an
appreciable percentage of the total object file size of objects in Sema.
By adding a key function, clang is able to optimize the debug info size
by emitting a forward declaration in TUs that do not define the key
function.

On Windows, with clang-cl, these are the total object file sizes before
and after this change when compiling with optimizations and debug info:

  before: 335,012 KB
  after:  278,116 KB
  delta:  -56,896 KB
  percent: -17.0%

The effect on link time was negligible, despite having ~56MB less input.

On Linux, with clang, these are the same sizes using DWARF -g and
optimizations:

  before: 603,756 KB
  after:  515,340 KB
  delta:  -88,416 KB
  percent: -14.6%

I didn't use type units, DWARF-5, fission, or any other special flags.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70340

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -189,6 +189,9 @@
   SemaPPCallbackHandler->set(*this);
 }
 
+// Anchor Sema's type info to this TU.
+void Sema::anchor() {}
+
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
   DeclarationName DN = &Context.Idents.get(Name);
   if (IdResolver.begin(DN) == IdResolver.end())
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -329,6 +329,7 @@
 
 /// Sema - This implements semantic analysis and AST building for C.
 class Sema {
+  virtual void anchor();
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -189,6 +189,9 @@
   SemaPPCallbackHandler->set(*this);
 }
 
+// Anchor Sema's type info to this TU.
+void Sema::anchor() {}
+
 void Sema::addImplicitTypedef(StringRef Name, QualType T) {
   DeclarationName DN = &Context.Idents.get(Name);
   if (IdResolver.begin(DN) == IdResolver.end())
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -329,6 +329,7 @@
 
 /// Sema - This implements semantic analysis and AST building for C.
 class Sema {
+  virtual void anchor();
   Sema(const Sema &) = delete;
   void operator=(const Sema &) = delete;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits