[PATCH] D81775: [COFF] Add cg_profile directive and .llvm.call-graph-profile section

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/MC/MCParser/COFFAsmParser.cpp:336
+
+  // Only store non-temporary symbols.
+  if (!FromSym->isTemporary() && !ToSym->isTemporary())
----------------
zequanwu wrote:
> Only store non-temporary symbols pairs.
> 
> In ELF, temporary symbols are also stored. I don't get the point of doing that. As I know, temporary symbols will not show up in object file. So, there is no reason to store temporary symbols.
Temporary symbols are a subcategory of local symbols. I think in ELF, it does not really matter whether a profile edge references a local symbol or not, a temporary symbol or not. A temporary symbol can also encode a profile edge.

Is there any particular reason you want to exclude temporary symbols from COFF?


================
Comment at: llvm/lib/MC/MCWinCOFFStreamer.cpp:342
+  if (Created) {
+    cast<MCSymbolCOFF>(S)->setIsWeakExternal();
+    cast<MCSymbolCOFF>(S)->setExternal(true);
----------------
Why setIsWeakExternal?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81775