[llvm] r353639 - [TargetLowering] refactor setcc folds to fix another miscompile (PR40657)
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Sun Feb 10 06:29:57 PST 2019
Author: spatel
Date: Sun Feb 10 06:29:57 2019
New Revision: 353639
URL: http://llvm.org/viewvc/llvm-project?rev=353639&view=rev
Log:
[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.
Modified:
llvm/trunk/include/llvm/CodeGen/TargetLowering.h
llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp
llvm/trunk/test/CodeGen/X86/setcc-combine.ll
Modified: llvm/trunk/include/llvm/CodeGen/TargetLowering.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/TargetLowering.h?rev=353639&r1=353638&r2=353639&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/TargetLowering.h (original)
+++ llvm/trunk/include/llvm/CodeGen/TargetLowering.h Sun Feb 10 06:29:57 2019
@@ -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,
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp?rev=353639&r1=353638&r2=353639&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/TargetLowering.cpp Sun Feb 10 06:29:57 2019
@@ -2136,10 +2136,9 @@ bool TargetLowering::isExtendedTrueVal(c
/// 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::optimizeSetCCOfS
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(EV
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;
}
Modified: llvm/trunk/test/CodeGen/X86/setcc-combine.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/setcc-combine.ll?rev=353639&r1=353638&r2=353639&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/setcc-combine.ll (original)
+++ llvm/trunk/test/CodeGen/X86/setcc-combine.ll Sun Feb 10 06:29:57 2019
@@ -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
More information about the llvm-commits
mailing list