[PATCH] D139870: [BOLT] using jump weights in profi

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SampleProfileInference.cpp:1088
     // Edges from S and to T
-    assert((!Block.isEntry() || !Block.isExit()) &&
-           "a block cannot be an entry and an exit");
----------------
Does this still hold?


================
Comment at: llvm/lib/Transforms/Utils/SampleProfileInference.cpp:1120
-    // as all of the increase can be attributed to the self-edge
-    if (Block.HasSelfEdge) {
-      AuxCostDec = 0;
----------------
Where is this handled in the new implementation?


================
Comment at: llvm/lib/Transforms/Utils/SampleProfileInference.cpp:1125
-    Network.addEdge(Bin, Baux, AuxCostInc);
-    Network.addEdge(Baux, Bout, AuxCostInc);
     if (Block.Weight > 0) {
----------------
Previously the cost it takes to go from `Bin` to `Bout` is `2*AuxCostInc`. Now without `Baux` the cost for the same path becomes `AuxCostInc`. Is it a discrepancy or am I missing anything? 


================
Comment at: llvm/lib/Transforms/Utils/SampleProfileInference.cpp:1164
+  // Update the costs depending on the block metadata
+  if (Jump.Source + 1 == Jump.Target) {
+    CostInc = Params.CostJumpFTInc;
----------------
Add a comment to indicate the check is for fall-through branches?


================
Comment at: llvm/lib/Transforms/Utils/SampleProfileInference.cpp:1222
+  for (size_t I = 1; I < Func.Blocks.size(); I++) {
+    assert(!Func.Blocks[I].isEntry());
+  }
----------------
Please add a message for the assert and the one right below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139870