[PATCH] D131043: [InstrProf] Set prof global variables to internal linkage if adding a comdat

[Reid Kleckner via Phabricator via llvm-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:917-918
       GV->setComdat(C);
+      // COFF does not allow a private global variable in a comdat of the same
+      // name.
+      if (TT.isOSBinFormatCOFF() && GV->hasPrivateLinkage())
----------------
I would phase this differently: COFF doesn't allow the comdat group leader to have private linkage, so upgrade private linkage to internal linkage to produce a symbol table entry.

That's the difference between private and internal linkage, at least for ELF and COFF: whether or not there is a symbol table entry. COFF comdat info lives in the symbol table, so if you don't have one, you can't be a comdat symbol.

I think the next best alternative fix would be to paper over this platform specific detail in the backend, and produce symbol table entries for private+comdat symbols. I don't recall why the verifier error was preferred over this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131043