ychen added inline comments.
================
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:580
+ I = DelayedCXXInitPosition.find(D);
+ unsigned LexOrder = I == DelayedCXXInitPosition.end() ? ~0U : I->second;
+ AddGlobalCtor(Fn, 65535, LexOrder, COMDATKey);
----------------
efriedma wrote:
> This ensures delayed initialization calls are ordered relative to each
> other... but are they ordered correctly relative to non-delayed
> initialization calls? I'm skeptical that using a LexOrder of "~0U" is really
> correct. (For example, if you change the variable "b" in your testcase to a
> struct with a destructor.)
Hmm, That's a great point. I didn't think of that. I'll take a look.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:544
}
+ llvm::stable_sort(GlobalCtors, [](const Structor &L, const Structor &R) {
+ return L.LexOrder < R.LexOrder;
----------------
efriedma wrote:
> ychen wrote:
> > rnk wrote:
> > > Please move this sorting into EmitCtorList and apply it to destructors. I
> > > believe they are currently emitted in source order, and the loader
> > > executes them in reverse order, so we get the desired reverse source
> > > order cleanup behavior.
> > I looked into this. They are indeed "currently emitted in source order".
> > However, if a dtor ever uses an entry in `llvm.global_dtors`, it must have
> > bailed out of deferred emission here
> > (https://github.com/llvm/llvm-project/blob/69c09d11f877a35655e285cda96ec0699e385fc9/clang/lib/CodeGen/CodeGenModule.cpp#L3256-L3263),
> > which is to say, the corresponding ctor is also in lexing order. So the
> > situation is either the ctor/dtor pair both use lexing order, or there is
> > no dtor (like `inline int` with an initializer) and the ctor is
> > deferred/reordered where this patch kicks in.
> >
> Is MayBeEmittedEagerly always true for variables with destructors?
Yeah, for this and `CodeGenModule::MustBeEmitted`, as long as the destructors
are not constexpr where init order is not an issue.
https://github.com/llvm/llvm-project/blob/aacf1a9742f714dd432117d82d19a007289c3dee/clang/lib/AST/ASTContext.cpp#L11646-L11648
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127233/new/
https://reviews.llvm.org/D127233
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
