efriedma added inline comments.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4852
+ if (!CD->isTrivial() && !D->getTLSKind())
+ NeedsGlobalCtor = true;
+ }
----------------
zahiraam wrote:
> efriedma wrote:
> > zahiraam wrote:
> > > efriedma wrote:
> > > > I have no idea what this code is supposed to do.
> > > I have removed from this the thread_local variables for being processed
> > > the same way. Maybe they should be included, not sure?
> > >
> > > For the rest is to differentiate between these cases. I found the 2nd
> > > test case while doing some unit testing.
> > >
> > > struct B {
> > > constexpr B() {}
> > > ~B() {};
> > > };
> > > constinit B b;
> > >
> > >
> > > and
> > >
> > > struct A {
> > > int s;
> > > ~A();
> > > };
> > >
> > > struct D : A {};
> > > D d1 = {};
> > >
> > > I think the second test case is not supposed to EmitDeclInit in
> > > EmitCXXGlobalVarDeclInit right? But since now tryEmitForInitializer is
> > > returning an Initializer, then NeedsGlobalCtor = true;
> > >
> > > Actually, when I removed this code, I have 2 LIT tests failing with the
> > > same crash.
> > > WDYT?
> > >
> > >
> > I'm not sure how thread-local var init works on Windows off the top of my
> > head; I'd need to check.
> >
> > For the given testcases, neither one of those cases should be calling
> > EmitDeclInit; we use constant initialization for both cases, so the global
> > init func should just be registering the destructors. We shouldn't need to
> > examine the initializer beyond checking whether tryEmitForInitializer
> > succeeds.
> >
> > Not sure what's crashing from your description.
> Removing this code will still result, in both cases to the call to
> EmitDeclInit, because the first call to EmitCXXCtorInit is called with
> PerformInit true.
>
> Calling EmitCXXCtorInit with false would result into a ctor function with an
> empty body for the 1st test case (struct B):
> define internal void @ctor() #0 {
> entry:
> ret void
> }
>
> I would have thought that we should have something like this:
>
> define internal void @ctor() #0 {
> entry:
> %call = call x86_thiscallcc ptr @"??0B@@QAE@XZ"(ptr nonnull align 1
> dereferenceable(1) @"?b@@3UB@@A")
> ret void
> }
>
> No?
> because the first call to EmitCXXCtorInit is called with PerformInit true.
That sounds wrong; if tryEmitForInitializer succeeds, we shouldn't perform the
init at all.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5009
+ if (isStaticInit(D, getLangOpts()) && NeedsGlobalCtor && NeedsGlobalDtor) {
+ EmitCXXCtorInit(D, GV, true, 201, llvm::StringLiteral("ctor"), false);
+ EmitCXXCtorInit(D, GV, false, 65535, llvm::StringLiteral("dtor"), true);
----------------
zahiraam wrote:
> efriedma wrote:
> > I think you want to use priority 201 whether or not there's a destructor.
> Is that what you mean?
I think it should look something more like this:
```
if (isStaticInit(D, getLangOpts()) {
if (NeedsGlobalCtor)
EmitCXXCtorInit(D, GV, true, 201, llvm::StringLiteral("ctor"), false);
if (NeedsGlobalDtor)
EmitCXXCtorInit(D, GV, false, 65535, llvm::StringLiteral("dtor"), true);
DelayedCXXInitPosition[D] = ~0U;
} else {
EmitCXXGlobalVarDeclInitFunc(D, GV, NeedsGlobalCtor);
}
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137107/new/
https://reviews.llvm.org/D137107
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
