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

[Zequan Wu via Phabricator via llvm-commits]

zequanwu marked 2 inline comments as done.
zequanwu added inline comments.


================
Comment at: llvm/lib/MC/MCParser/COFFAsmParser.cpp:336
+
+  // Only store non-temporary symbols.
+  if (!FromSym->isTemporary() && !ToSym->isTemporary())
----------------
MaskRay wrote:
> 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?
>From my understanding, since temporary symbols doesn't show up in final binary and the purpose of `.llvm.call-graph-profile`  is to be used to optimize section layouts of final binary, storing temporary symbols doesn't help. 


================
Comment at: llvm/lib/MC/MCWinCOFFStreamer.cpp:342
+  if (Created) {
+    cast<MCSymbolCOFF>(S)->setIsWeakExternal();
+    cast<MCSymbolCOFF>(S)->setExternal(true);
----------------
MaskRay wrote:
> Why setIsWeakExternal?
Doing the same as `finalizeCGProfileEntry` in ELF, https://github.com/llvm/llvm-project/blob/master/llvm/lib/MC/MCELFStreamer.cpp#L483.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81775