[PATCH] D11662: Filter cold blocks off the loop chain when profile data is available.

[David]

davidxl added a comment.

How do you come up with the 5/1 ratio?  Why not making it 1/1? Here are the reasons

1. if the loop has high trip count, a block that executed fewer than 1 time per loop does not have any cache reuse
2. if the loop has very short trip count, it is likely that the loop won't be rotated. Splitting the cold block out increases the chances to connect loop with the exit BB with a fallthrough ..


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:825
@@ +824,3 @@
+
+    const BranchProbability ColdProb(1, 5); // 20%
+    for (MachineBasicBlock *LoopBB : L.getBlocks()) {
----------------
Overloading BranchProbablity for the purpose of computing ratios is confusing IMO. Why not just 

const unsigned LoopToColdBBRatio = 5;

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:830
@@ +829,3 @@
+          (Freq == 0 ||
+           BranchProbability(1, LoopFreq.getFrequency() / Freq) < ColdProb))
+        continue;
----------------
if (Freq * LoopToColdBBRatio < LoopFreq.getFrequency()) ..


http://reviews.llvm.org/D11662