[PATCH] Optionally disable the BadCFGConflict check in MachineBlockPlacement.cpp

[Daniel Jasper]

Hi chandlerc,

Some benchmarks have shown that this could lead to a potential performance benefit.

A possible explanation. In diamond-shaped CFGs (A followed by either B or C both followed by D), putting B and C both in between A and D leads to the code being less dense than it could be. Always either B or C have to be skipped increasing the chance of cache misses etc. Moving either B or C to after D might be beneficial on average.

In the long run, but we should probably do a better job of analyzing the basic block and branch probabilities to move the correct one of B or C to after D. But even if we don't use this in the long run, it is a good baseline for benchmarking.

http://reviews.llvm.org/D6969

Files:
  lib/CodeGen/MachineBlockPlacement.cpp
  test/CodeGen/X86/code_placement_bad_cfg_check.ll

Index: lib/CodeGen/MachineBlockPlacement.cpp
===================================================================
--- lib/CodeGen/MachineBlockPlacement.cpp
+++ lib/CodeGen/MachineBlockPlacement.cpp
@@ -60,6 +60,12 @@
                                                 "blocks in the function."),
                                        cl::init(0), cl::Hidden);
 
+static cl::opt<bool> NoBadCFGConflictCheck(
+    "no-bad-cfg-conflict-check",
+    cl::desc("Don't check whether a hot successor has a more important "
+             "predecessor."),
+    cl::init(false), cl::Hidden);
+
 // FIXME: Find a good default for this flag and remove the flag.
 static cl::opt<unsigned>
 ExitBlockBias("block-placement-exit-block-bias",
@@ -374,29 +380,31 @@
         continue;
       }
 
-      // Make sure that a hot successor doesn't have a globally more important
-      // predecessor.
-      BlockFrequency CandidateEdgeFreq
-        = MBFI->getBlockFreq(BB) * SuccProb * HotProb.getCompl();
-      bool BadCFGConflict = false;
-      for (MachineBasicBlock::pred_iterator PI = (*SI)->pred_begin(),
-                                            PE = (*SI)->pred_end();
-           PI != PE; ++PI) {
-        if (*PI == *SI || (BlockFilter && !BlockFilter->count(*PI)) ||
-            BlockToChain[*PI] == &Chain)
+      if (!NoBadCFGConflictCheck) {
+        // Make sure that a hot successor doesn't have a globally more important
+        // predecessor.
+        BlockFrequency CandidateEdgeFreq =
+            MBFI->getBlockFreq(BB) * SuccProb * HotProb.getCompl();
+        bool BadCFGConflict = false;
+        for (MachineBasicBlock::pred_iterator PI = (*SI)->pred_begin(),
+                                              PE = (*SI)->pred_end();
+             PI != PE; ++PI) {
+          if (*PI == *SI || (BlockFilter && !BlockFilter->count(*PI)) ||
+              BlockToChain[*PI] == &Chain)
+            continue;
+          BlockFrequency PredEdgeFreq =
+              MBFI->getBlockFreq(*PI) * MBPI->getEdgeProbability(*PI, *SI);
+          if (PredEdgeFreq >= CandidateEdgeFreq) {
+            BadCFGConflict = true;
+            break;
+          }
+        }
+        if (BadCFGConflict) {
+          DEBUG(dbgs() << "    " << getBlockName(*SI) << " -> " << SuccProb
+                       << " (prob) (non-cold CFG conflict)\n");
           continue;
-        BlockFrequency PredEdgeFreq
-          = MBFI->getBlockFreq(*PI) * MBPI->getEdgeProbability(*PI, *SI);
-        if (PredEdgeFreq >= CandidateEdgeFreq) {
-          BadCFGConflict = true;
-          break;
         }
       }
-      if (BadCFGConflict) {
-        DEBUG(dbgs() << "    " << getBlockName(*SI) << " -> " << SuccProb
-                     << " (prob) (non-cold CFG conflict)\n");
-        continue;
-      }
     }
 
     DEBUG(dbgs() << "    " << getBlockName(*SI) << " -> " << SuccProb
Index: test/CodeGen/X86/code_placement_bad_cfg_check.ll
===================================================================
--- /dev/null
+++ test/CodeGen/X86/code_placement_bad_cfg_check.ll
@@ -0,0 +1,25 @@
+; RUN: llc  -mcpu=corei7 -mtriple=x86_64-linux -no-bad-cfg-conflict-check < %s | FileCheck %s
+
+;CHECK: callq h
+;CHECK: ret
+;CHECK: callq
+define void @foo(i32 %t) nounwind readnone ssp uwtable {
+  %cmp = icmp eq i32 %t, 0
+  br i1 %cmp, label %if.then, label %if.else
+
+if.then:
+  call void @f()
+  br label %if.end
+
+if.else:
+  call void @g()
+  br label %if.end
+
+if.end:
+  call void @h()
+  ret void
+}
+
+declare void @f()
+declare void @g()
+declare void @h()

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D6969.18160.patch
Type: text/x-patch
Size: 3556 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150114/0add88ef/attachment.bin>