[PATCH] D44543: [AsmPrinter] Emit .weak directive for weak linkage on COFF

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

In https://reviews.llvm.org/D44543#1039640, @espindola wrote:

> In https://reviews.llvm.org/D44543#1039582, @smeenai wrote:
>
> > Yeah, you're right. Introducing a new linkage type for `__attribute__((weak))` definitions is probably the way to go then?
>
>
> I don't think so.
>
> The cases Reid lists are not really weak when targeting COFF.  If they were not in a comdat we would get a linker error.
>
> The solution I think is to have weak in the IR really mean weak (I.E., this change is fine once the dependencies are done) but change the producers to not use weak/linkonce if they don't want weak semantics.
>
> This would require first splitting linkonce out of linkage to be just a can_drop_if_unused bit.


I think my perspective is informed by the text in LangRef and whatever LLVM used to do for COFF when I started working on it. Meaning, weak and linkonce linkages just create a comdat, nothing else. The weak vs. linkonce bit indicates what's discardable. The ODR bit indicates whether you can optimize.

Being able to have undefined weak symbols was always just some weird ELF feature that wasn't spelled out in LangRef and didn't work on COFF so it must not be inside the model. It's entirely possible that I've given that old model too much weight and we should update the LangRef to talk about how weak declarations can be undefined at link time.

Now that we have comdats, weak_odr and linkonce_odr are pretty weird linkages. We could, for example, migrate weak_odr+comdat things to just external+comdat, and that would work pretty well. External GVs aren't discardable, but most of the optimizer still reasons from the linkage without looking at the comdat, so this would be a pretty big change.


Repository:
  rL LLVM

https://reviews.llvm.org/D44543