From: Sanjay Patel Date: Sun, 10 Feb 2019 14:29:57 +0000 (+0000) Subject: [TargetLowering] refactor setcc folds to fix another miscompile (PR40657) X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0c9f4335a38b922268c1373ed98326845f3d1a23;p=llvm [TargetLowering] refactor setcc folds to fix another miscompile (PR40657) SimplifySetCC still has much room for improvement, but this should fix the remaining problem examples from: https://bugs.llvm.org/show_bug.cgi?id=40657 The initial fix for this problem was rL353615. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@353639 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/CodeGen/TargetLowering.h b/include/llvm/CodeGen/TargetLowering.h index 31d43433c55..7bb5944d99a 100644 --- a/include/llvm/CodeGen/TargetLowering.h +++ b/include/llvm/CodeGen/TargetLowering.h @@ -3919,9 +3919,10 @@ public: SDValue lowerCmpEqZeroToCtlzSrl(SDValue Op, SelectionDAG &DAG) const; private: - SDValue simplifySetCCWithAnd(EVT VT, SDValue N0, SDValue N1, - ISD::CondCode Cond, DAGCombinerInfo &DCI, - const SDLoc &DL) const; + SDValue foldSetCCWithAnd(EVT VT, SDValue N0, SDValue N1, ISD::CondCode Cond, + const SDLoc &DL, DAGCombinerInfo &DCI) const; + SDValue foldSetCCWithBinOp(EVT VT, SDValue N0, SDValue N1, ISD::CondCode Cond, + const SDLoc &DL, DAGCombinerInfo &DCI) const; SDValue optimizeSetCCOfSignedTruncationCheck(EVT SCCVT, SDValue N0, SDValue N1, ISD::CondCode Cond, diff --git a/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/lib/CodeGen/SelectionDAG/TargetLowering.cpp index eae5a45906f..370e0d5a088 100644 --- a/lib/CodeGen/SelectionDAG/TargetLowering.cpp +++ b/lib/CodeGen/SelectionDAG/TargetLowering.cpp @@ -2136,10 +2136,9 @@ bool TargetLowering::isExtendedTrueVal(const ConstantSDNode *N, EVT VT, /// This helper function of SimplifySetCC tries to optimize the comparison when /// either operand of the SetCC node is a bitwise-and instruction. -SDValue TargetLowering::simplifySetCCWithAnd(EVT VT, SDValue N0, SDValue N1, - ISD::CondCode Cond, - DAGCombinerInfo &DCI, - const SDLoc &DL) const { +SDValue TargetLowering::foldSetCCWithAnd(EVT VT, SDValue N0, SDValue N1, + ISD::CondCode Cond, const SDLoc &DL, + DAGCombinerInfo &DCI) const { // Match these patterns in any of their permutations: // (X & Y) == Y // (X & Y) != Y @@ -2292,6 +2291,49 @@ SDValue TargetLowering::optimizeSetCCOfSignedTruncationCheck( return T2; } +/// Try to fold an equality comparison with a {add/sub/xor} binary operation as +/// the 1st operand (N0). Callers are expected to swap the N0/N1 parameters to +/// handle the commuted versions of these patterns. +SDValue TargetLowering::foldSetCCWithBinOp(EVT VT, SDValue N0, SDValue N1, + ISD::CondCode Cond, const SDLoc &DL, + DAGCombinerInfo &DCI) const { + unsigned BOpcode = N0.getOpcode(); + assert((BOpcode == ISD::ADD || BOpcode == ISD::SUB || BOpcode == ISD::XOR) && + "Unexpected binop"); + assert((Cond == ISD::SETEQ || Cond == ISD::SETNE) && "Unexpected condcode"); + + // (X + Y) == X --> Y == 0 + // (X - Y) == X --> Y == 0 + // (X ^ Y) == X --> Y == 0 + SelectionDAG &DAG = DCI.DAG; + EVT OpVT = N0.getValueType(); + SDValue X = N0.getOperand(0); + SDValue Y = N0.getOperand(1); + if (X == N1) + return DAG.getSetCC(DL, VT, Y, DAG.getConstant(0, DL, OpVT), Cond); + + if (Y != N1) + return SDValue(); + + // (X + Y) == Y --> X == 0 + // (X ^ Y) == Y --> X == 0 + if (BOpcode == ISD::ADD || BOpcode == ISD::XOR) + return DAG.getSetCC(DL, VT, X, DAG.getConstant(0, DL, OpVT), Cond); + + // The shift would not be valid if the operands are boolean (i1). + if (!N0.hasOneUse() || OpVT.getScalarSizeInBits() == 1) + return SDValue(); + + // (X - Y) == Y --> X == Y << 1 + EVT ShiftVT = getShiftAmountTy(OpVT, DAG.getDataLayout(), + !DCI.isBeforeLegalize()); + SDValue One = DAG.getConstant(1, DL, ShiftVT); + SDValue YShl1 = DAG.getNode(ISD::SHL, DL, N1.getValueType(), Y, One); + if (!DCI.isCalledByLegalizer()) + DCI.AddToWorklist(YShl1.getNode()); + return DAG.getSetCC(DL, VT, X, YShl1, Cond); +} + /// Try to simplify a setcc built with the specified operands and cc. If it is /// unable to simplify it, return a null SDValue. SDValue TargetLowering::SimplifySetCC(EVT VT, SDValue N0, SDValue N1, @@ -3061,63 +3103,21 @@ SDValue TargetLowering::SimplifySetCC(EVT VT, SDValue N0, SDValue N1, LegalRHSImm = isLegalICmpImmediate(RHSC->getSExtValue()); } - // Simplify (X+Z) == X --> Z == 0 + // (X+Y) == X --> Y == 0 and similar folds. // Don't do this if X is an immediate that can fold into a cmp - // instruction and X+Z has other uses. It could be an induction variable + // instruction and X+Y has other uses. It could be an induction variable // chain, and the transform would increase register pressure. - if (!LegalRHSImm || N0.getNode()->hasOneUse()) { - if (N0.getOperand(0) == N1) - return DAG.getSetCC(dl, VT, N0.getOperand(1), - DAG.getConstant(0, dl, N0.getValueType()), Cond); - if (N0.getOperand(1) == N1) { - if (isCommutativeBinOp(N0.getOpcode())) - return DAG.getSetCC(dl, VT, N0.getOperand(0), - DAG.getConstant(0, dl, N0.getValueType()), - Cond); - // The shift is not valid if this is a bool (i1). - if (N0.getNode()->hasOneUse() && OpVT.getScalarSizeInBits() != 1) { - assert(N0.getOpcode() == ISD::SUB && "Unexpected operation!"); - auto &DL = DAG.getDataLayout(); - // (Z-X) == X --> Z == X<<1 - SDValue SH = DAG.getNode( - ISD::SHL, dl, N1.getValueType(), N1, - DAG.getConstant(1, dl, - getShiftAmountTy(N1.getValueType(), DL, - !DCI.isBeforeLegalize()))); - if (!DCI.isCalledByLegalizer()) - DCI.AddToWorklist(SH.getNode()); - return DAG.getSetCC(dl, VT, N0.getOperand(0), SH, Cond); - } - } - } + if (!LegalRHSImm || N0.hasOneUse()) + if (SDValue V = foldSetCCWithBinOp(VT, N0, N1, Cond, dl, DCI)) + return V; } if (N1.getOpcode() == ISD::ADD || N1.getOpcode() == ISD::SUB || - N1.getOpcode() == ISD::XOR) { - // Simplify X == (X+Z) --> Z == 0 - if (N1.getOperand(0) == N0) - return DAG.getSetCC(dl, VT, N1.getOperand(1), - DAG.getConstant(0, dl, N1.getValueType()), Cond); - if (N1.getOperand(1) == N0) { - if (isCommutativeBinOp(N1.getOpcode())) - return DAG.getSetCC(dl, VT, N1.getOperand(0), - DAG.getConstant(0, dl, N1.getValueType()), Cond); - if (N1.getNode()->hasOneUse()) { - assert(N1.getOpcode() == ISD::SUB && "Unexpected operation!"); - auto &DL = DAG.getDataLayout(); - // X == (Z-X) --> X<<1 == Z - SDValue SH = DAG.getNode( - ISD::SHL, dl, N1.getValueType(), N0, - DAG.getConstant(1, dl, getShiftAmountTy(N0.getValueType(), DL, - !DCI.isBeforeLegalize()))); - if (!DCI.isCalledByLegalizer()) - DCI.AddToWorklist(SH.getNode()); - return DAG.getSetCC(dl, VT, SH, N1.getOperand(0), Cond); - } - } - } + N1.getOpcode() == ISD::XOR) + if (SDValue V = foldSetCCWithBinOp(VT, N1, N0, Cond, dl, DCI)) + return V; - if (SDValue V = simplifySetCCWithAnd(VT, N0, N1, Cond, DCI, dl)) + if (SDValue V = foldSetCCWithAnd(VT, N0, N1, Cond, dl, DCI)) return V; } diff --git a/test/CodeGen/X86/setcc-combine.ll b/test/CodeGen/X86/setcc-combine.ll index 8c3e3226b2e..65e8a6d6ed2 100644 --- a/test/CodeGen/X86/setcc-combine.ll +++ b/test/CodeGen/X86/setcc-combine.ll @@ -283,12 +283,18 @@ define i64 @PR40657(i8 %var2, i8 %var9) { ret i64 %res.cast } -; FIXME: This should not get folded to 0. +; This should not get folded to 0. define i64 @PR40657_commute(i8 %var7, i8 %var8, i8 %var9) { ; CHECK-LABEL: PR40657_commute: ; CHECK: # %bb.0: -; CHECK-NEXT: xorl %eax, %eax +; CHECK-NEXT: subb %dil, %sil +; CHECK-NEXT: subb %sil, %dl +; CHECK-NEXT: subb %dl, %sil +; CHECK-NEXT: xorb %dl, %sil +; CHECK-NEXT: subb %sil, %dl +; CHECK-NEXT: movzbl %dl, %eax +; CHECK-NEXT: andl $1, %eax ; CHECK-NEXT: retq %var4 = trunc i8 %var9 to i1 %var5 = trunc i8 %var8 to i1