[llvm] r284824 - [DAG] use SDNode flags 'nsz' to enable fadd/fsub with zero folds

Sanjay Patel via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 21 07:36:59 PDT 2016


Author: spatel
Date: Fri Oct 21 09:36:58 2016
New Revision: 284824

URL: http://llvm.org/viewvc/llvm-project?rev=284824&view=rev
Log:
[DAG] use SDNode flags 'nsz' to enable fadd/fsub with zero folds

As discussed in D24815, let's start the process of killing off the broken fast-math global
state housed in TargetOptions and eliminate the need for function-level fast-math attributes.

Here we enable two similar folds that are possible when we don't care about signed-zero:
fadd nsz x, 0 --> x
fsub nsz 0, x --> -x

Note that although the test cases include a 'sin' function call, I'm side-stepping the 
FMF-on-calls question (and lack of support in the DAG) for now. It's not needed for these
tests - isNegatibleForFree/GetNegatedExpression just look through a ISD::FSIN node.

Also, when we create an FNEG node and propagate the Flags of the FSUB to it, this doesn't
actually do anything today because Flags are silently dropped for any node that is not a
binary operator.

Differential Revision: https://reviews.llvm.org/D25297

Modified:
    llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
    llvm/trunk/test/CodeGen/X86/negative-sin.ll

Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=284824&r1=284823&r2=284824&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Fri Oct 21 09:36:58 2016
@@ -622,7 +622,8 @@ static char isNegatibleForFree(SDValue O
                               Depth + 1);
   case ISD::FSUB:
     // We can't turn -(A-B) into B-A when we honor signed zeros.
-    if (!Options->UnsafeFPMath) return 0;
+    if (!Options->UnsafeFPMath && !Op.getNode()->getFlags()->hasNoSignedZeros())
+      return 0;
 
     // fold (fneg (fsub A, B)) -> (fsub B, A)
     return 1;
@@ -685,9 +686,6 @@ static SDValue GetNegatedExpression(SDVa
                                             LegalOperations, Depth+1),
                        Op.getOperand(0), Flags);
   case ISD::FSUB:
-    // We can't turn -(A-B) into B-A when we honor signed zeros.
-    assert(Options.UnsafeFPMath);
-
     // fold (fneg (fsub 0, B)) -> B
     if (ConstantFPSDNode *N0CFP = dyn_cast<ConstantFPSDNode>(Op.getOperand(0)))
       if (N0CFP->isZero())
