Commit cdf5cfea authored by Sanjay Patel's avatar Sanjay Patel
Browse files

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

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

This reverts commit d1f0bdf2.
The patch can cause infinite loops in DAGCombiner.
parent 34ef51b5

llvm/include/llvm/CodeGen/TargetLowering.h

+0 −8
Original line number Diff line number Diff line
@@ -3442,16 +3442,8 @@ public: 
  /// 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.

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

+16 −19
Original line number Diff line number Diff line
@@ -5413,18 +5413,15 @@ 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;
@@ -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(

llvm/lib/Target/X86/X86ISelLowering.cpp

+3 −5
Original line number Diff line number Diff line
@@ -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);

























llvm/lib/Target/X86/X86ISelLowering.h



+1
−2






























Original line number
Diff line number
Diff line








@@ -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,

































llvm/test/CodeGen/AArch64/arm64-fmadd.ll



+0
−18






























Original line number
Diff line number
Diff line








@@ -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