[llvm] r258927 - Revert "Allow X86::COND_NE_OR_P and X86::COND_NP_OR_E to be reversed."
Benjamin Kramer via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 27 04:44:14 PST 2016
Author: d0k
Date: Wed Jan 27 06:44:12 2016
New Revision: 258927
URL: http://llvm.org/viewvc/llvm-project?rev=258927&view=rev
Log:
Revert "Allow X86::COND_NE_OR_P and X86::COND_NP_OR_E to be reversed."
and "Add a missing test case for r258847."
This reverts commit r258847, r258848. Causes miscompilations and backend
errors.
Removed:
llvm/trunk/test/CodeGen/X86/x86-analyze-branch-jne-jp.ll
Modified:
llvm/trunk/lib/Target/X86/X86InstrInfo.cpp
llvm/trunk/lib/Target/X86/X86InstrInfo.h
llvm/trunk/test/CodeGen/X86/block-placement.ll
llvm/trunk/test/CodeGen/X86/fast-isel-cmp-branch2.ll
llvm/trunk/test/CodeGen/X86/fast-isel-cmp-branch3.ll
llvm/trunk/test/CodeGen/X86/fp-une-cmp.ll
Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.cpp?rev=258927&r1=258926&r2=258927&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86InstrInfo.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86InstrInfo.cpp Wed Jan 27 06:44:12 2016
@@ -3805,10 +3805,6 @@ X86::CondCode X86::GetOppositeBranchCond
case X86::COND_NP: return X86::COND_P;
case X86::COND_O: return X86::COND_NO;
case X86::COND_NO: return X86::COND_O;
- case X86::COND_NE_OR_P: return X86::COND_E_AND_NP;
- case X86::COND_NP_OR_E: return X86::COND_P_AND_NE;
- case X86::COND_E_AND_NP: return X86::COND_NE_OR_P;
- case X86::COND_P_AND_NE: return X86::COND_NP_OR_E;
}
}
@@ -4002,9 +3998,9 @@ bool X86InstrInfo::AnalyzeBranchImpl(
MachineBasicBlock::iterator OldInst = I;
BuildMI(MBB, UnCondBrIter, MBB.findDebugLoc(I), get(JNCC))
- .addMBB(UnCondBrIter->getOperand(0).getMBB());
+ .addMBB(UnCondBrIter->getOperand(0).getMBB());
BuildMI(MBB, UnCondBrIter, MBB.findDebugLoc(I), get(X86::JMP_1))
- .addMBB(TargetBB);
+ .addMBB(TargetBB);
OldInst->eraseFromParent();
UnCondBrIter->eraseFromParent();
@@ -4028,6 +4024,11 @@ bool X86InstrInfo::AnalyzeBranchImpl(
assert(Cond.size() == 1);
assert(TBB);
+ // Only handle the case where all conditional branches branch to the same
+ // destination.
+ if (TBB != I->getOperand(0).getMBB())
+ return true;
+
// If the conditions are the same, we can leave them alone.
X86::CondCode OldBranchCode = (X86::CondCode)Cond[0].getImm();
if (OldBranchCode == BranchCode)
@@ -4036,40 +4037,17 @@ bool X86InstrInfo::AnalyzeBranchImpl(
// If they differ, see if they fit one of the known patterns. Theoretically,
// we could handle more patterns here, but we shouldn't expect to see them
// if instruction selection has done a reasonable job.
- auto NewTBB = I->getOperand(0).getMBB();
- if (TBB == NewTBB &&
- ((OldBranchCode == X86::COND_NP && BranchCode == X86::COND_E) ||
- (OldBranchCode == X86::COND_E && BranchCode == X86::COND_NP))) {
+ if ((OldBranchCode == X86::COND_NP &&
+ BranchCode == X86::COND_E) ||
+ (OldBranchCode == X86::COND_E &&
+ BranchCode == X86::COND_NP))
BranchCode = X86::COND_NP_OR_E;
- } else if (TBB == NewTBB &&
- ((OldBranchCode == X86::COND_P && BranchCode == X86::COND_NE) ||
- (OldBranchCode == X86::COND_NE && BranchCode == X86::COND_P))) {
+ else if ((OldBranchCode == X86::COND_P &&
+ BranchCode == X86::COND_NE) ||
+ (OldBranchCode == X86::COND_NE &&
+ BranchCode == X86::COND_P))
BranchCode = X86::COND_NE_OR_P;
- } else if ((OldBranchCode == X86::COND_NE && BranchCode == X86::COND_NP) ||
- (OldBranchCode == X86::COND_P && BranchCode == X86::COND_E)) {
- // X86::COND_P_AND_NE usually has two different branch destinations.
- //
- // JNP B1
- // JNE B2
- // B1: (fall-through)
- // B2:
- //
- // Here this condition branches to B2 only if P && NE. It has another
- // equivalent form:
- //
- // JE B1
- // JP B2
- // B1: (fall-through)
- // B2:
- //
- // Similarly it branches to B2 only if NE && P. That is why this condition
- // is named COND_P_AND_NE.
- BranchCode = X86::COND_P_AND_NE;
- } else if ((OldBranchCode == X86::COND_NP && BranchCode == X86::COND_NE) ||
- (OldBranchCode == X86::COND_E && BranchCode == X86::COND_P)) {
- // See comments above for X86::COND_P_AND_NE.
- BranchCode = X86::COND_E_AND_NP;
- } else
+ else
return true;
// Update the MachineOperand.
@@ -4178,13 +4156,6 @@ unsigned X86InstrInfo::RemoveBranch(Mach
return Count;
}
-static MachineBasicBlock *getFallThroughMBB(MachineBasicBlock *MBB) {
- auto I = std::next(MBB->getIterator());
- if (I == MBB->getParent()->end())
- return nullptr;
- return &*I;
-}
-
unsigned
X86InstrInfo::InsertBranch(MachineBasicBlock &MBB, MachineBasicBlock *TBB,
MachineBasicBlock *FBB, ArrayRef<MachineOperand> Cond,
@@ -4201,9 +4172,6 @@ X86InstrInfo::InsertBranch(MachineBasicB
return 1;
}
- // If FBB is null, it is implied to be a fall-through block.
- bool FallThru = FBB == nullptr;
-
// Conditional branch.
unsigned Count = 0;
X86::CondCode CC = (X86::CondCode)Cond[0].getImm();
@@ -4222,39 +4190,13 @@ X86InstrInfo::InsertBranch(MachineBasicB
BuildMI(&MBB, DL, get(X86::JP_1)).addMBB(TBB);
++Count;
break;
- case X86::COND_P_AND_NE:
- // Use the next block of MBB as FBB if it is null.
- if (FBB == nullptr) {
- FBB = getFallThroughMBB(&MBB);
- assert(FBB && "MBB cannot be the last block in function when the false "
- "body is a fall-through.");
- }
- // Synthesize NEG_NP_OR_E with two branches.
- BuildMI(&MBB, DL, get(X86::JNP_1)).addMBB(FBB);
- ++Count;
- BuildMI(&MBB, DL, get(X86::JNE_1)).addMBB(TBB);
- ++Count;
- break;
- case X86::COND_E_AND_NP:
- // Use the next block of MBB as FBB if it is null.
- if (FBB == nullptr) {
- FBB = getFallThroughMBB(&MBB);
- assert(FBB && "MBB cannot be the last block in function when the false "
- "body is a fall-through.");
- }
- // Synthesize NEG_NE_OR_P with two branches.
- BuildMI(&MBB, DL, get(X86::JNE_1)).addMBB(FBB);
- ++Count;
- BuildMI(&MBB, DL, get(X86::JNP_1)).addMBB(TBB);
- ++Count;
- break;
default: {
unsigned Opc = GetCondBranchFromCond(CC);
BuildMI(&MBB, DL, get(Opc)).addMBB(TBB);
++Count;
}
}
- if (!FallThru) {
+ if (FBB) {
// Two-way Conditional branch. Insert the second branch.
BuildMI(&MBB, DL, get(X86::JMP_1)).addMBB(FBB);
++Count;
@@ -6775,6 +6717,8 @@ bool X86InstrInfo::
ReverseBranchCondition(SmallVectorImpl<MachineOperand> &Cond) const {
assert(Cond.size() == 1 && "Invalid X86 branch condition!");
X86::CondCode CC = static_cast<X86::CondCode>(Cond[0].getImm());
+ if (CC == X86::COND_NE_OR_P || CC == X86::COND_NP_OR_E)
+ return true;
Cond[0].setImm(GetOppositeBranchCondition(CC));
return false;
}
Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.h?rev=258927&r1=258926&r2=258927&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86InstrInfo.h (original)
+++ llvm/trunk/lib/Target/X86/X86InstrInfo.h Wed Jan 27 06:44:12 2016
@@ -29,44 +29,35 @@ namespace llvm {
namespace X86 {
// X86 specific condition code. These correspond to X86_*_COND in
// X86InstrInfo.td. They must be kept in synch.
-enum CondCode {
- COND_A = 0,
- COND_AE = 1,
- COND_B = 2,
- COND_BE = 3,
- COND_E = 4,
- COND_G = 5,
- COND_GE = 6,
- COND_L = 7,
- COND_LE = 8,
- COND_NE = 9,
- COND_NO = 10,
- COND_NP = 11,
- COND_NS = 12,
- COND_O = 13,
- COND_P = 14,
- COND_S = 15,
- LAST_VALID_COND = COND_S,
+ enum CondCode {
+ COND_A = 0,
+ COND_AE = 1,
+ COND_B = 2,
+ COND_BE = 3,
+ COND_E = 4,
+ COND_G = 5,
+ COND_GE = 6,
+ COND_L = 7,
+ COND_LE = 8,
+ COND_NE = 9,
+ COND_NO = 10,
+ COND_NP = 11,
+ COND_NS = 12,
+ COND_O = 13,
+ COND_P = 14,
+ COND_S = 15,
+ LAST_VALID_COND = COND_S,
- // Artificial condition codes. These are used by AnalyzeBranch
- // to indicate a block terminated with two conditional branches to
- // the same location. This occurs in code using FCMP_OEQ or FCMP_UNE,
- // which can't be represented on x86 with a single condition. These
- // are never used in MachineInstrs.
- COND_NE_OR_P,
- COND_NP_OR_E,
+ // Artificial condition codes. These are used by AnalyzeBranch
+ // to indicate a block terminated with two conditional branches to
+ // the same location. This occurs in code using FCMP_OEQ or FCMP_UNE,
+ // which can't be represented on x86 with a single condition. These
+ // are never used in MachineInstrs.
+ COND_NE_OR_P,
+ COND_NP_OR_E,
- // Artificial condition codes. These are used to represent the negation of
- // above two conditions. The only scenario we need these two conditions is
- // when we try to reverse above two conditions in order to remove redundant
- // unconditional jumps. Note that both true and false bodies need to be
- // avaiable in order to correctly synthesize instructions for them. These are
- // never used in MachineInstrs.
- COND_E_AND_NP, // negate of COND_NE_OR_P
- COND_P_AND_NE, // negate of COND_NP_OR_E
-
- COND_INVALID
-};
+ COND_INVALID
+ };
// Turn condition code into conditional branch opcode.
unsigned GetCondBranchFromCond(CondCode CC);
Modified: llvm/trunk/test/CodeGen/X86/block-placement.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/block-placement.ll?rev=258927&r1=258926&r2=258927&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/block-placement.ll (original)
+++ llvm/trunk/test/CodeGen/X86/block-placement.ll Wed Jan 27 06:44:12 2016
@@ -463,23 +463,26 @@ exit:
}
define void @fpcmp_unanalyzable_branch(i1 %cond) {
-; This function's CFG contains an once-unanalyzable branch (une on floating
-; points). As now it becomes analyzable, we should get best layout in which each
-; edge in 'entry' -> 'entry.if.then_crit_edge' -> 'if.then' -> 'if.end' is
-; fall-through.
+; This function's CFG contains an unanalyzable branch that is likely to be
+; split due to having a different high-probability predecessor.
; CHECK: fpcmp_unanalyzable_branch
; CHECK: %entry
-; CHECK: %entry.if.then_crit_edge
-; CHECK: %if.then
-; CHECK: %if.end
; CHECK: %exit
+; CHECK-NOT: %if.then
+; CHECK-NOT: %if.end
+; CHECK-NOT: jne
+; CHECK-NOT: jnp
; CHECK: jne
; CHECK-NEXT: jnp
+; CHECK-NEXT: %if.then
entry:
; Note that this branch must be strongly biased toward
; 'entry.if.then_crit_edge' to ensure that we would try to form a chain for
-; 'entry' -> 'entry.if.then_crit_edge' -> 'if.then' -> 'if.end'.
+; 'entry' -> 'entry.if.then_crit_edge' -> 'if.then'. It is the last edge in that
+; chain which would violate the unanalyzable branch in 'exit', but we won't even
+; try this trick unless 'if.then' is believed to almost always be reached from
+; 'entry.if.then_crit_edge'.
br i1 %cond, label %entry.if.then_crit_edge, label %lor.lhs.false, !prof !1
entry.if.then_crit_edge:
@@ -491,7 +494,7 @@ lor.lhs.false:
exit:
%cmp.i = fcmp une double 0.000000e+00, undef
- br i1 %cmp.i, label %if.then, label %if.end, !prof !3
+ br i1 %cmp.i, label %if.then, label %if.end
if.then:
%0 = phi i8 [ %.pre14, %entry.if.then_crit_edge ], [ undef, %exit ]
@@ -504,7 +507,6 @@ if.end:
}
!1 = !{!"branch_weights", i32 1000, i32 1}
-!3 = !{!"branch_weights", i32 1, i32 1000}
declare i32 @f()
declare i32 @g()
@@ -663,14 +665,11 @@ define void @unanalyzable_branch_to_best
; Ensure that we can handle unanalyzable branches where the destination block
; gets selected as the optimal successor to merge.
;
-; This branch is now analyzable and hence the destination block becomes the
-; hotter one. The right order is entry->bar->exit->foo.
-;
; CHECK: unanalyzable_branch_to_best_succ
; CHECK: %entry
+; CHECK: %foo
; CHECK: %bar
; CHECK: %exit
-; CHECK: %foo
entry:
; Bias this branch toward bar to ensure we form that chain.
Modified: llvm/trunk/test/CodeGen/X86/fast-isel-cmp-branch2.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fast-isel-cmp-branch2.ll?rev=258927&r1=258926&r2=258927&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/fast-isel-cmp-branch2.ll (original)
+++ llvm/trunk/test/CodeGen/X86/fast-isel-cmp-branch2.ll Wed Jan 27 06:44:12 2016
@@ -5,7 +5,7 @@ define i32 @fcmp_oeq(float %x, float %y)
; CHECK-LABEL: fcmp_oeq
; CHECK: ucomiss %xmm1, %xmm0
; CHECK-NEXT: jne {{LBB.+_1}}
-; CHECK-NEXT: jp {{LBB.+_1}}
+; CHECK-NEXT: jnp {{LBB.+_2}}
%1 = fcmp oeq float %x, %y
br i1 %1, label %bb1, label %bb2
bb2:
@@ -162,7 +162,8 @@ define i32 @fcmp_une(float %x, float %y)
; CHECK-LABEL: fcmp_une
; CHECK: ucomiss %xmm1, %xmm0
; CHECK-NEXT: jne {{LBB.+_2}}
-; CHECK-NEXT: jnp {{LBB.+_1}}
+; CHECK-NEXT: jp {{LBB.+_2}}
+; CHECK-NEXT: jmp {{LBB.+_1}}
%1 = fcmp une float %x, %y
br i1 %1, label %bb1, label %bb2
bb2:
Modified: llvm/trunk/test/CodeGen/X86/fast-isel-cmp-branch3.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fast-isel-cmp-branch3.ll?rev=258927&r1=258926&r2=258927&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/fast-isel-cmp-branch3.ll (original)
+++ llvm/trunk/test/CodeGen/X86/fast-isel-cmp-branch3.ll Wed Jan 27 06:44:12 2016
@@ -17,7 +17,7 @@ define i32 @fcmp_oeq2(float %x) {
; CHECK: xorps %xmm1, %xmm1
; CHECK-NEXT: ucomiss %xmm1, %xmm0
; CHECK-NEXT: jne {{LBB.+_1}}
-; CHECK-NEXT: jp {{LBB.+_1}}
+; CHECK-NEXT: jnp {{LBB.+_2}}
%1 = fcmp oeq float %x, 0.000000e+00
br i1 %1, label %bb1, label %bb2
bb2:
@@ -338,7 +338,8 @@ define i32 @fcmp_une2(float %x) {
; CHECK: xorps %xmm1, %xmm1
; CHECK-NEXT: ucomiss %xmm1, %xmm0
; CHECK-NEXT: jne {{LBB.+_2}}
-; CHECK-NEXT: jnp {{LBB.+_1}}
+; CHECK-NEXT: jp {{LBB.+_2}}
+; CHECK-NEXT: jmp {{LBB.+_1}}
%1 = fcmp une float %x, 0.000000e+00
br i1 %1, label %bb1, label %bb2
bb2:
Modified: llvm/trunk/test/CodeGen/X86/fp-une-cmp.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/fp-une-cmp.ll?rev=258927&r1=258926&r2=258927&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/fp-une-cmp.ll (original)
+++ llvm/trunk/test/CodeGen/X86/fp-une-cmp.ll Wed Jan 27 06:44:12 2016
@@ -19,43 +19,18 @@
; addsd ...
; LBB0_2:
-define float @func1(float %x, float %y) nounwind readnone optsize ssp {
-; CHECK: func1
+; CHECK: func
; CHECK: jne [[LABEL:.*]]
; CHECK-NEXT: jp [[LABEL]]
; CHECK-NOT: jmp
-;
-entry:
- %0 = fpext float %x to double
- %1 = fpext float %y to double
- %2 = fmul double %0, %1
- %3 = fcmp une double %2, 0.000000e+00
- br i1 %3, label %bb2, label %bb1
-
-bb1:
- %4 = fadd double %2, -1.000000e+00
- br label %bb2
-bb2:
- %.0.in = phi double [ %4, %bb1 ], [ %2, %entry ]
- %.0 = fptrunc double %.0.in to float
- ret float %.0
-}
-
-define float @func2(float %x, float %y) nounwind readnone optsize ssp {
-; CHECK: func2
-; CHECK: jne [[LABEL:.*]]
-; CHECK-NEXT: jp [[LABEL]]
-; CHECK: %bb2
-; CHECK: %bb1
-; CHECK: jmp
-;
+define float @func(float %x, float %y) nounwind readnone optsize ssp {
entry:
%0 = fpext float %x to double
%1 = fpext float %y to double
%2 = fmul double %0, %1
%3 = fcmp une double %2, 0.000000e+00
- br i1 %3, label %bb1, label %bb2, !prof !1
+ br i1 %3, label %bb2, label %bb1
bb1:
%4 = fadd double %2, -1.000000e+00
@@ -66,5 +41,3 @@ bb2:
%.0 = fptrunc double %.0.in to float
ret float %.0
}
-
-!1 = !{!"branch_weights", i32 1, i32 1000}
Removed: llvm/trunk/test/CodeGen/X86/x86-analyze-branch-jne-jp.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/x86-analyze-branch-jne-jp.ll?rev=258926&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/X86/x86-analyze-branch-jne-jp.ll (original)
+++ llvm/trunk/test/CodeGen/X86/x86-analyze-branch-jne-jp.ll (removed)
@@ -1,21 +0,0 @@
-; RUN: llc -mcpu=corei7 -mtriple=x86_64-linux < %s | FileCheck %s -check-prefix=CHECK
-
-; Test if the negation of the non-equality check between floating points are
-; translated to jnp followed by jne.
-
-; CHECK: jne
-; CHECK-NEXT: jnp
-define void @foo(float %f) {
-entry:
- %cmp = fcmp une float %f, 0.000000e+00
- br i1 %cmp, label %if.then, label %if.end
-
-if.then:
- tail call void @a()
- br label %if.end
-
-if.end:
- ret void
-}
-
-declare void @a()
More information about the llvm-commits
mailing list