[PATCH] D71572: [ItaniumCXXABI] Make tls wrappers comdat on Windows

rsmith added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2551 + // actually handle multiple TUs defining the same wrapper. + if (CGM.getTriple().isOSWindows() && CGM.supportsCOMDAT() && + Wrapper->isWeakForLinker()) mstorsjo wrote: >

[PATCH] D71572: [ItaniumCXXABI] Make tls wrappers comdat on Windows

mstorsjo added a comment. Ping @rsmith CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71572/new/ https://reviews.llvm.org/D71572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D71572: [ItaniumCXXABI] Make tls wrappers comdat on Windows

mstorsjo added a comment. In D71572#1788786 , @rnk wrote: > Looks like @rsmith did this here: > > https://github.com/llvm/llvm-project/commit/fbe2369f1a514423e4c25417ab3532502fde6f2a > > I see that it was replacing a CHECK for a specific comdat group

[PATCH] D71572: [ItaniumCXXABI] Make tls wrappers comdat on Windows

rnk added a comment. Looks like @rsmith did this here: https://github.com/llvm/llvm-project/commit/fbe2369f1a514423e4c25417ab3532502fde6f2a I see that it was replacing a CHECK for a specific comdat group with no comdat at all. I *think* that's not correct, we should be checking for the trivial

[PATCH] D71572: [ItaniumCXXABI] Make tls wrappers comdat on Windows

mstorsjo marked an inline comment as done. mstorsjo added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2551 + // actually handle multiple TUs defining the same wrapper. + if (CGM.getTriple().isOSWindows() && CGM.supportsCOMDAT() && +

[PATCH] D71572: [ItaniumCXXABI] Make tls wrappers comdat on Windows

mstorsjo marked an inline comment as done. mstorsjo added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2551 + // actually handle multiple TUs defining the same wrapper. + if (CGM.getTriple().isOSWindows() && CGM.supportsCOMDAT() && +

[PATCH] D71572: [ItaniumCXXABI] Make tls wrappers comdat on Windows

rnk added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2551 + // actually handle multiple TUs defining the same wrapper. + if (CGM.getTriple().isOSWindows() && CGM.supportsCOMDAT() && + Wrapper->isWeakForLinker()) IMO we should be

[PATCH] D71572: [ItaniumCXXABI] Make tls wrappers comdat on Windows

mstorsjo updated this revision to Diff 234269. mstorsjo retitled this revision from "[ItaniumCXXABI] Use linkonce_odr instead of weak_odr for tls wrappers on Windows" to "[ItaniumCXXABI] Make tls wrappers comdat on Windows". mstorsjo edited the summary of this revision. mstorsjo added a