[PATCH] D127031: [CSSPGO][llvm-profgen] Reimplement SampleContextTracker using context trie

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleContextTracker.cpp:380
 
+      SampleContext &ContextOnNode =
+          FromNode->getFunctionSamples()->getContext();
----------------
ContextOnNode and OldContext should be the same thing now?


================
Comment at: llvm/lib/Transforms/IPO/SampleContextTracker.cpp:602
       ToSamples->getContext().setAttribute(ContextShouldBeInlined);
+    FromNode.setFunctionSamples(nullptr);
   } else if (FromSamples) {
----------------
This is no longer needed?


================
Comment at: llvm/tools/llvm-profgen/CSPreInliner.cpp:311
+         !FProfile->getContext().hasState(InlinedContext))) {
+      Node->setFunctionSamples(nullptr);
     }
----------------
Should we just keep the original condition checks? 

Also, where do we actually deallocate the function profile?


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.h:164
   // closest matching context.
-  uint32_t getFuncSizeForContext(const SampleContext &Context);
+  uint32_t getFuncSizeForContext(ContextTrieNode *Context);
 
----------------
nit: use const for the parameter


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127031