[cfe-commits] [patch][pr13124] Use strong symbols for functions that go in a strong vtables

[Rafael Espíndola]

> It's not a rejection, it's a lack of coalescing.  In particular, I know that
> the Darwin dyld does not coalesce strong symbols with weak ones.

OK. I agree that strong symbols are out of question then (for darwin
at least). Not all functions in a vtable are unnamed_addr and we would
break pointer comparison when using and old and a new DSO.

> Weak linkage across shared-library boundaries is a serious launch-time
> performance problem, because every symbol that's weakly exported from
> a dynamic object has to be checked for potential coalescing with symbols
> from every other dynamic object.  This is why the Darwin dynamic linker
> does not coalesce strong symbols with weak ones, and this is why we would
> like to not have to export function symbols at all in the common case that
> they're only called (or placed in vtables) within a dynamic library.

Yes, trying to implement this is exactly how I found this bug :-(

But if the functions are emitted with weak_odr only in the TU where
the vtable is, there will be only one DSO where they are defined and
the coalescing will be trivial (ignoring the transition period where
we have DSO built with older compilers, but it would still not be any
worse than what we have now).

> Devirtualization, on the other hand, is a fairly minor optimization when
> it does not create inlining opportunities.

Sure, and we can inline with available_externally or linkonce_odr
functions, so the optimization opportunities we have are orthogonal to
allowing undefined references in cases like this one.

>> Given that both clang and gcc (both 4.6 and 4.7) produce an undefined
>> reference to _ZN3barD0Ev in
>
> So a recent optimization has a bug, and it's the same bug in two
> different compilers for basically the same reason.  This is not surprising.
>

...

> I agree that we should coordinate with GCC to make sure that both
> compilers fix this bug.

I will wait a bit to see if you or someone else has an opinion about
the above comment that we can put the weak_odr function in a single
DSO. Depending on the result I will then start a new thread with the
gcc list tomorrow.

> John.

Cheers,
Rafael