[PATCH] D64376: [MBP] Avoid tail duplication if it can't bring benefit

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1075
 
+  bool Duplicate = true;
+  unsigned int NumDup = 0;
----------------
Add comments for the two variables.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1123
         continue;
-      return false;
+      Duplicate = false;
+      continue;
----------------
Early return is still valid here, right?


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1137
+  // other machanism to handle different cases.
+  if (Succ->succ_size() == 0)
+    return true;
----------------
Move the check earlier than the loop.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1143
+
+  // If the duplication candidate has more unplaced predecessors than
+  // successors, the extra duplication can't bring more fallthrough.
----------------
This does not seem right.   Dup was Pred1's fall through successor and Succ1 was Dup's fall through (without tailDup). Also assume Pred2 and Pred3 has one single successor (otherwise Dup can not be tailDup ed into them).

Here, we should look at reduced taken branches, not total fallthrough --- because once Dup is tail duped into Pred2 and Pred3, it is merged with them thus has an implicit fall through there.

Let the incoming frequency to Dup from Pred1,2,3 are F1, F2, and F3, and assume the Probability from Dup to Succ1 is P(S1).  We also assume path insensitivity here in the calculation.

Before the tail duplication, the total taken branches are

T1 = F2 + F3 + (F1+F2+F3)*(1-P(S1))

After taildup, assuming Succ1 is a fall through of Pred1->Dup chain, and Succ2 is a fall through of Pred2,  then the total taken branches is

T2 = F1 * (1-(P(S1)) +  F2 * P(S1) + F3

so the difference is :

T1 - T2 = 2*(1-P(S1)) * F2 + (1-P(S1)) * F3   which is greater than 0.



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64376/new/

https://reviews.llvm.org/D64376