diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp --- a/llvm/lib/Target/X86/X86ISelLowering.cpp +++ b/llvm/lib/Target/X86/X86ISelLowering.cpp @@ -43316,9 +43316,9 @@ static SDValue combineSetCC(SDNode *N, SelectionDAG &DAG, const X86Subtarget &Subtarget) { - ISD::CondCode CC = cast(N->getOperand(2))->get(); - SDValue LHS = N->getOperand(0); - SDValue RHS = N->getOperand(1); + const ISD::CondCode CC = cast(N->getOperand(2))->get(); + const SDValue LHS = N->getOperand(0); + const SDValue RHS = N->getOperand(1); EVT VT = N->getValueType(0); EVT OpVT = LHS.getValueType(); SDLoc DL(N); @@ -43345,30 +43345,35 @@ if (VT.isVector() && VT.getVectorElementType() == MVT::i1 && (CC == ISD::SETNE || CC == ISD::SETEQ || ISD::isSignedIntSetCC(CC))) { - // Put build_vectors on the right. - if (LHS.getOpcode() == ISD::BUILD_VECTOR) { - std::swap(LHS, RHS); - CC = ISD::getSetCCSwappedOperands(CC); + // Using temporaries to avoid messing up operand ordering for later + // transformations if this doesn't work. + SDValue Op0 = LHS; + SDValue Op1 = RHS; + ISD::CondCode TmpCC = CC; + // Put build_vector on the right. + if (Op0.getOpcode() == ISD::BUILD_VECTOR) { + std::swap(Op0, Op1); + TmpCC = ISD::getSetCCSwappedOperands(TmpCC); } bool IsSEXT0 = - (LHS.getOpcode() == ISD::SIGN_EXTEND) && - (LHS.getOperand(0).getValueType().getVectorElementType() == MVT::i1); - bool IsVZero1 = ISD::isBuildVectorAllZeros(RHS.getNode()); + (Op0.getOpcode() == ISD::SIGN_EXTEND) && + (Op0.getOperand(0).getValueType().getVectorElementType() == MVT::i1); + bool IsVZero1 = ISD::isBuildVectorAllZeros(Op1.getNode()); if (IsSEXT0 && IsVZero1) { - assert(VT == LHS.getOperand(0).getValueType() && + assert(VT == Op0.getOperand(0).getValueType() && "Uexpected operand type"); - if (CC == ISD::SETGT) + if (TmpCC == ISD::SETGT) return DAG.getConstant(0, DL, VT); - if (CC == ISD::SETLE) + if (TmpCC == ISD::SETLE) return DAG.getConstant(1, DL, VT); - if (CC == ISD::SETEQ || CC == ISD::SETGE) - return DAG.getNOT(DL, LHS.getOperand(0), VT); + if (TmpCC == ISD::SETEQ || TmpCC == ISD::SETGE) + return DAG.getNOT(DL, Op0.getOperand(0), VT); - assert((CC == ISD::SETNE || CC == ISD::SETLT) && + assert((TmpCC == ISD::SETNE || TmpCC == ISD::SETLT) && "Unexpected condition code!"); - return LHS.getOperand(0); + return Op0.getOperand(0); } } @@ -43381,8 +43386,7 @@ VT.getVectorElementType() == MVT::i1 && (OpVT.getVectorElementType() == MVT::i8 || OpVT.getVectorElementType() == MVT::i16)) { - SDValue Setcc = DAG.getNode(ISD::SETCC, DL, OpVT, LHS, RHS, - N->getOperand(2)); + SDValue Setcc = DAG.getSetCC(DL, OpVT, LHS, RHS, CC); return DAG.getNode(ISD::TRUNCATE, DL, VT, Setcc); } diff --git a/llvm/test/CodeGen/X86/avx512-vec-cmp.ll b/llvm/test/CodeGen/X86/avx512-vec-cmp.ll --- a/llvm/test/CodeGen/X86/avx512-vec-cmp.ll +++ b/llvm/test/CodeGen/X86/avx512-vec-cmp.ll @@ -1532,8 +1532,7 @@ ret void } -; FIXME: This test is currently miscompiled for KNL with the operands of the -; vcmpgtb swapped. +; This test used to end up with the vpcmpgtb on KNL having its operands in the wrong order. define <8 x i64> @cmp_swap_bug(<16 x i8>* %x, <8 x i64> %y, <8 x i64> %z) { ; KNL-LABEL: cmp_swap_bug: ; KNL: ## %bb.0: ## %entry @@ -1542,7 +1541,7 @@ ; KNL-NEXT: ## encoding: [0xc4,0xe2,0x69,0x00,0x15,A,A,A,A] ; KNL-NEXT: ## fixup A - offset: 5, value: LCPI69_0-4, kind: reloc_riprel_4byte ; KNL-NEXT: vpxor %xmm3, %xmm3, %xmm3 ## encoding: [0xc5,0xe1,0xef,0xdb] -; KNL-NEXT: vpcmpgtb %xmm3, %xmm2, %xmm2 ## encoding: [0xc5,0xe9,0x64,0xd3] +; KNL-NEXT: vpcmpgtb %xmm2, %xmm3, %xmm2 ## encoding: [0xc5,0xe1,0x64,0xd2] ; KNL-NEXT: vpmovsxbd %xmm2, %zmm2 ## encoding: [0x62,0xf2,0x7d,0x48,0x21,0xd2] ; KNL-NEXT: vptestmd %zmm2, %zmm2, %k1 ## encoding: [0x62,0xf2,0x6d,0x48,0x27,0xca] ; KNL-NEXT: vpblendmq %zmm0, %zmm1, %zmm0 {%k1} ## encoding: [0x62,0xf2,0xf5,0x49,0x64,0xc0]