[cfe-dev] [LLVMdev] weak_odr constant versus weak_odr global

[Reid Kleckner]

On Thu, Sep 4, 2014 at 7:48 PM, Rafael Espíndola <rafael.espindola at gmail.com
> wrote:

> > I see. Using two comdats would still cause the same problem for us,
> > no? So the solution in the end is to emit:
> >
> > TU1:
> > --------------------------------
> > @_ZN1UI1SE1kE = weak_odr constant i32 42, align 4, comdat _ZN1UI1SE1kE
> > @_ZGVN1UI1SE1kE = weak_odr global i64 1, comdat _ZN1UI1SE1kE
> > --------------------------------
> >
> > TU2:
> > -----------------------------------
> > @_ZN1UI1SE1kE = weak_odr global i32 0, align 4, comdat _ZN1UI1SE1kE
> > @_ZGVN1UI1SE1kE = weak_odr global i64 0, comdat _ZN1UI1SE1kE
> > ...
> > @llvm.global_ctors = ....
> > define internal void @_GLOBAL__I_a() nounwind section ".text.startup"
> ....
> > -----------------------------------
>
> Restarting a really old thread now that we have comdat support in the IR.
>
> While the above idea would work, there are two problems with it
>
> * Existing compilers (clang and gcc) produce a comdat with just the
> constant in TU1. Linking one of those with TU2 can still cause a crash
> since the guard variable would be undefined.
> * It requires always outputting the guard variable.
>
> Since neither gcc nor clang implement this part of the ABI, I was
> thinking if there was a better way to do it. One interesting option is
> putting the .init_array of TU2 in the comdat. That is exactly what we
> do for windows. In fact, just passing -Xclang -mllvm -Xclang
> -enable-structor-comdat will avoids the crash in the above example.
>
> Given that this has been broken since forever, waiting a bit more for
> https://sourceware.org/bugzilla/show_bug.cgi?id=17350 to be fixed and
> then flipping -enable-structor-comdat might be the best way to fix
> this. With that done we can add the function and the guard variable in
> TU2 to the comdat to remove a bit of bloat (see attached patch).
>

Even if this doesn't address the constant vs global crash issues, I think
adding the .init_array entry to the comdat is a great startup time
optimization. Everything continues to work as it does today, but less code
is run during dynamic initialization because all the new TU initializers
get deduplicated by the linker.

It sounds like you are proposing to emit this sort of IR, after the linker
bug is fixed:

// some header:
int f();
template <typename T> struct A { static int var; };
template <typename T> int A<T>::var = f();
template struct A<int>;

New TU1:
-------------
$var = comdat
@var = weak_odr global i32 0, comdat $var
@guard = weak_odr global i32 0, comdat $var
define weak_odr void @initializer() comdat $var {
  ; check @guard
  store i32 42, i32* @var
}
@llvm.global_ctors = [ { i32, void ()*, i8* } x 1 ] [ { i32 65535, void ()*
@initializer, i8* bitcast (i32* @var to i8*) } ]

Old TU2:
--------------
@var = weak_odr global i32 0
@guard = weak_odr global i32 0
define weak_odr void @initializer() {
  ; check @guard
  store i32 42, i32* @var
}
@llvm.global_ctors = [ { i32, void ()*, i8* } x 1 ] [ { i32 65535, void ()*
@initializer, i8* null } ]

It sounds like we still can't make @var into a constant if we want ABI
compatibility with old object files, though.

However, if we wait until we don't care about old object file
compatibility, we can transition to linking new TU1 with this new TU2:
-------
$var = comdat
@var = weak_odr constant i32 42, comdat $var
; no guard, no initializer, no global_ctors entry

Is that what you're thinking?

What is the discussion list for the itanium abi? Should I propose a patch?
>

cxx-abi-dev at codesourcery.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140905/8a01b9e3/attachment.html>