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

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:954
 
+void CSProfileGenerator::convertProfileMap() {
+  assert(ProfileMap.empty() &&
----------------
nit: convertProfileMap -> convertToProfileMap


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:971-972
+      FunctionSamples *FSamples = Node->getFunctionSamples();
+      if (FSamples && !FSamples->getBodySamples().empty())
+        return true;
+
----------------
what makes function samples empty during conversion? 


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:980-981
+  assert(
+      !CheckIfHasValidFSamples(&getRootContext()) &&
+      "After the conversion, there should be no valid FunctionSamples on trie");
+}
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125246