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

[David]

davidxl added inline comments.

================
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()) {
----------------
congh wrote:
> davidxl wrote:
> > congh wrote:
> > > 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.
> > I think the adjustment can be done without even needing to have a top level for loop like
> > 
> > for (auto Succ: BB->successors()) {
> >    if (...) continue;
> >    for (auto SuccBB : BB->successors())
> > 	​        if (MLI->getLoopFor(SuccBB) == L)
> > 		​          SumWeight2 -= MBPI->getEdgeWeight(BB, SuccBB) / WeightScale;
> >     ..
> > }
> > 
> > but simply:
> > 
> > 
> >  for (auto SuccBB : BB->successors())
> >         // We find out that SuccBB is already in 'Chain', but it is not the end of 'Chain' -- it must be a block in the inner 
> >         // loop including BB that has already be laid out. Skip it 
> >         if (BlockToChain(SuccBB) == &Chain)   
> > 		​   SumWeight -= MBPI->getEdgeWeight(BB, SuccBB) / WeightScale;
> But if the first successor in BB->successors() is not in the same loop as BB, we won't get the correct SumWeight in this way.
> 
> As most CFG nodes have at most two outgoing edges, we won't get many repeated computations.
If the successor of BB is not in the same loop as BB but matches the condition I listed, there are two cases:
1) the succBB is in an inner loop of BB's loop, in this case we want to skip this SuccBB
2) the succBB is in the current loop being laid out.  If succBB satisfies the condition, this block must be a hotter block that is laid out before 'BB' with top-order broken. In this case, edge weight (BB->SuccBB) should also be skipped -- this is not any different from the backedge case. 


http://reviews.llvm.org/D10825