[PATCH] D12778: Change the probability to check if a hot successor has a more important predecessor in block-placement.

[Cong Hou via llvm-commits]

congh added inline comments.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:354
@@ -353,3 +353,3 @@
                                            const BlockFilterSet *BlockFilter) {
   const BranchProbability HotProb(4, 5); // 80%
 
----------------
davidxl wrote:
> This value should be tuned when PGO is on to follow a clear cost model. I would like to see a separate patch for that.
The patch is ready but I am waiting for this patch being approved firstly.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:423
@@ -418,1 +422,3 @@
+      // This is why we use 25% instead of 20% as the cold probability below.
+      const BranchProbability ColdProb(1, 4); // 25%
       BlockFrequency CandidateEdgeFreq =
----------------
davidxl wrote:
> For global conflict detection, do we know why we need to apply the multiplier on the successor edge frequency and then compare with the conflicting incoming edge in the first place? 
At this point the decision whether to break CFG constraint is made. Considering the diamond branch I gave in the description and assume the probabilities are 60% and 40% respectively (for B and C). After B is chosen, we now decide whether to layout the other branch C. If we don't apply the multiplier here, we can see 40% < 60% and we may choose the D instead of C, which is not right (as we are using 80% as the threshold). 


http://reviews.llvm.org/D12778