[PATCH] D109860: profi - a flow-based profile inference algorithm: Part I (out of 3)

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h:893
 
-    // Compute dominance and loop info needed for propagation.
-    computeDominanceAndLoopInfo(F);
+    if (!SampleProfileUseProfi) {
+        // Compute dominance and loop info needed for propagation.
----------------
It is probably better to restructure the code a little more.

Add a helper routine called "PrepareForPropagation(..)' or "InitForPropagation'. In this helper 
1) buildEdges;
2) compute equiv class for non-profi or profi specific initialization (basically move some code from propagateWeights' to here.


================
Comment at: llvm/include/llvm/Transforms/Utils/SampleProfileLoaderBaseImpl.h:910
+    // Samples->getHeadSamples() + 1 to avoid functions with zero count.
+    if (SampleProfileUseProfi) {
+      const BasicBlockT *EntryBB = getEntryBB(&F);
----------------
Move it into a Finalize helper method?


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:1561
     Instruction *MaxDestInst;
+    // Since profi treats multiple edges (multiway branches) as a single edge,
+    // we need to distribute the computed weight among the branches. We do
----------------
Is this a limitation? Can it be made to handle multi-graph?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109860