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

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/include/llvm/Transforms/IPO/SampleContextTracker.h:106
   // deterministically.
-  using ContextSamplesTy = std::set<FunctionSamples *, ProfileComparer>;
 
----------------
Is std::set unnecessary or is it wrong? 


================
Comment at: llvm/lib/Transforms/IPO/SampleContextTracker.cpp:389
+      // Skip inlined context profile
+      if (ContextOnNode.hasState(InlinedContext))
+        continue;
----------------
Why we need to check MergedContext on OldContext, but InlinedContext on ContextOnNode?


================
Comment at: llvm/lib/Transforms/IPO/SampleContextTracker.cpp:472
+  if (Node == &RootContext)
+    return "";
+  Res.emplace_back(Node->getFuncName(), LineLocation(0, 0));
----------------
`return string();`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127031