change DAG's JumpIsExpensive check for x86 (PR23827)

[spatel]

Summary

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).

[chandlerc]

This seems really unlikely to be a good change.

Jump instructions are not expensive on x86 in general. They certainly are not more expensive than things like sete! I think we usually want to use more jumps rather than moving things out of condition registers.

I think the bug report you're trying to fix is flat out incorrect and backwards. I've updated it.

[spatel]

Thanks, Chandler.

From: https://llvm.org/bugs/show_bug.cgi?id=23827#c6
"We should not encode some magical meaning to a bitwise-and"

I'd always assumed that a bitwise op caused type promotion of the bool, so it was a legitimate way to tell the compiler to avoid the branch. I've followed up with a message to the dev lists.

[spatel]

Abandoning. See new patch in D10846 after discussion on the mailing lists and PR23827.

lib/Target/X86/X86ISelLowering.cpp

 STATISTIC(NumTailCalls, "Number of tail calls");
 
 static cl::opt<bool> ExperimentalVectorWideningLegalization(
     "x86-experimental-vector-widening-legalization", cl::init(false),
     cl::desc("Enable an experimental vector type legalization through widening "
              "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);
 
 X86TargetLowering::X86TargetLowering(const X86TargetMachine &TM,
                                      const X86Subtarget &STI)
     : TargetLowering(TM), Subtarget(&STI) {
   X86ScalarSSEf64 = Subtarget->hasSSE2();
X86TargetLowering::X86TargetLowering(const X86TargetMachine &TM,
   // Bypass expensive divides on Atom when compiling with O2.
   if (TM.getOptLevel() >= CodeGenOpt::Default) {
     if (Subtarget->hasSlowDivide32())
       addBypassSlowDiv(32, 8);
     if (Subtarget->hasSlowDivide64() && Subtarget->is64Bit())
       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");
     setLibcallName(RTLIB::UDIV_I64, "_aulldiv");
     setLibcallName(RTLIB::SREM_I64, "_allrem");
     setLibcallName(RTLIB::UREM_I64, "_aullrem");
     setLibcallName(RTLIB::MUL_I64, "_allmul");
     setLibcallCallingConv(RTLIB::SDIV_I64, CallingConv::X86_StdCall);

test/CodeGen/X86/2008-02-18-TailMergingBug.ll

 ; 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]
 
 define void @minmax(float* %result) nounwind optsize {
 entry:
   %tmp2 = load float, float* %result, align 4    ; <float> [#uses=6]
   %tmp4 = getelementptr float, float* %result, i32 2    ; <float*> [#uses=5]

test/CodeGen/X86/MachineBranchProb.ll

-; 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.
 
 @max_regno = common global i32 0, align 4
 
 define void @test(i32* %old, i32 %final) {
 for.cond:

test/CodeGen/X86/cmov.ll

 bb.i.i.i:                                         ; preds = %entry
   %4 = load volatile i8, i8* @g_100, align 1          ; <i8> [#uses=0]
   br label %func_4.exit.i
 
 ; CHECK-LABEL: test4:
 ; CHECK: g_100
 ; CHECK: testb
 ; CHECK-NOT: xor
-; CHECK: setne
+; CHECK: sete
 ; CHECK: testb
 
 func_4.exit.i:                                    ; preds = %bb.i.i.i, %entry
   %.not.i = xor i1 %2, true                       ; <i1> [#uses=1]
   %brmerge.i = or i1 %3, %.not.i                  ; <i1> [#uses=1]
   %.mux.i = select i1 %2, i8 %g_96.promoted.i, i8 0 ; <i8> [#uses=1]
   br i1 %brmerge.i, label %func_1.exit, label %bb.i.i
 

test/CodeGen/X86/or-branch.ll

-; 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]
   %tmp3 = icmp slt i32 %Y, 5    ; <i1> [#uses=1]
   %tmp4 = or i1 %tmp3, %tmp.upgrd.1   ; <i1> [#uses=1]
   br i1 %tmp4, label %cond_true, label %UnifiedReturnBlock
 
 cond_true:    ; preds = %entry
   %tmp5 = tail call i32 (...) @bar( )    ; <i32> [#uses=0]
   ret void
 
 UnifiedReturnBlock:   ; preds = %entry
   ret void
 }
 
 declare i32 @bar(...)

test/CodeGen/X86/remat-invalid-liveness.ll

-; 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.
 ; I.e., the faulty pattern looked like:
 ; CH = movb 64
 ; ECX = movl 3 <- CH was killed here.
 ; CH = subb CH, ...
 ;