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

[Cong Hou]

congh added inline comments.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:365-371
@@ -364,4 +364,9 @@
   uint32_t WeightScale = 0;
   uint32_t SumWeight = MBPI->getSumForBlock(BB, WeightScale);
-  DEBUG(dbgs() << "Attempting merge from: " << getBlockName(BB) << "\n");
+
+  // Adjust sum of weights by excluding weights on edges pointing to blocks that
+  // cannot be BB's successor in the chain.
+  uint32_t AdjustedSumWeight = SumWeight;
+  SmallVector<MachineBasicBlock *, 4> Successors;
   for (MachineBasicBlock *Succ : BB->successors()) {
+    bool SkipSucc = false;
----------------
congh wrote:
> chandlerc wrote:
> > This is made incredibly annoying because we don't rescale the branch probabilities in MBPI the way we do in BPI, and so we don't know for a fact that they fit in 32-bits.
> > 
> > I think we need to fix MBPI to automatically scale all the weights so that the sum of weights is known to fit in 32-bits. Once that's done, WeightScale and all the code in getSumForBlock to rescale things goes away.
> > 
> > Then, here, you should change this to just compute the sum directly rather than getting the sum from MBPI and then adjusting it after-the-fact.
> > 
> > Also, I would just embed the predicate into a lambda, and write two loops over BB->successors() rather than building up a vector of successors.
> OK. Let me try to produce a patch doing edge weight normalization for MBPI. 
I found it not straightforward to normalize weights in MBPI. Note that MBPI reads weights from MBB, which provides the interface addSuccessor(MachineBasicBlock *succ, uint32_t weight) to let users indicate arbitrary weights for new added edges. Because we don't know how users use this interface, it can be difficult to adjust weights in MBB. I think there may be two solutions:

1. We provide the interface normalizeWeights in MBB. Once we call this interface, the sum of weights is guaranteed to be less than UINT32_MAX. However, after the user add new successors with weights, we need to re-normalize it. This can still be useful when we want to calculate branch probabilities or do other adjustment after the weights are normalized.

2. We maintained both normalized and original weights in MBB, so that once the original weights is modified, we also adjust the normalized weights. This solutions requires more space and more frequent calculations comparing to the first one, but the normalized weights are always guaranteed.

Do you have any other ideas? Or which one do you think is better?


http://reviews.llvm.org/D10825