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() && + Wrapper->isWeakForLinker()) ---------------- mstorsjo wrote: > rnk wrote: > > IMO we should be doing the same thing on ELF, you can see the code pattern > > used elsewhere: > > http://llvm-cs.pcc.me.uk/?q=setComdat > > > > Before comdats were made explicit in the IR and we stopped implicitly > > making comdat groups for odr things on ELF, these wrappers would've been in > > a comdat group. > Ok, so remove the `isOSWindows()` and maybe remove the comment altogether as > it isn't windows specific any longer? In the current tests, for linux there's explicit checks that these aren't marked as comdat, see e.g. https://github.com/llvm/llvm-project/blob/master/clang/test/CodeGenCXX/cxx11-thread-local.cpp#L208. Is there a specific reason for that, or should it be changed? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71572/new/ https://reviews.llvm.org/D71572 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
