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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D44543#2956752 <https://reviews.llvm.org/D44543#2956752>, @mstorsjo wrote:

> In D44543#2956262 <https://reviews.llvm.org/D44543#2956262>, @MaskRay wrote:
>
>> `} else if (MAI->avoidWeakIfComdat() && GV->hasComdat()) {` in AsmPrinter.cpp makes the comdat leader symbol unconditionally `.globl`.
>> Will it be possible to allow `.weak` leader?
>
> I'm not sure...
>
>> If allowed, `clang -fprofile-instr-generate a.c` can place the weak `__profc_weak` in an associative comdat (`comdat nodeduplicate`) for the following code:
>>
>>   __attribute__((weak)) void foo() {}
>>
>> and not getting "duplicate definition error" with `lld-link -lldmingw`.
>
> Can you elaborate on the issue? Right now I can successfully build and link code that uses `__attribute__((weak))` with `-fprofile-instr-generate` in mingw mode with clang+lld. Or is the situation relating to the upcoming state after some other patches? (I'm entirely unfamiliar with the internals of the PGO implementation.)

For `__attribute__((weak)) void foo() {}`, currently the profd and profc variables don't use comdat.
`compiler-rt/test/profile/Windows/coverage-weak-lld.cpp` works because in `lld-link -lldmingw` mode,  weak definitions are allowed to be duplicated.

If I use comdat (rGfbb8e772ec501a1b71643db90e9c6445e17d7cac <https://reviews.llvm.org/rGfbb8e772ec501a1b71643db90e9c6445e17d7cac> sorry, I accidentally pushed it, but reverted) for profd and profc, the comdat leader symbol will become `.globl`.
In the `ninja check-profile` result, `compiler-rt/test/profile/Windows/coverage-weak-lld.cpp` fails.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D44543/new/

https://reviews.llvm.org/D44543