[PATCH] D28583: CodeGen: Allow small copyable blocks to "break" the CFG.

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:151
+static cl::opt<unsigned> TailDuplicatePlacementCFGPenalty(
+    "tail-dup-placement-cfg-penalty",
+    cl::desc("Cost penalty for blocks that can avoid breaking CFG by copying. "
----------------
perhaps simplify it to tail-dup-penalty ?


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:620
+/// TODO(iteratee): Use 64-bit fixed point edge frequencies everywhere.
+static bool greaterWithBias(BlockFrequency A, BlockFrequency B) {
+  if (A.getFrequency() < ((uint64_t) 1 << 32)) {
----------------
This basically treats the penality percent parameter as the threshold of normalized improvement:  (A-B)/B 

if ((A-B)/B > PenaltyPercent/100)
       return true;


The problem with this formula is that if B is very hot, it makes (A-B)/B become small, even though the (A-B) is still large.

So I think it is better to compute the normalized improvement as   (A-B)/Entry_Freq

basically the improvement relative to the entry frequency.  This will help prevent tail dup from happening in very cold paths.

The implementation can makes use of BranchProbablity as well. Suppose we want to implement condition:
   if ( (A-B)/Entry_Freq > P/100)
        return true;

do this 3 lines:

   BlockFrequency Profit = A - B;
   BlockFrequency Threshold = Entry_Freq  * BranchProbability(P, 100);

   return Profit > Threshold;


https://reviews.llvm.org/D28583