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

[Wenlei He via Phabricator via llvm-commits]

wenlei accepted this revision.
wenlei added a comment.

lgtm, thanks.



================
Comment at: llvm/lib/Transforms/IPO/SampleContextTracker.cpp:86
 
   while (!NodeToUpdate.empty()) {
     ContextTrieNode *Node = NodeToUpdate.front();
----------------
wlei wrote:
> wenlei wrote:
> > We can use the iterator for this as well? 
> We can use iterator for this. But we need to call `setParentContext` to set Parent for all childNode, there is still the below for loop inside, it's not like others very straightforward good for the readability. WDYT?
> 
> ``` 
>   for(.....) {
>      ....
>     for (auto &It : Node->getAllChildContext()) {
>       ContextTrieNode *ChildNode = &It.second;
>       ChildNode->setParentContext(Node);
>    }
> ```
> 
Ok, then it works either way. Keep current version is also good because this loop involves modification. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127031