[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

Zahira Ammarguellat via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 9 11:11:53 PST 2022


zahiraam 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,
----------------
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) 
  }


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137107/new/

https://reviews.llvm.org/D137107



More information about the cfe-commits mailing list