[PATCH] D19435: [LowerExpectIntrinsic] make default likely/unlikely ratio bigger

[Sanjay Patel via llvm-commits]

spatel retitled this revision from "[LowerExpectIntrinsic] pin default likely/unlikely weights to min/max values" to "[LowerExpectIntrinsic] make default likely/unlikely ratio bigger".
spatel updated the summary for this revision.
spatel updated this revision to Diff 55061.
spatel added a comment.

Patch updated:
Changed weights to 2000 and 1 as suggested by David.


http://reviews.llvm.org/D19435

Files:
  lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  test/Transforms/LowerExpectIntrinsic/basic.ll

Index: test/Transforms/LowerExpectIntrinsic/basic.ll
===================================================================
--- test/Transforms/LowerExpectIntrinsic/basic.ll
+++ test/Transforms/LowerExpectIntrinsic/basic.ll
@@ -275,7 +275,7 @@
 
 declare i1 @llvm.expect.i1(i1, i1) nounwind readnone
 
-; CHECK: !0 = !{!"branch_weights", i32 64, i32 4}
-; CHECK: !1 = !{!"branch_weights", i32 4, i32 64}
-; CHECK: !2 = !{!"branch_weights", i32 4, i32 64, i32 4}
-; CHECK: !3 = !{!"branch_weights", i32 64, i32 4, i32 4}
+; CHECK: !0 = !{!"branch_weights", i32 2000, i32 1}
+; CHECK: !1 = !{!"branch_weights", i32 1, i32 2000}
+; CHECK: !2 = !{!"branch_weights", i32 1, i32 2000, i32 1}
+; CHECK: !3 = !{!"branch_weights", i32 2000, i32 1, i32 1}
Index: lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
===================================================================
--- lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
+++ lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
@@ -34,12 +34,24 @@
 STATISTIC(ExpectIntrinsicsHandled,
           "Number of 'expect' intrinsic instructions handled");
 
-static cl::opt<uint32_t>
-LikelyBranchWeight("likely-branch-weight", cl::Hidden, cl::init(64),
-                   cl::desc("Weight of the branch likely to be taken (default = 64)"));
-static cl::opt<uint32_t>
-UnlikelyBranchWeight("unlikely-branch-weight", cl::Hidden, cl::init(4),
-                   cl::desc("Weight of the branch unlikely to be taken (default = 4)"));
+// These default values are chosen to represent an extremely skewed outcome for
+// a condition, but they leave some room for interpretation by later passes.
+//
+// If the documentation for __builtin_expect() was made explicit that it should
+// only be used in extreme cases, we could make this ratio higher. As it stands,
+// programmers may be using __builtin_expect() as a substitute for actual
+// profile data.
+//
+// There is a known dependency on this ratio in CodeGenPrepare when transforming
+// 'select' instructions. It may be worthwhile to hoist these values to some
+// shared space, so they can be used directly by other passes.
+
+static cl::opt<uint32_t> LikelyBranchWeight(
+    "likely-branch-weight", cl::Hidden, cl::init(2000),
+    cl::desc("Weight of the branch likely to be taken (default = 2000)"));
+static cl::opt<uint32_t> UnlikelyBranchWeight(
+    "unlikely-branch-weight", cl::Hidden, cl::init(1),
+    cl::desc("Weight of the branch unlikely to be taken (default = 1)"));
 
 static bool handleSwitchExpect(SwitchInst &SI) {
   CallInst *CI = dyn_cast<CallInst>(SI.getCondition());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D19435.55061.patch
Type: text/x-patch
Size: 2580 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160426/23fab135/attachment.bin>