[llvm] r329429 - [InstCombine] limit nsz: -(X - Y) --> Y - X to hasOneUse()

[Sanjay Patel via llvm-commits]

Author: spatel
Date: Fri Apr  6 10:24:08 2018
New Revision: 329429

URL: http://llvm.org/viewvc/llvm-project?rev=329429&view=rev
Log:
[InstCombine] limit nsz: -(X - Y) --> Y - X to hasOneUse()

As noted in the post-commit discussion for r329350, we shouldn't
generally assume that fsub is the same cost as fneg.

Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
    llvm/trunk/test/Transforms/InstCombine/fsub.ll

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp?rev=329429&r1=329428&r2=329429&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp Fri Apr  6 10:24:08 2018
@@ -1698,21 +1698,18 @@ Instruction *InstCombiner::visitFSub(Bin
                                   SQ.getWithInstruction(&I)))
     return replaceInstUsesWith(I, V);
 
-  Value *X, *Y;
-  if (I.hasNoSignedZeros()) {
-    // Subtraction from -0.0 is the canonical form of fneg.
-    // fsub nsz 0, X ==> fsub nsz -0.0, X
-    if (match(Op0, m_PosZeroFP()))
-      return BinaryOperator::CreateFNegFMF(Op1, &I);
-
-    // With no-signed-zeros: -(X - Y) --> Y - X
-    if (match(Op0, m_NegZeroFP()) && match(Op1, m_FSub(m_Value(X), m_Value(Y))))
-      return BinaryOperator::CreateFSubFMF(Y, X, &I);
-  }
+  // Subtraction from -0.0 is the canonical form of fneg.
+  // fsub nsz 0, X ==> fsub nsz -0.0, X
+  if (I.hasNoSignedZeros() && match(Op0, m_PosZeroFP()))
+    return BinaryOperator::CreateFNegFMF(Op1, &I);
 
-  // More generally than above, if Op0 is not -0.0: Z - (X - Y) --> Z + (Y - X)
+  // If Op0 is not -0.0 or we can ignore -0.0: Z - (X - Y) --> Z + (Y - X)
   // Canonicalize to fadd to make analysis easier.
   // This can also help codegen because fadd is commutative.
+  // Note that if this fsub was really an fneg, the fadd with -0.0 will get
+  // killed later. We still limit that particular transform with 'hasOneUse'
+  // because an fneg is assumed better/cheaper than a generic fsub.
+  Value *X, *Y;
   if (I.hasNoSignedZeros() || CannotBeNegativeZero(Op0, SQ.TLI)) {
     if (match(Op1, m_OneUse(m_FSub(m_Value(X), m_Value(Y))))) {
       Value *NewSub = Builder.CreateFSubFMF(Y, X, &I);

Modified: llvm/trunk/test/Transforms/InstCombine/fsub.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/fsub.ll?rev=329429&r1=329428&r2=329429&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/fsub.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/fsub.ll Fri Apr  6 10:24:08 2018
@@ -19,20 +19,24 @@ define float @test1(float %x, float %y)
 
 define float @neg_sub_nsz(float %x, float %y) {
 ; CHECK-LABEL: @neg_sub_nsz(
-; CHECK-NEXT:    [[T2:%.*]] = fsub nsz float [[Y:%.*]], [[X:%.*]]
-; CHECK-NEXT:    ret float [[T2]]
+; CHECK-NEXT:    [[TMP1:%.*]] = fsub nsz float [[Y:%.*]], [[X:%.*]]
+; CHECK-NEXT:    ret float [[TMP1]]
 ;
   %t1 = fsub float %x, %y
   %t2 = fsub nsz float -0.0, %t1
   ret float %t2
 }
 
+; If the subtract has another use, we don't do the transform (even though it
+; doesn't increase the IR instruction count) because we assume that fneg is 
+; easier to analyze and generally cheaper than generic fsub.
+
 declare void @use(float)
 
 define float @neg_sub_nsz_extra_use(float %x, float %y) {
 ; CHECK-LABEL: @neg_sub_nsz_extra_use(
 ; CHECK-NEXT:    [[T1:%.*]] = fsub float [[X:%.*]], [[Y:%.*]]
-; CHECK-NEXT:    [[T2:%.*]] = fsub nsz float [[Y]], [[X]]
+; CHECK-NEXT:    [[T2:%.*]] = fsub nsz float -0.000000e+00, [[T1]]
 ; CHECK-NEXT:    call void @use(float [[T1]])
 ; CHECK-NEXT:    ret float [[T2]]
 ;