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

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/include/llvm/Transforms/IPO/SampleContextTracker.h:185
+  // FunctionSamples points to.
+  std::unordered_map<ContextTrieNode *, std::unordered_set<FunctionSamples *>>
+      Node2FSamplesSetMap;
----------------
wlei wrote:
> hoy wrote:
> > Actually my previous question could be made more clear as why we need to maintain this map.
> I see, so you're suggested that 
>  1  FSamples2NodeMap make sure FSamples always pointed to its belonging context node. 
> 2  even the FSamples whose context is `mergedcontext` point to a stale trie node, we can filter out by the 
> ```
>       SampleContext &OldContext = CSamples->getContext();
>       // Check to avoid re-merge any context.
>       if (OldContext.hasState(MergedContext))
> ```
> So we won't have a redundant node to be merged. We also don't need to update FSample to point to the merged node.
> 
>  That sounds a clear way!
Exactly. Hopefully the logic can be simplified a little bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127031