[LLVMdev] anchoring explicit template instantiations

[David Blaikie]

Mix & matching the various replies to perhaps form a coherent discussion

>>> David:
>>> (also, I implemented this by adding private anchors, like my original
>>> version - this does actually have a difference at runtime, of course -
>>> since now each of these types has another entry in their vtable.
>>
>> Chris:
>> I think that's a perfectly fine cost :)
>
> Ted:
> Why is this acceptable?
>
> Jakob:
> I am skeptical too. David just added 17 translation units with the purpose of speeding up the build.
> Did it work? Numbers, please.

I mentioned similar concerns when I sent this for review (to quote the
full text):

"Actually it'd be kind of interesting to see how much/if any difference
in compile time this actually makes. I do wonder.

(also, I implemented this by adding private anchors, like my original
version - this does actually have a difference at runtime, of course -
since now each of these types has another entry in their vtable.
Alternatively I could've used their destructors (but then they
wouldn't be inline anymore - but probably most of them are called
virtually anyway so their inline-ness doesn't matter). Also I had to
add about 10 source files in total as implementation files for
header-only cases that needed anchors)"

Though, admittedly, I didn't run the numbers myself. At best, I expect
this will reduce link times which, at least for me, tend to dominate
many of my incremental builds (unless I go touching Parser.h or Sema.h
in clang, for example - though the long link times have been pointed
out to perhaps be a particular issue with linux builds, even when
using gold (though gold does help matters somewhat)) while coming at
(hopefully a small) cost for full rebuilds.

But yes, real numbers would help.

> Ted:
> Can't we just move more virtual methods out-of-line?

I considered that - Chris seemed to think it wasn't the way to go.

>>> David:
>>> Alternatively I could've used their destructors (but then they
>>> wouldn't be inline anymore - but probably most of them are called
>>> virtually anyway so their inline-ness doesn't matter). Also I had to
>>> add about 10 source files in total as implementation files for
>>> header-only cases that needed anchors)
>>
>> Chris:
>> Losing an inlined dtor would be a bigger cost.

That being said - I wonder how much LTO removes the need to make
functions inline anyway. Any thoughts? Should we get faster builds by
making everything non-inline (at a runtime cost, of course) if LTO
could compensate for it? Honestly I'd prefer that - it'd reduce
incremental build times on average (less chance your change is in a
header included all over the place, etc).

Honestly the inlined dtor case seems sort of unimportant, as I
mentioned - if the dtors are really called virtually anyway, though
some of those cases may be able to be devirtualized I (dangerous,
second-guessing the compiler, but that's what this whole thread is
about I Suppose) don't think many of them would, in reality.

Uninlining virtual dtors for types that aren't actually destroyed
polymorphically (these have been added in many cases to silence GCC's
simplistic "either have virtual dtors or protected non-virtual dtors
on any type with virtual functions") would be a bit of a loss, since
those ones really should be easily devirtualized (I suppose they would
still be devirtualized, but not inlined without LTO - which isn't such
a problem). Sadly we don't have any annotation/comments/etc to look
for to find these faux virtual dtors.

> Ted:
> If we had a warning to find which classes we need to fix, why not just enforce that warning going forward as part of building the codebase once all of the offending classes are fixed?

I wondered the same thing (though there are a few other cases to be
considered - the patches I submitted /almost/ make us -Wweak-vtable
clean, llvm-tblgen's generated code and gtest are the only remaining
culprits):

>> David:
>> Also - do we have anywhere we could put standard flags that LLVM
>> should successfully compile with? At least if clang is being used,
>> perhaps? So we could add -Wweak-vtables -Werror for this case & add
>> more as we deem it appropriate.
>
> Chris:
> Makefile.rules is the right place, but we don't have great autoconf logic to detect what flags the host compiler supports (at least as far as I know).

So any further ideas on how to add standard warnings would be great.

All that being said - I'm in Hawaii for the next 3 weeks or so with
limited access to contribute, so if someone feels strongly enough to
back this out or take it in a different direction, please don't
hesitate to do so & I can rework it/decide what to do with it when I
get back.

Apologies for the surprise though it had been on the list for over a
week now (I mean this in jest, but it seems Grace Hopper was right:
"It's easier to ask for forgiveness than it is to get permission" when
it comes to code reviews (I don't actually mind the delay all that
much, really - I just find other things to patch, evidently))

- David