[llvm] cdf5cfe - Revert "[SDAG] remove use restriction in isNegatibleForFree() when called from getNegatedExpression()"
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 11 13:57:08 PST 2019
Author: Sanjay Patel
Date: 2019-12-11T16:56:58-05:00
New Revision: cdf5cfea8e5204e5d2facd6663c4b49b9e0378e9
URL: https://github.com/llvm/llvm-project/commit/cdf5cfea8e5204e5d2facd6663c4b49b9e0378e9
DIFF: https://github.com/llvm/llvm-project/commit/cdf5cfea8e5204e5d2facd6663c4b49b9e0378e9.diff
LOG: Revert "[SDAG] remove use restriction in isNegatibleForFree() when called from getNegatedExpression()"
This reverts commit d1f0bdf2d2df9bdf11ee2ddfff3df50e53f2f042.
The patch can cause infinite loops in DAGCombiner.
Added:
Modified:
llvm/include/llvm/CodeGen/TargetLowering.h
llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
llvm/lib/Target/X86/X86ISelLowering.cpp
llvm/lib/Target/X86/X86ISelLowering.h
llvm/test/CodeGen/AArch64/arm64-fmadd.ll
llvm/test/CodeGen/X86/fma-fneg-combine-2.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index 687a2eb9296f..0726bdfec20e 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -3442,16 +3442,8 @@ class TargetLowering : public TargetLoweringBase {
/// Return 1 if we can compute the negated form of the specified expression
/// for the same cost as the expression itself, or 2 if we can compute the
/// negated form more cheaply than the expression itself. Else return 0.
- ///
- /// EnableUseCheck specifies whether the number of uses of a value affects
- /// if negation is considered free. This is needed because the number of uses
- /// of any value may change as we rewrite the expression. Therefore, when
- /// called from getNegatedExpression(), we must explicitly set EnableUseCheck
- /// to false to avoid getting a
diff erent answer than when called from other
- /// contexts.
virtual char isNegatibleForFree(SDValue Op, SelectionDAG &DAG,
bool LegalOperations, bool ForCodeSize,
- bool EnableUseCheck = true,
unsigned Depth = 0) const;
/// If isNegatibleForFree returns true, return the newly negated expression.
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index 05011aebb9d3..f8afdaf086ab 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -5413,21 +5413,18 @@ verifyReturnAddressArgumentIsConstant(SDValue Op, SelectionDAG &DAG) const {
char TargetLowering::isNegatibleForFree(SDValue Op, SelectionDAG &DAG,
bool LegalOperations, bool ForCodeSize,
- bool EnableUseCheck,
unsigned Depth) const {
// fneg is removable even if it has multiple uses.
if (Op.getOpcode() == ISD::FNEG)
return 2;
- // If the caller requires checking uses, don't allow anything with multiple
- // uses unless we know it is free.
+ // Don't allow anything with multiple uses unless we know it is free.
EVT VT = Op.getValueType();
const SDNodeFlags Flags = Op->getFlags();
const TargetOptions &Options = DAG.getTarget().Options;
- if (EnableUseCheck)
- if (!Op.hasOneUse() && !(Op.getOpcode() == ISD::FP_EXTEND &&
- isFPExtFree(VT, Op.getOperand(0).getValueType())))
- return 0;
+ if (!Op.hasOneUse() && !(Op.getOpcode() == ISD::FP_EXTEND &&
+ isFPExtFree(VT, Op.getOperand(0).getValueType())))
+ return 0;
// Don't recurse exponentially.
if (Depth > SelectionDAG::MaxRecursionDepth)
@@ -5471,11 +5468,11 @@ char TargetLowering::isNegatibleForFree(SDValue Op, SelectionDAG &DAG,
// fold (fneg (fadd A, B)) -> (fsub (fneg A), B)
if (char V = isNegatibleForFree(Op.getOperand(0), DAG, LegalOperations,
- ForCodeSize, EnableUseCheck, Depth + 1))
+ ForCodeSize, Depth + 1))
return V;
// fold (fneg (fadd A, B)) -> (fsub (fneg B), A)
return isNegatibleForFree(Op.getOperand(1), DAG, LegalOperations,
- ForCodeSize, EnableUseCheck, Depth + 1);
+ ForCodeSize, Depth + 1);
case ISD::FSUB:
// We can't turn -(A-B) into B-A when we honor signed zeros.
if (!Options.NoSignedZerosFPMath && !Flags.hasNoSignedZeros())
@@ -5488,7 +5485,7 @@ char TargetLowering::isNegatibleForFree(SDValue Op, SelectionDAG &DAG,
case ISD::FDIV:
// fold (fneg (fmul X, Y)) -> (fmul (fneg X), Y) or (fmul X, (fneg Y))
if (char V = isNegatibleForFree(Op.getOperand(0), DAG, LegalOperations,
- ForCodeSize, EnableUseCheck, Depth + 1))
+ ForCodeSize, Depth + 1))
return V;
// Ignore X * 2.0 because that is expected to be canonicalized to X + X.
@@ -5497,7 +5494,7 @@ char TargetLowering::isNegatibleForFree(SDValue Op, SelectionDAG &DAG,
return 0;
return isNegatibleForFree(Op.getOperand(1), DAG, LegalOperations,
- ForCodeSize, EnableUseCheck, Depth + 1);
+ ForCodeSize, Depth + 1);
case ISD::FMA:
case ISD::FMAD: {
@@ -5507,15 +5504,15 @@ char TargetLowering::isNegatibleForFree(SDValue Op, SelectionDAG &DAG,
// fold (fneg (fma X, Y, Z)) -> (fma (fneg X), Y, (fneg Z))
// fold (fneg (fma X, Y, Z)) -> (fma X, (fneg Y), (fneg Z))
char V2 = isNegatibleForFree(Op.getOperand(2), DAG, LegalOperations,
- ForCodeSize, EnableUseCheck, Depth + 1);
+ ForCodeSize, Depth + 1);
if (!V2)
return 0;
// One of Op0/Op1 must be cheaply negatible, then select the cheapest.
char V0 = isNegatibleForFree(Op.getOperand(0), DAG, LegalOperations,
- ForCodeSize, EnableUseCheck, Depth + 1);
+ ForCodeSize, Depth + 1);
char V1 = isNegatibleForFree(Op.getOperand(1), DAG, LegalOperations,
- ForCodeSize, EnableUseCheck, Depth + 1);
+ ForCodeSize, Depth + 1);
char V01 = std::max(V0, V1);
return V01 ? std::max(V01, V2) : 0;
}
@@ -5524,7 +5521,7 @@ char TargetLowering::isNegatibleForFree(SDValue Op, SelectionDAG &DAG,
case ISD::FP_ROUND:
case ISD::FSIN:
return isNegatibleForFree(Op.getOperand(0), DAG, LegalOperations,
- ForCodeSize, EnableUseCheck, Depth + 1);
+ ForCodeSize, Depth + 1);
}
return 0;
@@ -5568,7 +5565,7 @@ SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG,
// fold (fneg (fadd A, B)) -> (fsub (fneg A), B)
if (isNegatibleForFree(Op.getOperand(0), DAG, LegalOperations, ForCodeSize,
- false, Depth + 1))
+ Depth + 1))
return DAG.getNode(ISD::FSUB, SDLoc(Op), Op.getValueType(),
getNegatedExpression(Op.getOperand(0), DAG,
LegalOperations, ForCodeSize,
@@ -5595,7 +5592,7 @@ SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG,
case ISD::FDIV:
// fold (fneg (fmul X, Y)) -> (fmul (fneg X), Y)
if (isNegatibleForFree(Op.getOperand(0), DAG, LegalOperations, ForCodeSize,
- false, Depth + 1))
+ Depth + 1))
return DAG.getNode(Op.getOpcode(), SDLoc(Op), Op.getValueType(),
getNegatedExpression(Op.getOperand(0), DAG,
LegalOperations, ForCodeSize,
@@ -5619,9 +5616,9 @@ SDValue TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG,
ForCodeSize, Depth + 1);
char V0 = isNegatibleForFree(Op.getOperand(0), DAG, LegalOperations,
- ForCodeSize, false, Depth + 1);
+ ForCodeSize, Depth + 1);
char V1 = isNegatibleForFree(Op.getOperand(1), DAG, LegalOperations,
- ForCodeSize, false, Depth + 1);
+ ForCodeSize, Depth + 1);
if (V0 >= V1) {
// fold (fneg (fma X, Y, Z)) -> (fma (fneg X), Y, (fneg Z))
SDValue Neg0 = getNegatedExpression(
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index cdb588ddb8a2..866ee5b9a602 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -41898,7 +41898,6 @@ static SDValue combineFneg(SDNode *N, SelectionDAG &DAG,
char X86TargetLowering::isNegatibleForFree(SDValue Op, SelectionDAG &DAG,
bool LegalOperations,
bool ForCodeSize,
- bool EnableUseCheck,
unsigned Depth) const {
// fneg patterns are removable even if they have multiple uses.
if (isFNEG(DAG, Op.getNode(), Depth))
@@ -41927,7 +41926,7 @@ char X86TargetLowering::isNegatibleForFree(SDValue Op, SelectionDAG &DAG,
// extra operand negations as well.
for (int i = 0; i != 3; ++i) {
char V = isNegatibleForFree(Op.getOperand(i), DAG, LegalOperations,
- ForCodeSize, EnableUseCheck, Depth + 1);
+ ForCodeSize, Depth + 1);
if (V == 2)
return V;
}
@@ -41936,8 +41935,7 @@ char X86TargetLowering::isNegatibleForFree(SDValue Op, SelectionDAG &DAG,
}
return TargetLowering::isNegatibleForFree(Op, DAG, LegalOperations,
- ForCodeSize, EnableUseCheck,
- Depth);
+ ForCodeSize, Depth);
}
SDValue X86TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG,
@@ -41969,7 +41967,7 @@ SDValue X86TargetLowering::getNegatedExpression(SDValue Op, SelectionDAG &DAG,
SmallVector<SDValue, 4> NewOps(Op.getNumOperands(), SDValue());
for (int i = 0; i != 3; ++i) {
char V = isNegatibleForFree(Op.getOperand(i), DAG, LegalOperations,
- ForCodeSize, false, Depth + 1);
+ ForCodeSize, Depth + 1);
if (V == 2)
NewOps[i] = getNegatedExpression(Op.getOperand(i), DAG, LegalOperations,
ForCodeSize, Depth + 1);
diff --git a/llvm/lib/Target/X86/X86ISelLowering.h b/llvm/lib/Target/X86/X86ISelLowering.h
index 3bbf3b59ac5a..016120064134 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.h
+++ b/llvm/lib/Target/X86/X86ISelLowering.h
@@ -809,8 +809,7 @@ namespace llvm {
/// for the same cost as the expression itself, or 2 if we can compute the
/// negated form more cheaply than the expression itself. Else return 0.
char isNegatibleForFree(SDValue Op, SelectionDAG &DAG, bool LegalOperations,
- bool ForCodeSize, bool EnableUseCheck,
- unsigned Depth) const override;
+ bool ForCodeSize, unsigned Depth) const override;
/// If isNegatibleForFree returns true, return the newly negated expression.
SDValue getNegatedExpression(SDValue Op, SelectionDAG &DAG,
diff --git a/llvm/test/CodeGen/AArch64/arm64-fmadd.ll b/llvm/test/CodeGen/AArch64/arm64-fmadd.ll
index dffa83aa11b2..203ce623647f 100644
--- a/llvm/test/CodeGen/AArch64/arm64-fmadd.ll
+++ b/llvm/test/CodeGen/AArch64/arm64-fmadd.ll
@@ -88,23 +88,5 @@ entry:
ret double %0
}
-; This would crash while trying getNegatedExpression().
-
-define float @negated_constant(float %x) {
-; CHECK-LABEL: negated_constant:
-; CHECK: // %bb.0:
-; CHECK-NEXT: mov w8, #-1037565952
-; CHECK-NEXT: mov w9, #1109917696
-; CHECK-NEXT: fmov s1, w8
-; CHECK-NEXT: fmul s1, s0, s1
-; CHECK-NEXT: fmov s2, w9
-; CHECK-NEXT: fmadd s0, s0, s2, s1
-; CHECK-NEXT: ret
- %m = fmul float %x, 42.0
- %fma = call nsz float @llvm.fma.f32(float %x, float -42.0, float %m)
- %nfma = fneg float %fma
- ret float %nfma
-}
-
declare float @llvm.fma.f32(float, float, float) nounwind readnone
declare double @llvm.fma.f64(double, double, double) nounwind readnone
diff --git a/llvm/test/CodeGen/X86/fma-fneg-combine-2.ll b/llvm/test/CodeGen/X86/fma-fneg-combine-2.ll
index 9c846e3f555c..f9e87955270b 100644
--- a/llvm/test/CodeGen/X86/fma-fneg-combine-2.ll
+++ b/llvm/test/CodeGen/X86/fma-fneg-combine-2.ll
@@ -86,24 +86,4 @@ entry:
ret float %1
}
-; This would crash while trying getNegatedExpression().
-
-define float @negated_constant(float %x) {
-; FMA3-LABEL: negated_constant:
-; FMA3: # %bb.0:
-; FMA3-NEXT: vmulss {{.*}}(%rip), %xmm0, %xmm1
-; FMA3-NEXT: vfmadd132ss {{.*#+}} xmm0 = (xmm0 * mem) + xmm1
-; FMA3-NEXT: retq
-;
-; FMA4-LABEL: negated_constant:
-; FMA4: # %bb.0:
-; FMA4-NEXT: vmulss {{.*}}(%rip), %xmm0, %xmm1
-; FMA4-NEXT: vfmaddss %xmm1, {{.*}}(%rip), %xmm0, %xmm0
-; FMA4-NEXT: retq
- %m = fmul float %x, 42.0
- %fma = call nsz float @llvm.fma.f32(float %x, float -42.0, float %m)
- %nfma = fneg float %fma
- ret float %nfma
-}
-
declare float @llvm.fma.f32(float, float, float)
More information about the llvm-commits
mailing list