efriedma added inline comments.
================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:572
PrioritizedCXXGlobalInits.push_back(std::make_pair(Key, Fn));
+ } else if (D->hasConstantInitialization() && !(D->hasAttr<ConstInitAttr>()))
{
+ OrderGlobalInitsOrStermFinalizers Key(201,
----------------
zahiraam wrote:
> efriedma wrote:
> > zahiraam wrote:
> > > efriedma wrote:
> > > > zahiraam wrote:
> > > > > efriedma wrote:
> > > > > > zahiraam wrote:
> > > > > > > efriedma wrote:
> > > > > > > > zahiraam wrote:
> > > > > > > > > efriedma wrote:
> > > > > > > > > > How is ConstInitAttr relevant here?
> > > > > > > > > This change made (without the !(D->hasAttr<ConstInitAttr>())
> > > > > > > > > made the LIT behavior of aix-static-init.cpp. The IR
> > > > > > > > > generated for
> > > > > > > > > namespace test3 {
> > > > > > > > > struct Test3 {
> > > > > > > > > constexpr Test3() {};
> > > > > > > > > ~Test3() {};
> > > > > > > > > };
> > > > > > > > >
> > > > > > > > > constinit Test3 t;
> > > > > > > > > } // namespace test3
> > > > > > > > >
> > > > > > > > > was different. I would have thought that the change we made
> > > > > > > > > for constexpr wouldn't affter constinit?
> > > > > > > > I think the significant bit there isn't the use of constinit;
> > > > > > > > it's the non-trivial destructor. I think the priority
> > > > > > > > modification should only affect constructors, not destructors.
> > > > > > > > (Not sure how to make that work, at first glance.)
> > > > > > > Let's see if this is an acceptable solution.
> > > > > > To fake constant initialization, we need to initialize the variable
> > > > > > before anything else runs. But the rearranged prioritization isn't
> > > > > > supposed to affect the destructor. From [basic.start.term]: "If an
> > > > > > object is initialized statically, the object is destroyed in the
> > > > > > same order as if the object was dynamically initialized."
> > > > > >
> > > > > > What you're doing here isn't exactly implementing that. What
> > > > > > you're doing here is delaying both the initialization and the
> > > > > > destruction if the variable has a non-trivial destructor. We need
> > > > > > to separate the two to get the behavior we want.
> > > > > Could we consider adding a field to EvaluatedStmt called
> > > > > "HasTrivialDestrutor" and only perform the prioritization change when
> > > > > !D->HasTrivialDesctructor? Instead of using the test for
> > > > > D->hasConstantInitialization(). This seems to be englobing to many
> > > > > cases.
> > > > >
> > > > > I considered returning null for HasConstantInitialization in case of
> > > > > var has a non-trivial destructor but that doesn't seem to work.
> > > > Once you separate out the initialization from the destruction, the rest
> > > > should come together naturally, I think? I'm not sure what other cases
> > > > could be caught by hasConstantInitialization().
> > > Does this change accomplish this? Thanks.
> > When a global variable is dynamically initialized, there are two parts to
> > the initialization:
> >
> > - Constructing the variable (calling the constructor, or equivalent)
> > - Registering the destructor with the runtime (atexit on Windows)
> >
> > If an object is initialized statically, the two parts are separated: the
> > variable is emitted already initialized by the compiler. But we still
> > register the destructor at the same time, in the same way.
> >
> > If a dllimport object is initialized statically, we need to make it appear
> > to user code that the same thing happened. So we need to initialize the
> > object early, but we need to register the destructor at the same time we
> > would have otherwise registered it. To make this work, we need two
> > separate initializer functions with different priorities.
> Thanks for the clarification. I am slowly getting it, I think.
>
> In terms of IR, for example for this:
> struct Test3 {
> constexpr Test3() {};
> ~Test3() {};
> };
>
> constinit Test3 t;
>
> Are we looking for something like this (partial IR):
>
> @t = global %struct.Test3 undef
> @llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr,
> ptr } { i32 **201**, ptr @_GLOBAL__I_000201, ptr null }]
> @llvm.global_dtors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr,
> ptr } { i32 **65535**, ptr @_GLOBAL__D_a, ptr null }]
> define internal void @__cxx_global_var_init() {
> entry:
> call void @_ZN5Test3C1Ev(ptr noundef nonnull align 1 dereferenceable(1)
> @t)
> %0 = call i32 @atexit(ptr @__dtor_t)
> ret void
> }
> define internal void @__finalize_t() {
> %0 = call i32 @unatexit(ptr @__dtor_t)
> %needs_destruct = icmp eq i32 %0, 0
> br i1 %needs_destruct, label %destruct.call, label %destruct.end
>
> destruct.call:
> call void @__dtor_t()
> .....}
> }
>
> Or
>
> define internal void @__cxx_global_var_init() {
> entry:
> call void @_ZN5Test3C1Ev(ptr noundef nonnull align 1 dereferenceable(1)
> @t)
> ret void
> }
> define internal void @__finalize_t() {
> %0 = call i32 @atexit(ptr @__dtor_t)
> }
global_dtors is not at all relevant. (It's not something we ever use for C++
globals, and it doesn't have the right semantics for that anyway, and I don't
think we even support it on Windows.) The sequence we currently generate looks
something like this:
```
@llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr
} { i32 65535, ptr @_GLOBAL__sub_I__, ptr null }]
define internal void @_GLOBAL__sub_I__() #1 {
entry:
store ptr @"?x@@3HA", ptr @"?y@@3UX@@A", align 8, !tbaa !4
%0 = tail call i32 @atexit(ptr nonnull @"??__Fy@@YAXXZ") #2
ret void
}
```
What we want is something like this:
```
@llvm.global_ctors = appending global [2 x { i32, ptr, ptr }] [{ i32, ptr, ptr
} { i32 201, ptr @ctor, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @dtor,
ptr null }]
define internal void @ctor() {
entry:
store ptr @"?x@@3HA", ptr @"?y@@3UX@@A", align 8, !tbaa !4
ret void
}
define internal void @dtor() {
entry:
%0 = tail call i32 @atexit(ptr nonnull @"??__Fy@@YAXXZ") #2
ret void
}
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137107/new/
https://reviews.llvm.org/D137107
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
