[PATCH] D17555: [Feedback requested] Implement cold spliting

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:875
@@ +874,3 @@
+
+  bool SplitColdCode = ColdSection && TM.Options.MCOptions.SplitColdCode;
+  if (SplitColdCode) {
----------------
Should the creation of the ColdTextSection be guarded with the option such that if it is not enabled, null pointer will be returned and checked here?

================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:888
@@ +887,3 @@
+      if (!IsCold) {
+        auto BP = BranchProbability::getBranchProbability(
+                        MBFI->getBlockFreq(&MBB).getFrequency(), EntryFreq);
----------------
This is not correct -- it will trigger debug assert when MBB's frequency is larger than the Entry's -- skip that case first.  Also you are using BP as a ratio here not really as branch probability.

BBFreq = ...;

IsCold = (BBFreq < EntryFreq && BranchProbablity::getBranchProbablity(BBFreq, EntryFreq) <= BranchProbability(1, 1<<...);

================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:895
@@ +894,3 @@
+      if (IsCold) {
+        CurSection = ColdSection;
+        ColdFragmentStart = &MBB;
----------------
deadalnix wrote:
> In practice it works, as this ends up being more or less what you get out of the MachineBlockPlacement pass. Ideally, that's be indeed preferable that MachineBlockPlacement flag a BB after which all BB are cold or something.
> 
> Having one split is preferable, as well as having all exception unwinding related code is the same fragment. You don't want to jump back and forth between cold and hot code, and generating LSDA would become way too hairy =.
This does not look like a good assumption to make about MBP. MBP does outline cold blocks from loops and layout them last, but not all such blocks are suitable to be split out.


http://reviews.llvm.org/D17555