@@ -8462,17 +8460,20 @@ SDValue DAGCombiner::visitFADD(SDNode *N
     return DAG.getNode(ISD::FSUB, DL, VT, N1,
                        GetNegatedExpression(N0, DAG, LegalOperations), Flags);
 
+  // FIXME: Auto-upgrade the target/function-level option.
+  if (Options.UnsafeFPMath || N->getFlags()->hasNoSignedZeros()) {
+    // fold (fadd A, 0) -> A
+    if (ConstantFPSDNode *N1C = isConstOrConstSplatFP(N1))
+      if (N1C->isZero())
+        return N0;
+  }
+
   // If 'unsafe math' is enabled, fold lots of things.
   if (Options.UnsafeFPMath) {
     // No FP constant should be created after legalization as Instruction
     // Selection pass has a hard time dealing with FP constants.
     bool AllowNewConst = (Level < AfterLegalizeDAG);
 
-    // fold (fadd A, 0) -> A
-    if (ConstantFPSDNode *N1C = isConstOrConstSplatFP(N1))
-      if (N1C->isZero())
-        return N0;
-
     // fold (fadd (fadd x, c1), c2) -> (fadd x, (fadd c1, c2))
     if (N1CFP && N0.getOpcode() == ISD::FADD && N0.getNode()->hasOneUse() &&
         isConstantFPBuildVectorOrConstantFP(N0.getOperand(1)))
@@ -8599,19 +8600,22 @@ SDValue DAGCombiner::visitFSUB(SDNode *N
     return DAG.getNode(ISD::FADD, DL, VT, N0,
                        GetNegatedExpression(N1, DAG, LegalOperations), Flags);
 
-  // If 'unsafe math' is enabled, fold lots of things.
-  if (Options.UnsafeFPMath) {
-    // (fsub A, 0) -> A
-    if (N1CFP && N1CFP->isZero())
-      return N0;
-
+  // FIXME: Auto-upgrade the target/function-level option.
+  if (Options.UnsafeFPMath || N->getFlags()->hasNoSignedZeros()) {
     // (fsub 0, B) -> -B
     if (N0CFP && N0CFP->isZero()) {
       if (isNegatibleForFree(N1, LegalOperations, TLI, &Options))
         return GetNegatedExpression(N1, DAG, LegalOperations);
       if (!LegalOperations || TLI.isOperationLegal(ISD::FNEG, VT))
-        return DAG.getNode(ISD::FNEG, DL, VT, N1);
+        return DAG.getNode(ISD::FNEG, DL, VT, N1, Flags);
     }
+  }
+
+  // If 'unsafe math' is enabled, fold lots of things.
+  if (Options.UnsafeFPMath) {
+    // (fsub A, 0) -> A
+    if (N1CFP && N1CFP->isZero())
+      return N0;
 
     // (fsub x, x) -> 0.0
     if (N0 == N1)

Modified: llvm/trunk/test/CodeGen/X86/negative-sin.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/negative-sin.ll?rev=284824&r1=284823&r2=284824&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/negative-sin.ll (original)
+++ llvm/trunk/test/CodeGen/X86/negative-sin.ll Fri Oct 21 09:36:58 2016
@@ -23,21 +23,13 @@ define double @strict(double %e) nounwin
   ret double %h
 }
 
-; FIXME:
 ; 'fast' implies no-signed-zeros, so the negates fold away.
 ; The 'sin' does not need any fast-math-flags for this transform.
 
 define double @fast(double %e) nounwind {
 ; CHECK-LABEL: fast:
 ; CHECK:       # BB#0:
-; CHECK-NEXT:    pushq %rax
-; CHECK-NEXT:    vxorpd %xmm1, %xmm1, %xmm1
-; CHECK-NEXT:    vsubsd %xmm0, %xmm1, %xmm0
-; CHECK-NEXT:    callq sin
-; CHECK-NEXT:    vxorpd %xmm1, %xmm1, %xmm1
-; CHECK-NEXT:    vsubsd %xmm0, %xmm1, %xmm0
-; CHECK-NEXT:    popq %rax
-; CHECK-NEXT:    retq
+; CHECK-NEXT:    jmp sin
 ;
   %f = fsub fast double 0.0, %e
   %g = call double @sin(double %f) readonly
@@ -45,20 +37,12 @@ define double @fast(double %e) nounwind
   ret double %h
 }
 
-; FIXME:
 ; No-signed-zeros is all that we need for this transform.
 
 define double @nsz(double %e) nounwind {
 ; CHECK-LABEL: nsz:
 ; CHECK:       # BB#0:
-; CHECK-NEXT:    pushq %rax
-; CHECK-NEXT:    vxorpd %xmm1, %xmm1, %xmm1
-; CHECK-NEXT:    vsubsd %xmm0, %xmm1, %xmm0
-; CHECK-NEXT:    callq sin
-; CHECK-NEXT:    vxorpd %xmm1, %xmm1, %xmm1
-; CHECK-NEXT:    vsubsd %xmm0, %xmm1, %xmm0
-; CHECK-NEXT:    popq %rax
-; CHECK-NEXT:    retq
+; CHECK-NEXT:    jmp sin
 ;
   %f = fsub nsz double 0.0, %e
   %g = call double @sin(double %f) readonly
@@ -66,7 +50,6 @@ define double @nsz(double %e) nounwind {
   ret double %h
 }
 
-; FIXME:
 ; The 1st negate is strict, so we can't kill that sub, but the 2nd disappears.
 
 define double @semi_strict1(double %e) nounwind {
@@ -76,8 +59,7 @@ define double @semi_strict1(double %e) n
 ; CHECK-NEXT:    vxorpd %xmm1, %xmm1, %xmm1
 ; CHECK-NEXT:    vsubsd %xmm0, %xmm1, %xmm0
 ; CHECK-NEXT:    callq sin
-; CHECK-NEXT:    vxorpd %xmm1, %xmm1, %xmm1
-; CHECK-NEXT:    vsubsd %xmm0, %xmm1, %xmm0
+; CHECK-NEXT:    vxorpd {{.*}}(%rip), %xmm0, %xmm0
 ; CHECK-NEXT:    popq %rax
 ; CHECK-NEXT:    retq
 ;
@@ -87,18 +69,15 @@ define double @semi_strict1(double %e) n
   ret double %h
 }
 
-; FIXME:
 ; The 2nd negate is strict, so we can't kill it. It becomes an add of zero instead.
 
 define double @semi_strict2(double %e) nounwind {
 ; CHECK-LABEL: semi_strict2:
 ; CHECK:       # BB#0:
 ; CHECK-NEXT:    pushq %rax
-; CHECK-NEXT:    vxorpd %xmm1, %xmm1, %xmm1
-; CHECK-NEXT:    vsubsd %xmm0, %xmm1, %xmm0
 ; CHECK-NEXT:    callq sin
 ; CHECK-NEXT:    vxorpd %xmm1, %xmm1, %xmm1
-; CHECK-NEXT:    vsubsd %xmm0, %xmm1, %xmm0
+; CHECK-NEXT:    vaddsd %xmm1, %xmm0, %xmm0
 ; CHECK-NEXT:    popq %rax
 ; CHECK-NEXT:    retq
 ;




More information about the llvm-commits mailing list