[PATCH] D10825: Improvement on computing edge probabilities when choosing the best successor in machine block placement.

[Cong Hou]

congh marked 3 inline comments as done.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:367
@@ -365,3 +366,3 @@
   uint32_t SumWeight = MBPI->getSumForBlock(BB, WeightScale);
   DEBUG(dbgs() << "Attempting merge from: " << getBlockName(BB) << "\n");
   for (MachineBasicBlock *Succ : BB->successors()) {
----------------
davidxl wrote:
> Do SumWeight adjustment here -- outside the main loop:
> 
> uint32_t SumWeight = ...
> SumWeight = AdjustWeightSumExcludingInternalBackEdge(SumWeight, BB);
The adjustment on SumWeight is done only when it has a successor outside of its loop that is also in BlockFilter (if the successor outside of the loop is not in BlockFilter, we are building chains for the loop). It makes more sense to calculate this adjusted weight inside of this loop. Note that when we build chains for the loop, we don't need this adjusted weight at all.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:386
@@ +385,3 @@
+    // when BB belongs to a loop but we are building chains for another loop
+    // (either outer or peer).  In this case, the edge from BB to the successor
+    // outside of the inner loop usually has a small probability, due to edges
----------------
davidxl wrote:
> Do you have examples of the problem when chain formation is for a 'peer' loop?
The following partial CFG shows such a case:

```
A
⇵
B
↓
C
⇵
D
```

Here, B->C is an edge between two peer loops.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:393
@@ +392,3 @@
+    auto SuccLoop = MLI->getLoopFor(Succ);
+    if (L && L != SuccLoop && !(SuccLoop && L->contains(SuccLoop)))
+      for (auto SuccBB : BB->successors())
----------------
davidxl wrote:
> davidxl wrote:
> > need to check SuccLoop !=0 ?
> It is more readable to move the Sumweight adjustment loop outside the main for loop and probably in a helper function.
SuccLoop can be 0 when the successor is not in any loop.


http://reviews.llvm.org/D10825