efriedma added a comment.

> is this an ABI breaking change

It only affects the order of initialization within a file, so it doesn't really 
have any implications for the binary ABI.  It might break code, of course, but 
that's a different issue.

If we want a flag to maintain the old behavior, we need to define what the 
"old" order is.  Not sure that's worthwhile.



================
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);
----------------
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.)


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:544
   }
+  llvm::stable_sort(GlobalCtors, [](const Structor &L, const Structor &R) {
+    return L.LexOrder < R.LexOrder;
----------------
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?


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

Reply via email to