[llvm-commits] [llvm] r128175 - in /llvm/trunk: lib/Target/X86/X86ISelLowering.cpp test/CodeGen/X86/long-setcc.ll test/CodeGen/X86/sext-i1.ll
Andrew Trick
atrick at apple.com
Wed Mar 23 16:03:22 PDT 2011
On Mar 23, 2011, at 3:41 PM, Eli Friedman wrote:
> On Wed, Mar 23, 2011 at 3:16 PM, Andrew Trick <atrick at apple.com> wrote:
>> Author: atrick
>> Date: Wed Mar 23 17:16:02 2011
>> New Revision: 128175
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=128175&view=rev
>> Log:
>> Reapply Eli's r127852 now that the pre-RA scheduler can spill EFLAGS.
>> (target-specific branchless method for double-width relational comparisons on x86)
>
> Mmm... I'm still not very happy with the fact that we're generating
> pushfl/popfl in cases like the one I mentioned in
> http://llvm.org/bugs/show_bug.cgi?id=9509#c3 . Especially considering
> that I'm not entirely sure the method we use for copying EFLAGS is
> safe on 64-bit systems.
>
> -Eli
Thanks for verifying this corner case. I noticed the code is fairly bad, although the original before your patch wasn't good either. I debated filing another bug to follow up on the performance, but wasn't sure how much we cared about this case. It only occurred in a self-host builder; no benchmarks.
Of course, if you think we may have a correctness issue, please file another bug.
-Andy
>> Modified:
>> llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>> llvm/trunk/test/CodeGen/X86/long-setcc.ll
>> llvm/trunk/test/CodeGen/X86/sext-i1.ll
>>
>> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=128175&r1=128174&r2=128175&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
>> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Wed Mar 23 17:16:02 2011
>> @@ -447,12 +447,13 @@
>> setOperationAction(ISD::SETCC , MVT::i8 , Custom);
>> setOperationAction(ISD::SETCC , MVT::i16 , Custom);
>> setOperationAction(ISD::SETCC , MVT::i32 , Custom);
>> + setOperationAction(ISD::SETCC , MVT::i64 , Custom);
>> setOperationAction(ISD::SETCC , MVT::f32 , Custom);
>> setOperationAction(ISD::SETCC , MVT::f64 , Custom);
>> setOperationAction(ISD::SETCC , MVT::f80 , Custom);
>> if (Subtarget->is64Bit()) {
>> setOperationAction(ISD::SELECT , MVT::i64 , Custom);
>> - setOperationAction(ISD::SETCC , MVT::i64 , Custom);
>> + setOperationAction(ISD::SETCC , MVT::i128 , Custom);
>> }
>> setOperationAction(ISD::EH_RETURN , MVT::Other, Custom);
>>
>> @@ -2853,7 +2854,7 @@
>> } else if (SetCCOpcode == ISD::SETLT && RHSC->isNullValue()) {
>> // X < 0 -> X == 0, jump on sign.
>> return X86::COND_S;
>> - } else if (SetCCOpcode == ISD::SETLT && RHSC->getZExtValue() == 1) {
>> + } else if (SetCCOpcode == ISD::SETLT && RHSC->isOne()) {
>> // X < 1 -> X <= 0
>> RHS = DAG.getConstant(0, RHS.getValueType());
>> return X86::COND_LE;
>> @@ -7436,7 +7437,8 @@
>> // Lower (X & (1 << N)) == 0 to BT(X, N).
>> // Lower ((X >>u N) & 1) != 0 to BT(X, N).
>> // Lower ((X >>s N) & 1) != 0 to BT(X, N).
>> - if (Op0.getOpcode() == ISD::AND && Op0.hasOneUse() &&
>> + if (isTypeLegal(Op0.getValueType()) &&
>> + Op0.getOpcode() == ISD::AND && Op0.hasOneUse() &&
>> Op1.getOpcode() == ISD::Constant &&
>> cast<ConstantSDNode>(Op1)->isNullValue() &&
>> (CC == ISD::SETEQ || CC == ISD::SETNE)) {
>> @@ -7448,7 +7450,7 @@
>> // Look for X == 0, X == 1, X != 0, or X != 1. We can simplify some forms of
>> // these.
>> if (Op1.getOpcode() == ISD::Constant &&
>> - (cast<ConstantSDNode>(Op1)->getZExtValue() == 1 ||
>> + (cast<ConstantSDNode>(Op1)->isOne() ||
>> cast<ConstantSDNode>(Op1)->isNullValue()) &&
>> (CC == ISD::SETEQ || CC == ISD::SETNE)) {
>>
>> @@ -7471,6 +7473,73 @@
>> if (X86CC == X86::COND_INVALID)
>> return SDValue();
>>
>> + if ((!Subtarget->is64Bit() && Op0.getValueType() == MVT::i64) ||
>> + (Subtarget->is64Bit() && Op0.getValueType() == MVT::i128)) {
>> + switch (X86CC) {
>> + case X86::COND_E:
>> + case X86::COND_NE:
>> + case X86::COND_S:
>> + case X86::COND_NS:
>> + // Just use the generic lowering, which works well on x86.
>> + return SDValue();
>> + case X86::COND_B:
>> + case X86::COND_AE:
>> + case X86::COND_L:
>> + case X86::COND_GE:
>> + // Use SBB-based lowering.
>> + break;
>> + case X86::COND_A:
>> + // Use SBB-based lowering; commute so ZF isn't used.
>> + X86CC = X86::COND_B;
>> + std::swap(Op0, Op1);
>> + break;
>> + case X86::COND_BE:
>> + // Use SBB-based lowering; commute so ZF isn't used.
>> + X86CC = X86::COND_AE;
>> + std::swap(Op0, Op1);
>> + break;
>> + case X86::COND_G:
>> + // Use SBB-based lowering; commute so ZF isn't used.
>> + X86CC = X86::COND_L;
>> + std::swap(Op0, Op1);
>> + break;
>> + case X86::COND_LE:
>> + // Use SBB-based lowering; commute so ZF isn't used.
>> + X86CC = X86::COND_GE;
>> + std::swap(Op0, Op1);
>> + break;
>> + default:
>> + assert(0 && "Unexpected X86CC.");
>> + return SDValue();
>> + }
>> + MVT HalfType = getPointerTy();
>> + // FIXME: Refactor this code out to implement ISD::SADDO and friends.
>> + SDValue Op0Low = DAG.getNode(ISD::EXTRACT_ELEMENT, dl, HalfType,
>> + Op0, DAG.getIntPtrConstant(0));
>> + SDValue Op1Low = DAG.getNode(ISD::EXTRACT_ELEMENT, dl, HalfType,
>> + Op1, DAG.getIntPtrConstant(0));
>> + SDValue Op0High = DAG.getNode(ISD::EXTRACT_ELEMENT, dl, HalfType,
>> + Op0, DAG.getIntPtrConstant(1));
>> + SDValue Op1High = DAG.getNode(ISD::EXTRACT_ELEMENT, dl, HalfType,
>> + Op1, DAG.getIntPtrConstant(1));
>> + // Redirect some cases which will simplify to the generic expansion;
>> + // X86ISD::SUB and X86ISD::SBB are not optimized well at the moment.
>> + // FIXME: We really need to add DAGCombines for SUB/SBB/etc.
>> + if (Op1Low.getOpcode() == ISD::Constant &&
>> + cast<ConstantSDNode>(Op1Low)->isNullValue())
>> + return SDValue();
>> + if (Op0Low.getOpcode() == ISD::Constant &&
>> + cast<ConstantSDNode>(Op0Low)->isAllOnesValue())
>> + return SDValue();
>> + SDValue res1, res2;
>> + SDVTList VTList = DAG.getVTList(HalfType, MVT::i32);
>> + res1 = DAG.getNode(X86ISD::SUB, dl, VTList, Op0Low, Op1Low).getValue(1);
>> + res2 = DAG.getNode(X86ISD::SBB, dl, VTList, Op0High, Op1High,
>> + res1).getValue(1);
>> + return DAG.getNode(X86ISD::SETCC, dl, MVT::i8,
>> + DAG.getConstant(X86CC, MVT::i8), res2);
>> + }
>> +
>> SDValue EFLAGS = EmitCmp(Op0, Op1, X86CC, DAG);
>> return DAG.getNode(X86ISD::SETCC, dl, MVT::i8,
>> DAG.getConstant(X86CC, MVT::i8), EFLAGS);
>>
>> Modified: llvm/trunk/test/CodeGen/X86/long-setcc.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/long-setcc.ll?rev=128175&r1=128174&r2=128175&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/long-setcc.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/long-setcc.ll Wed Mar 23 17:16:02 2011
>> @@ -1,18 +1,35 @@
>> -; RUN: llc < %s -march=x86 | grep cmp | count 1
>> -; RUN: llc < %s -march=x86 | grep shr | count 1
>> -; RUN: llc < %s -march=x86 | grep xor | count 1
>> +; RUN: llc < %s -march=x86 | FileCheck %s
>>
>> -define i1 @t1(i64 %x) nounwind {
>> - %B = icmp slt i64 %x, 0
>> - ret i1 %B
>> +; General case
>> +define i1 @t1(i64 %x, i64 %y) nounwind {
>> +; CHECK: @t1
>> +; CHECK: subl
>> +; CHECK: sbbl
>> +; CHECK: setl %al
>> + %B = icmp slt i64 %x, %y
>> + ret i1 %B
>> }
>>
>> +; Some special cases
>> define i1 @t2(i64 %x) nounwind {
>> - %tmp = icmp ult i64 %x, 4294967296
>> - ret i1 %tmp
>> +; CHECK: @t2
>> +; CHECK: shrl $31, %eax
>> + %B = icmp slt i64 %x, 0
>> + ret i1 %B
>> }
>>
>> -define i1 @t3(i32 %x) nounwind {
>> - %tmp = icmp ugt i32 %x, -1
>> - ret i1 %tmp
>> +define i1 @t3(i64 %x) nounwind {
>> +; CHECK: @t3
>> +; CHECX: cmpl $0
>> +; CHECX: sete %al
>> + %tmp = icmp ult i64 %x, 4294967296
>> + ret i1 %tmp
>> +}
>> +
>> +define i1 @t4(i64 %x) nounwind {
>> +; CHECK: @t4
>> +; CHECX: cmpl $0
>> +; CHECX: setne %al
>> + %tmp = icmp ugt i64 %x, 4294967295
>> + ret i1 %tmp
>> }
>>
>> Modified: llvm/trunk/test/CodeGen/X86/sext-i1.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/sext-i1.ll?rev=128175&r1=128174&r2=128175&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/X86/sext-i1.ll (original)
>> +++ llvm/trunk/test/CodeGen/X86/sext-i1.ll Wed Mar 23 17:16:02 2011
>> @@ -39,7 +39,8 @@
>> ; 32: t3:
>> ; 32: cmpl $1
>> ; 32: sbbl
>> -; 32: cmpl
>> +; 32: subl
>> +; 32: sbbl
>> ; 32: xorl
>>
>> ; 64: t3:
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
More information about the llvm-commits
mailing list