[PATCH] D73387: [MBP] Partial tail duplication into hot predecessors

[Guozhi Wei via Phabricator via llvm-commits]

Carrot added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineBlockPlacement.cpp:1156
+  // precise benefit analysis.
+  if (F->getFunction().hasProfileData())
+    return true;
----------------
davidxl wrote:
> should the benefit analysis be done here instead of just returning true?
It's possible to call findDuplicateCandidates here. But it needs to be called again just before TailDup.tailDuplicateAndUpdate to provide the duplicate candidates. 


================
Comment at: llvm/lib/CodeGen/MachineBlockPlacement.cpp:1818
+      // it out, just go round the loop again with BB as the chain end.
+      if (!BB->isSuccessor(BestSucc))
         continue;
----------------
davidxl wrote:
> can you explain this new condition?
Previously if function repeatedlyTailDuplicateBlock returns true, BestSucc is duplicated into all predecessors and removed. So there is no BestSucc any more, so we goto the loop header to find another best successor for BB.

Now with partial duplication,  BestSucc may or may not be duplicated into BB.
    * If BestSucc is duplicated into BB, thus the condition is true, BestSucc is not BB's successor, we need to find another successor of BB.
    * If BestSucc is not duplicated into BB, it falls through to the code to layout BestSucc after BB.


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

https://reviews.llvm.org/D73387