[PATCH] D99146: [CSSPGO][llvm-profgen] Context-sensitive global pre-inliner

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:105
+
+  // Ideally we want to consider everything a functions calls, but as far as
+  // context profile is concerned, only those frames that are children of
----------------
nit: a function call


================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:150
+  unsigned int SampleThreshold = SampleColdCallSiteThreshold;
+  if (Candidate.CallsiteCount > HotCountThreshold)
+    SampleThreshold = SampleHotCallSiteThreshold;
----------------
Should this be `>=`?


================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:168
+  // Use the number of lines/probes as proxy for function size for now.
+  unsigned FuncSize = FSamples->getBodySamples().size();
+  unsigned FuncFinalSize = FuncSize;
----------------
This currently only reflects the number of live/hot lines. Might be extended to using static size from dwarf/probe decoding or disassembling. Can you leave a TODO for this? 


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:470
+    // After CSPreInliner the key of ProfileMap is no longer accurate for
+    // contex, use the context attached to function samples instead.
+    std::string ContextWithBracket =
----------------
Nit: context


================
Comment at: llvm/tools/llvm-profgen/ProfiledCallGraph.h:29
+
+  struct ProfiledCallGraphNodeComparor {
+    bool operator()(const ProfiledCallGraphNode *L,
----------------
Nit: ProfiledCallGraphNodeComparer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99146