[PATCH] change DAG's JumpIsExpensive check for x86 (PR23827)

[Sanjay Patel]

Hi hfinkel, chandlerc, nadav,

I think the !isJumpExpensive() optimization in SelectionDAGBuilder is not appropriate for any target given that we're not using any branch predictability information or even asking what the cost of boolean logic ops are relative to branches, but this 1-line patch just disables the transform for x86 to solve PR23827:
https://llvm.org/bugs/show_bug.cgi?id=23827

The proper long-term fix IMO is to make isJumpExpensive() a virtual method rather than a bool so targets can override it to use whatever branch prediction and micro-arch info they have. As noted in PR23827, SimplifyCFG does the opposite transform, and that may be a separate bug.

I've added a command-line override to preserve secondary regression test behavior and to make it easier to measure performance differences going forward. The existing or-branch.ll regression test confirms that we're not splitting the condition logic whereas before it confirmed that we were splitting that logic (creating an extra branch).

http://reviews.llvm.org/D10741

Files:
  lib/Target/X86/X86ISelLowering.cpp
  test/CodeGen/X86/2008-02-18-TailMergingBug.ll
  test/CodeGen/X86/MachineBranchProb.ll
  test/CodeGen/X86/cmov.ll
  test/CodeGen/X86/or-branch.ll
  test/CodeGen/X86/remat-invalid-liveness.ll

Index: lib/Target/X86/X86ISelLowering.cpp
===================================================================
--- lib/Target/X86/X86ISelLowering.cpp
+++ lib/Target/X86/X86ISelLowering.cpp
@@ -67,6 +67,11 @@
              "rather than promotion."),
     cl::Hidden);
 
+static cl::opt<bool> JumpIsExpensiveOverride(
+    "x86-jump-is-expensive", cl::init(true),
+    cl::desc("Do not split complex comparison logic into extra branches."),
+    cl::Hidden);
+
 // Forward declarations.
 static SDValue getMOVL(SelectionDAG &DAG, SDLoc dl, EVT VT, SDValue V1,
                        SDValue V2);
@@ -106,6 +111,11 @@
       addBypassSlowDiv(64, 16);
   }
 
+  // Don't undo bitwise concatenations of comparisons. These are created by
+  // the IR optimizer or the program itself with more knowledge about the
+  // predictability of the comparisons than we have.
+  setJumpIsExpensive(JumpIsExpensiveOverride);
+
   if (Subtarget->isTargetKnownWindowsMSVC()) {
     // Setup Windows compiler runtime calls.
     setLibcallName(RTLIB::SDIV_I64, "_alldiv");
Index: test/CodeGen/X86/2008-02-18-TailMergingBug.ll
===================================================================
--- test/CodeGen/X86/2008-02-18-TailMergingBug.ll
+++ test/CodeGen/X86/2008-02-18-TailMergingBug.ll
@@ -1,5 +1,5 @@
 ; REQUIRES: asserts
-; RUN: llc < %s -march=x86 -mcpu=yonah -stats 2>&1 | grep "Number of block tails merged" | grep 16
+; RUN: llc < %s -march=x86 -mcpu=yonah -x86-jump-is-expensive=0 -stats 2>&1 | grep "Number of block tails merged" | grep 16
 ; PR1909
 
 @.str = internal constant [48 x i8] c"transformed bounds: (%.2f, %.2f), (%.2f, %.2f)\0A\00"		; <[48 x i8]*> [#uses=1]
Index: test/CodeGen/X86/MachineBranchProb.ll
===================================================================
--- test/CodeGen/X86/MachineBranchProb.ll
+++ test/CodeGen/X86/MachineBranchProb.ll
@@ -1,4 +1,4 @@
-; RUN: llc < %s -mtriple=x86_64-apple-darwin -print-machineinstrs=expand-isel-pseudos -o /dev/null 2>&1 | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-apple-darwin -print-machineinstrs=expand-isel-pseudos -x86-jump-is-expensive=0 -o /dev/null 2>&1 | FileCheck %s
 
 ;; Make sure a transformation in SelectionDAGBuilder that converts "or + br" to
 ;; two branches correctly updates the branch probability.
Index: test/CodeGen/X86/cmov.ll
===================================================================
--- test/CodeGen/X86/cmov.ll
+++ test/CodeGen/X86/cmov.ll
@@ -91,7 +91,7 @@
 ; CHECK: g_100
 ; CHECK: testb
 ; CHECK-NOT: xor
-; CHECK: setne
+; CHECK: sete
 ; CHECK: testb
 
 func_4.exit.i:                                    ; preds = %bb.i.i.i, %entry
Index: test/CodeGen/X86/or-branch.ll
===================================================================
--- test/CodeGen/X86/or-branch.ll
+++ test/CodeGen/X86/or-branch.ll
@@ -1,6 +1,17 @@
-; RUN: llc < %s -march=x86  | not grep set
+; RUN: llc < %s -march=x86  | FileCheck %s
+
+; Don't break a compound comparison into multiple branches.
+; Either the program or the optimizer has created this pattern,
+; and the backend should not undo it without knowing the
+; predictability of the branches.
 
 define void @foo(i32 %X, i32 %Y, i32 %Z) nounwind {
+; CHECK-LABEL: foo:
+; CHECK:         cmpl $0, {{[0-9]+}}(%esp)
+; CHECK-NEXT:    sete %al
+; CHECK-NEXT:    cmpl $5, %esi
+; CHECK-NEXT:    setl %cl
+; CHECK-NEXT:    orb %al, %cl
 entry:
 	%tmp = tail call i32 (...) @bar( )		; <i32> [#uses=0]
 	%tmp.upgrd.1 = icmp eq i32 %X, 0		; <i1> [#uses=1]
Index: test/CodeGen/X86/remat-invalid-liveness.ll
===================================================================
--- test/CodeGen/X86/remat-invalid-liveness.ll
+++ test/CodeGen/X86/remat-invalid-liveness.ll
@@ -1,4 +1,4 @@
-; RUN: llc %s -mcpu=core2 -o - | FileCheck %s
+; RUN: llc %s -mcpu=core2 -x86-jump-is-expensive=0 -o - | FileCheck %s
 ; This test was failing while tracking the liveness in the register scavenger
 ; during the branching folding pass. The allocation of the subregisters was
 ; incorrect.

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D10741.28486.patch
Type: text/x-patch
Size: 4012 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150625/c8fb5905/attachment.bin>