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

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleContextTracker.cpp:87
+ContextTrieNode &
+SampleContextTracker::moveToChildContext(ContextTrieNode &ToNodeParent,
+                                         const LineLocation &CallSite,
----------------
wenlei wrote:
> any reason to make this a member of SampleContextTracker rather than member of ContextTrieNode? asking because the naming `moveToChildContext` is assuming `this` is the parent to move to. 
Ok, I guess it's because `setContextNode` needs to update the map for context tracker. In that case, we probably want to make context tracker own the move completely, because `ContextTrieNode::moveToChildContext` now isn't complete without updating the map. What I'm suggesting is to move `ContextTrieNode::moveToChildContext` into this function completely. 

Also rename `SampleContextTracker::moveToChildContext` to something like `ContextTrieNode::moveContextSamples` since `this` is no longer the parent to move to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127031