rsmith marked an inline comment as done.
rsmith added inline comments.
================
Comment at: lib/CodeGen/CodeGenModule.cpp:3991-3992
+ if (auto *MD = dyn_cast<CXXMethodDecl>(D)) {
+ // FIXME: There's no reason to do this if the key function is inline.
+ // Formally, the ABI requires it, but the difference is not observable.
+ if (declaresSameEntity(Context.getCurrentKeyFunction(MD->getParent()), MD))
----------------
rjmccall wrote:
> rsmith wrote:
> > @rjmccall Is there any reason we (from the CodeGen perspective) should
> > treat an inline key function as emitting the vtable? I can't think of any
> > reason to do so -- it's not in a comdat with the vtable or anything like
> > that, so every translation unit that emits a reference to the vtable should
> > emit its own copy anyway.
> The thinking was as follows: translation units that can't see a definition of
> the key function don't know that the definition is actually inline, so
> they'll emit a reference to an external v-table definition, which will lead
> to link failures if the translation units that do contain the inline
> definition don't eagerly emit the v-table. However, ARM pointed out years
> ago that the ODR requires inline definitions of virtual functions to be
> present in every translation unit which declares the virtual function at all,
> so there's no legal situation where a translation unit can't see the
> definition of an inline key function. Furthermore, I believe Clang has never
> made v-tables undiscardable in translation units with inline key function
> definitions, so there's no real guarantee that ODR-violating code will
> actually link, although you can certainly imagine ways in which an
> ODR-violating program might get by without such a guarantee.
>
> Personally, I think the strongest argument for "deviating" here is that the
> language standard takes priority over the ABI, which means we're allowed to
> assume the program is overall well-formed, i.e. we're only required to
> interoperate with legal code. Now, that's a line of reasoning which leads us
> into some grey areas of implementation-definedness, but I feel fairly
> comfortable about deviating in this particular instance because I don't know
> why someone would really *want* to take advantage of v-tables being emitted
> eagerly; in general, eager emission of inline code is a bad thing that
> significantly slows down builds.
>
> Now, ARM used this property of inline definitions to change the key function
> to the first non-inline function, and unfortunately we can't do that on
> existing targets without breaking interoperation. (We did do it on some
> newer Darwin targets, though, and we haven't had any problem with it.) But I
> do think we could use this property of inline definitions to just treat the
> class as no longer having a key function when we see an inline definition of
> it. That would rid us of this particular scourge of eager emission of inline
> code.
My thinking is this: if a vtable has discardable linkage, then it can be
discarded when optimizing if it's not referenced. So there's no point emitting
it unless we also emit a reference to it. So we should only emit vtables with
discardable linkage if they're actually referenced, just like we do for other
symbols with discardable linkage. This is, I think, stronger than what you're
suggesting, because it affects internal-linkage explicit instantiations too.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54986/new/
https://reviews.llvm.org/D54986
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
