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

[Daniel Jasper via llvm-commits]

djasper added inline comments.

================
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 =
----------------
congh wrote:
> 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). 
I think we should not hard code dependent values here. The 1/4 is strictly dependent on HotProb. So either pull out the denominator of HotProb into a variable or calculate this directly. I think you might need to replace

  HotProb.getCompl()

with

  HotProb.getCompl() / HotProb

But I am still jetlagged, so my math might be off.


http://reviews.llvm.org/D12778