[PATCH] D10825: Improvement on computing edge probabilities when choosing the best successor in machine block placement.
David
davidxl at google.com
Thu Jul 23 00:30:08 PDT 2015
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()) {
----------------
Do SumWeight adjustment here -- outside the main loop:
uint32_t SumWeight = ...
SumWeight = AdjustWeightSumExcludingInternalBackEdge(SumWeight, BB);
================
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
----------------
Do you have examples of the problem when chain formation is for a 'peer' loop?
================
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())
----------------
need to check SuccLoop !=0 ?
================
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:
> 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.
================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:397
@@ -382,1 +396,3 @@
+ SumWeight2 -= MBPI->getEdgeWeight(BB, SuccBB) / WeightScale;
+ BranchProbability SuccProbInLoop(SuccWeight / WeightScale, SumWeight2);
----------------
No need to change SuccProb to SuccProbInLoop.
================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:402
@@ -385,3 +401,3 @@
// successors must be optional. Don't do this for cold branches.
- if (OutlineOptionalBranches && SuccProb > HotProb.getCompl() &&
+ if (OutlineOptionalBranches && SuccProbInLoop > HotProb.getCompl() &&
UnavoidableBlocks.count(Succ) > 0) {
----------------
No need for var name change.
================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:434
@@ -417,2 +433,3 @@
// important predecessor.
+ BranchProbability SuccProb(SuccWeight / WeightScale, SumWeight);
BlockFrequency CandidateEdgeFreq =
----------------
Change SuccProb to RealSuccProb
http://reviews.llvm.org/D10825
More information about the llvm-commits
mailing list