[PATCH] D152217: [profi][NFC] Refactor SampleProfileInference::apply

[Sergey Pupyrev via Phabricator via llvm-commits]

spupyrev added a comment.

Do you have a follow-up diff to adjust BOLT's part?



================
Comment at: llvm/include/llvm/Transforms/Utils/SampleProfileInference.h:261-264
+  // Keep a stable order for reachable blocks
+  BlockIndexMap BlockIndex;
+  BlockList BasicBlocks;
+  createStableOrder(BlockIndex, BasicBlocks, Reachable, InverseReachable);
----------------
"stable order" confuses me. How about "fixed order" or simply "an order of blocks"?


================
Comment at: llvm/include/llvm/Transforms/Utils/SampleProfileInference.h:266-268
+  bool HasSamples = preAssignWeights(BlockWeights, EdgeWeights, BasicBlocks);
+  if (!canApplyInference(BasicBlocks, HasSamples))
+    return;
----------------
I'd separate the computation of HasSamples and preAssignWeights, as they seem to be unrelated:



================
Comment at: llvm/include/llvm/Transforms/Utils/SampleProfileInference.h:277-278
   // Extract the resulting weights from the control flow
   // All weights are increased by one to avoid propagation errors introduced by
   // zero weights.
+  extractProfile(BlockWeights, EdgeWeights, Func, BasicBlocks, BlockIndex);
----------------
this comment doesn't make sense (perhaps, outdated). remove maybe


================
Comment at: llvm/include/llvm/Transforms/Utils/SampleProfileInference.h:302
+    const BlockVisitedSet &Reachable, const BlockVisitedSet &InverseReachable) {
 #ifndef NDEBUG
   // Unreachable blocks and edges should not have a weight.
----------------
should this marker be moved to the function call?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152217