[PATCH] D125246: [CSSPGO][llvm-profgen] Reimplement CS profile generator using context trie

[Lei Wang via Phabricator via llvm-commits]

wlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:971-972
+      FunctionSamples *FSamples = Node->getFunctionSamples();
+      if (FSamples && !FSamples->getBodySamples().empty())
+        return true;
+
----------------
wenlei wrote:
> what makes function samples empty during conversion? 
IMO, The std::move will move the `BodySamples` from the trie node's FSamples to the ProfileMap's FSamples. This is to check after the conversion no FSamples left  not moved.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:980-981
+  assert(
+      !CheckIfHasValidFSamples(&getRootContext()) &&
+      "After the conversion, there should be no valid FunctionSamples on trie");
+}
----------------
wenlei wrote:
> I was thinking about checking to make sure things are valid before conversion. Checking after conversion doesn't prevent double conversion where the 2nd conversion would be wrong. We could also have a simple state boolean to represent the statefulness and use that for checks.
By a valid state, do you mean to check that the trie node's `getAllChildren` is not empty?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125246