zahiraam added inline comments.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4852
+ if (!CD->isTrivial() && !D->getTLSKind())
+ NeedsGlobalCtor = true;
+ }
----------------
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?
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
