[PATCH] D130121: [3/3] [COFF] Emit embedded -exclude-symbols: directives for hidden visibility for MinGW

Thu Sep 1 00:40:20 PDT 2022

mstorsjo added a comment.

In D130121#3723783 <https://reviews.llvm.org/D130121#3723783>, @rnk wrote:

> Regarding backporting, I thought we had tried to backport bug fixes, not features, and this is definitely a new feature. OTOH it is quite useful. So, either way seems fine to me.

I think it makes sense to leave this one out for now still, but let downstream distros (i.e. msys2) play with the patch until the next release round. FWIW I found one detail where we can improve this patch too.

Also, regarding the necessity for this patch:

> In llvm-mingw, only the X86, ARM and AArch64 backends are enabled; for the LLVM 13.0.0 release, libLLVM-13.dll ended up with 58112 exported symbols. For LLVM 14.0.0, it was 62015 symbols. Current builds of git main end up at around 64650 symbols - i.e. extremely close to the limit.

Current git main of llvm as of today finally broke this threshold: If trying to build a dylib with these targets enabled, with a toolchain that doesn't handle exluding symbols based on hidden visibility, it now fails due to exporting too many symbols. (https://github.com/mstorsjo/llvm-mingw/runs/8125805911?check_suite_focus=true#logs) I.e., to build main/16.x of llvm as a mingw dylib, you need clang from main/16.x or a patched 15.x. Not a big issue in practice as msys2 does carry these patches downstream already in their current 14.x version though - but it shows how really how urgent this feature turned out. (The github actions build should be fixable by just setting a flag to force it to update the msys2 install before trying to build.)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130121