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

[Gerolf Hoflehner via llvm-commits]

Gerolf added a subscriber: Gerolf.
Gerolf added a comment.

Can you also elaborate a bit more on the iCache benefits.Is this specific to an architecture or the JIT environment? Or part of a more elaborate implementation of function splitting than shared in the patch? Perhaps I'm missing something  but I thought the splinter must be able to duplicate code to support code layout to reap Icache benefits.  And a good block ordering algorithm is might already eat the benefits.

Also I'm curious about your design evaluation. I think the information you are looking at at the low level (at least currently) is also be available at the IR level.  When the compiler could split there I can also see compile-time benefits eg. a function marked as cold would not have to be optimized. And there would be less code to optimize/analyze in the hot routines.


================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:887
@@ +886,3 @@
+      bool IsCold = MBB.isEHPad();
+      if (!IsCold) {
+        auto BP = BranchProbability::getBranchProbability(
----------------
That deserves at least more comment and rational. Is this a good heuristic on all machines?

================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:895
@@ +894,3 @@
+      if (IsCold) {
+        CurSection = ColdSection;
+        ColdFragmentStart = &MBB;
----------------
It looks weird that within the loop cold section is set, but never reset for a hot block. It seems that it would be cleaner to group all blocks into hot and cold.

================
Comment at: lib/CodeGen/AsmPrinter/AsmPrinter.cpp:977
@@ -919,1 +976,3 @@
 
+  if (CurSection == ColdSection) {
+    CurSection = HotSection;
----------------
I think when all block are grouped into hot and code the end_of_function directive could be issued at the same place for hot and cold section. It seems hard to mantain to have to think about hot and cold in various context.


http://reviews.llvm.org/D17555