[Patch] Use comdats for linkonce_odr constructors/destructors

[Richard Smith]

On Thu, Oct 9, 2014 at 6:44 AM, Rafael Espíndola <rafael.espindola at gmail.com
> wrote:

> The attached patch makes clang use comdats for linkonce_odr structors.
> This matches the gcc behavior and there are two potential advantages
> that I can think off
>
> * Avoid a bit of code bloat when mixing gcc and clang compiled code
> since they will have the same COMDATs.
>

Can you say a bit more about the cases where this reduces code bloat?
Either way, we'll end up with exactly one definition of each Dn variant
that is used, won't we?

* Avoid pain to users that developed a dependency on the gcc behavior,
> but it might be a bit late for that
> (http://llvm.org/devmtg/2014-04/PDFs/Talks/NickRefactoring.pdf).
>

I don't think making broken code work better has a lot of value; I think
it's better to focus our efforts on making it easier to detect that code is
broken.

I was hoping this wouldn't have a noticeable code quality impact, but
> I have seen at least two issues
>
> * Putting D0 in the D5 comdat is an ABI bug IMHO since it is never
> equal to D1 or D2 and doing so with linkonce_odr can bloat the code a
> bit by causing us to keep more functions then we need (keep D1/D2 if
> they get inlined into D0 for example).
>

It will cause us to emit more IR, but I don't see that it will cause us to
keep more functions; the D0 will still be discardable if it ends up unused,
right? (It looks like GNU ld's --gc-sections will keep sections that are
within a used COMDAT, but gold's --gc-sections will remove them.)

* Using a COMDAT for the destructors of a class bar prevents D2 from
> being replaced with D2 of the base class. That means we end up with
>
> define linkonce_odr void @_ZN3fooIiED2Ev(%struct.foo* %this)
> unnamed_addr #1 comdat $_ZN3fooIiED5Ev align 2 {
> entry:
>   %0 = bitcast %struct.foo* %this to %struct.bar*
>   tail call void @_ZN3barD2Ev(%struct.bar* %0)
>   ret void
> }
>
> Instead of rauw of _ZN3fooIiED2Ev with _ZN3barD2Ev. We could teach
> LLVM to do this, but we are not there yet.
>

I think you can still RAUW; you just need to leave the derived dtor behind
if you leave any part of its comdat behind.

So it is not clear if this is worth it having it upstream. What do you
> guys think?


Not sure yet.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141017/942529c3/attachment.html>