[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

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:

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

efriedma 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); ychen wrote:

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

ychen updated this revision to Diff 453095. ychen added a comment. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127233/new/ https://reviews.llvm.org/D127233 Files: clang/lib/CodeGen/CGDeclCXX.cpp clang/lib/CodeGen/CodeGenModule.cpp

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

ychen updated this revision to Diff 453094. ychen added a comment. - use correct lexing order for non-deferred constructors. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127233/new/ https://reviews.llvm.org/D127233 Files: clang/lib/CodeGen/CGDe

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

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:

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

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

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

aaron.ballman added a comment. In D127233#3726613 , @rjmccall wrote: > There's no compiler interoperation problem here; it's just a semantic concern > that someone could've been relying on the old behavior. The new behavior is > (AIUI) clearly required

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

rjmccall added a comment. There's no compiler interoperation problem here; it's just a semantic concern that someone could've been relying on the old behavior. The new behavior is (AIUI) clearly required by the language standard, and I don't think we want to get into the business of providing

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

ychen added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:544 } + llvm::stable_sort(GlobalCtors, [](const Structor &L, const Structor &R) { +return L.LexOrder < R.LexOrder; rnk wrote: > Please move this sorting into EmitCtorList and app

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

aaron.ballman added a comment. This looks correct per my reading of [basic.start.dynamic], but is this an ABI breaking change that we may want to use ABI versioning for in case someone is relying on the old order for some reason? Also, the change should have a release note for the fix. Reposi

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

rnk added subscribers: ayzhao, usaxena95, ilya-biryukov. rnk added a comment. I think this is a straightforward improvement. I would like to see it land. Do the other reviewers have any outstanding concerns? +cc other clang people @ayzhao @ilya-biryukov @usaxena95 Comment at:

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

ychen added a comment. ping 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-b

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

ychen added a comment. ping. 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-

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

ychen added a comment. gentle ping.. 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.