mstorsjo marked an inline comment as done.
mstorsjo added inline comments.

================
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2519-2520
       return VarLinkage;
+  // On Windows, WeakODR is a no-op, boiling down to the same as normal 
external
+  // linkage.
+  if (CGM.getTriple().isOSWindows())
----------------
mstorsjo wrote:
> rnk wrote:
> > mstorsjo wrote:
> > > rnk wrote:
> > > > I would say that this is inaccurate. It greatly affects what the 
> > > > optimizer is allowed to do.
> > > > 
> > > > It looks like we forgot to put a comdat on these things, is that not 
> > > > the correct fix?
> > > Oh, ok.
> > > 
> > > The full case I was trying to fix (but forgot to recheck after changing 
> > > this bit) is that when used with `-ffunction-sections`, the tls wrapper 
> > > function ends up as comdat `one_only`, which then gives multiple 
> > > definition errors. So perhaps the issue is in the handling of 
> > > `-ffunction-sections` wrt weak_odr?
> > Ah, that is an interesting wrinkle. I'm surprised that things worked 
> > without -ffunction-sections, though. I would've expected the two plain 
> > external definitions to produce multiple definition errors.
> Without `-ffunction-sections`, there's no separate thread wrapper produced at 
> all, so everything else is generated with working linkage. I just haven't 
> happened to build code that forces generation of a tls wrapper without 
> `-ffunction-sections` yet.
Actually, sorry, I misremembered. Yes, I think the same issue would be present 
even without `-ffunction-sections`.


Repository:
  rG LLVM Github Monorepo

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

Reply via email to