[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission
Yuanfang Chen via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 16 13:16:43 PDT 2022
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:
> ychen wrote:
> > 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.
> Using `CXXGlobalInits.size()` like this is sort of hard to reason about: multiple values can end up with the same LexOrder. (I'm not sure if all the values with the same LexOrder end up sorted correctly.)
>
> Instead of trying to reason about whether this is right, can we just use a separate counter to assign a unique index to each global init?
Agreed that this whole thing is confusing if not looked at closely. IIUC, LexOrder is unique for each global variables and all global variables with an initialization are pushed into `CXXGlobalInits` in lexing order regardless of deferred/non-deferred emission. I could add a counter which basically tracks the size of `CXXGlobalInits`. Which do you think is more clear: add an explicit counter or add more comments to `CXXGlobalInits` explaining the usage?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127233/new/
https://reviews.llvm.org/D127233
More information about the cfe-commits
mailing list