Revert "[SDAG] remove use restriction in isNegatibleForFree() when called from getNegatedExpression()"

rotateright · rotateright · commit cdf5cfea8e52 · 2019-12-11T16:56:58.000-05:00
This reverts commit d1f0bdf. The patch can cause infinite loops in DAGCombiner.
diff --git 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 different 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
