[llvm] r209015 - Add comdat key field to llvm.global_ctors and llvm.global_dtors

[Reid Kleckner]

On Wed, Jun 4, 2014 at 2:55 PM, Rafael Espíndola <rafael.espindola at gmail.com
> wrote:
>
> How would this work in ELF? In ELF all the pointer are in a single
> section, .init_array, so they cannot be in multiple comdats.


I think it's OK to have multiple sections with the same name.  Won't the
linker simply concatenate the multiple .init_array's during the link?

> While the code as is doesn't put the initializer function into a comdat,
> why
> > do you think we can't do this?  It seems like it'd be a nice code size
> > optimization.  Right now the initializer function has internal linkage
> > because there is no mangling for it in Itanium, and we end up with lots
> of
> > unmerged duplicate code.
> >
> > My plan for the future is to put both the initializer function and the
> guard
> > variable into the comdat group of the data being initialized using
> section
> > IR.  Then either GlobalOpt or Clang can replace the global with a
> constant,
> > so long as the initializer of the guard variable is updated.
>
> If the function currently is not in a comdat, it should be safe.
>
> Now that I think of it, what will not work is putting the two
> variables in a comdat with the same name as the variable (as suggested
> in the ABI). It would break linking with old objects. For example, a
> new .o file would look like
>
> ----------------------------------------------------------------
> .section .var, "G", @nobits, comdat_sym,comdat
> var:
>         .long 42
> .section .guard, "G", @nobits, comdat_sym,comdat
> guard:
>         .long 42
>
> .data
>         .quad guard
> ----------------------------------------------------------------
>
> and an old one looks like
>
> ----------------------------------------------------------------
> .section .var, "G", @nobits, comdat_sym,comdat
> var:
>         .long 42
>
> .section .guard, "G", @nobits, comdat_sym2,comdat
> guard:
>         .long 42
> ------------------------------------------------------------------
>
> Depending on on the link order we get an warning and invalid output
> from the linker:
>
> warning: relocation refers to discarded section
>
> What is needed is a new comdat symbol name for both variable and
> guard, something like
>
>
> ----------------------------------------------------------------------------
> .section .var, "G", @nobits, new_comdat_sym,comdat
> var:
>         .long 42
> .section .guard, "G", @nobits, new_comdat_sym,comdat
> guard:
>         .long 42
> .data
>         .quad guard
> -----------------------------------------------------------------------
>
> This is equivalent to using a *C5* comdat symbol to hold the *C1* and
> *C2* constructors.
>
> Should we report  a bug on the itanium abi?
>

Yes.  Also, this completely breaks the intended design of the comdat key in
global ctors, because now it needs to support an arbitrary symbol and not a
specific GV.  Maybe we can find a way to replace this with an explicit
section which will have the comdat, but I'll have to think of that later.


> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140604/fa1d060c/attachment.html>