rsmith added a comment.
If CodeGen is able to emit inline key functions when the vtable is needed
anyway, can we instead change `DeclMustBeEmitted` to not treat inline key
functions specially? There is no reason we need to emit any symbols for:
struct A { virtual void f(); };
inline void A::f() {}
... and if suppressing `DeclMustBeEmitted` works in the modules case, it really
should work in the general case.
Indeed, deleting the relevant code from `DeclMustBeEmitted` appears to work
fine. It's not actually enough to prevent us from spuriously emitting the
vtable and inline function definition in the above case, though: `Sema` marks
the vtable as "used" because the key function was defined (it doesn't know
whether CodeGen might decide to emit it and prepares for the worst, but that
then marks the vtable as "used", which *forces* it to be emitted). Fixing that
requires some more invasive changes.
See https://reviews.llvm.org/D54986 for my proposed alternative to this. The
idea is to allow CodeGen to decide when to emit a vtable, and to do so only
when we actually emit a use of the vtable / key function / explicit
instantiation definition of the class. This doesn't require any
eagerly-deserialized declaration tracking except for explicit instantiation
definitions.
================
Comment at: lib/Serialization/ASTWriterDecl.cpp:2267-2278
+ // If the ABI supports inline key functions we must emit them eagerly. The
+ // generic reason is that CodeGen might assume they are emitted and generate
a
+ // reference to the vtable. In deserialization this rule becomes a little
less
+ // clear because if a reference is generated, CodeGen will make a request and
+ // we will emit the vtable on demand.
+ //
+ // This change in behavior is driven mostly by performance considerations and
----------------
If the function must be emitted for some reason other than being a key function
(for instance, due to `__attribute__((used))` or simply not being declared
inline), this may result in it not getting emitted; that doesn't seem correct.
(For a module it might typically work because most such cases would be
redefinition errors, but it's wrong for cases like a precompiled preamble or
PCH, which can be intended to be used only once and thus can contain strong
external symbol definitions.)
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53925/new/
https://reviews.llvm.org/D53925
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
