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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: StephenFan.

Thanks!



================
Comment at: llvm/lib/IR/Mangler.cpp:248
+    OS << " -exclude-symbols:";
+
+    bool NeedQuotes = GV->hasName() && !canBeUnquotedInDirective(GV->getName());
----------------
mstorsjo wrote:
> MaskRay wrote:
> > Is it useful to encode several symbols joined by `,` with one `-exclude-symbols:`? Just to make the section content smaller.
> Yes, that would be useful given that the option does support multiple symbols per directive (contrary to `-export`). But I think that would require some amount of extra refactoring, because `emitLinkerFlagsForGlobalCOFF` is called with one `GlobalValue` at a time, and it is called from at least 3 different locations.
You mentioned

> And to doublecheck things; this object file now has a .drectve section at 409 KB.)

So I think it's worth doing some refactoring, but the change can be separate.


================
Comment at: llvm/test/CodeGen/X86/mingw-hidden.ll:72
+; CHECK-MINGW: .section .drectve
+; CHECK-MINGW: .ascii " -exclude-symbols:f1"
+; CHECK-MINGW: .ascii " -exclude-symbols:f2"
----------------
mstorsjo wrote:
> MaskRay wrote:
> > -SAME
> I don't see how `-SAME` would help anything here? All the `.section .drectve` and all the `.ascii` lines are on separate lines in the output.
Sorry, ignore my comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130121