[llvm] r362943 - [InstCombine] change canonicalization to fabs() to use FMF on fsub

[Sanjay Patel via llvm-commits]

Author: spatel
Date: Mon Jun 10 07:46:36 2019
New Revision: 362943

URL: http://llvm.org/viewvc/llvm-project?rev=362943&view=rev
Log:
[InstCombine] change canonicalization to fabs() to use FMF on fsub

Similar to rL362909:
This isn't the ideal fix (use FMF on the select), but it's still an
improvement until we have better FMF propagation to selects and other
FP math operators.

I don't think there's much risk of regression from this change by
not including the FMF on the fcmp any more. The nsz/nnan FMF
should be the same on the fcmp and the fsub because they have the
same operand.

Modified:
    llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp
    llvm/trunk/test/Transforms/InstCombine/fabs.ll

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp?rev=362943&r1=362942&r2=362943&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineSelect.cpp Mon Jun 10 07:46:36 2019
@@ -1866,27 +1866,29 @@ Instruction *InstCombiner::visitSelectIn
 
       // NOTE: if we wanted to, this is where to detect MIN/MAX
     }
-
-    // Canonicalize select with fcmp to fabs(). -0.0 makes this tricky. We need
-    // fast-math-flags (nsz) or fsub with +0.0 (not fneg) for this to work. We
-    // also require nnan because we do not want to unintentionally change the
-    // sign of a NaN value.
-    Value *X = FCI->getOperand(0);
-    FCmpInst::Predicate Pred = FCI->getPredicate();
-    if (match(FCI->getOperand(1), m_AnyZeroFP()) && FCI->hasNoNaNs()) {
-      // (X <= +/-0.0) ? (0.0 - X) : X --> fabs(X)
-      // (X >  +/-0.0) ? X : (0.0 - X) --> fabs(X)
-      if ((X == FalseVal && Pred == FCmpInst::FCMP_OLE &&
-           match(TrueVal, m_FSub(m_PosZeroFP(), m_Specific(X)))) ||
-          (X == TrueVal && Pred == FCmpInst::FCMP_OGT &&
-           match(FalseVal, m_FSub(m_PosZeroFP(), m_Specific(X))))) {
-        Value *Fabs = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, X, FCI);
-        return replaceInstUsesWith(SI, Fabs);
-      }
-    }
   }
 
-  // FIXME: These folds should test/propagate FMF from the select, not the fneg.
+  // Canonicalize select with fcmp to fabs(). -0.0 makes this tricky. We need
+  // fast-math-flags (nsz) or fsub with +0.0 (not fneg) for this to work. We
+  // also require nnan because we do not want to unintentionally change the
+  // sign of a NaN value.
+  // FIXME: These folds should test/propagate FMF from the select, not the
+  //        fsub or fneg.
+  // (X <= +/-0.0) ? (0.0 - X) : X --> fabs(X)
+  Instruction *FSub;
+  if (match(CondVal, m_FCmp(Pred, m_Specific(FalseVal), m_AnyZeroFP())) &&
+      match(TrueVal, m_FSub(m_PosZeroFP(), m_Specific(FalseVal))) &&
+      match(TrueVal, m_Instruction(FSub)) && FSub->hasNoNaNs()) {
+    Value *Fabs = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, FalseVal, FSub);
+    return replaceInstUsesWith(SI, Fabs);
+  }
+  // (X >  +/-0.0) ? X : (0.0 - X) --> fabs(X)
+  if (match(CondVal, m_FCmp(Pred, m_Specific(TrueVal), m_AnyZeroFP())) &&
+      match(FalseVal, m_FSub(m_PosZeroFP(), m_Specific(TrueVal))) &&
+      match(FalseVal, m_Instruction(FSub)) && FSub->hasNoNaNs()) {
+    Value *Fabs = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, TrueVal, FSub);
+    return replaceInstUsesWith(SI, Fabs);
+  }
   // With nnan and nsz:
   // (X <  +/-0.0) ? -X : X --> fabs(X)
   // (X <= +/-0.0) ? -X : X --> fabs(X)

Modified: llvm/trunk/test/Transforms/InstCombine/fabs.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/fabs.ll?rev=362943&r1=362942&r2=362943&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/fabs.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/fabs.ll Mon Jun 10 07:46:36 2019
@@ -269,8 +269,8 @@ define double @select_fcmp_nnan_ole_zero
 ; CHECK-NEXT:    [[TMP1:%.*]] = call nnan double @llvm.fabs.f64(double [[X:%.*]])
 ; CHECK-NEXT:    ret double [[TMP1]]
 ;
-  %lezero = fcmp nnan ole double %x, 0.0
-  %negx = fsub double 0.0, %x
+  %lezero = fcmp ole double %x, 0.0
+  %negx = fsub nnan double 0.0, %x
   %fabs = select i1 %lezero, double %negx, double %x
   ret double %fabs
 }
@@ -282,8 +282,8 @@ define <2 x float> @select_fcmp_nnan_ole
 ; CHECK-NEXT:    [[TMP1:%.*]] = call nnan <2 x float> @llvm.fabs.v2f32(<2 x float> [[X:%.*]])
 ; CHECK-NEXT:    ret <2 x float> [[TMP1]]
 ;
-  %lezero = fcmp nnan ole <2 x float> %x, <float -0.0, float -0.0>
-  %negx = fsub <2 x float> <float 0.0, float undef>, %x
+  %lezero = fcmp ole <2 x float> %x, <float -0.0, float -0.0>
+  %negx = fsub nnan <2 x float> <float 0.0, float undef>, %x
   %fabs = select <2 x i1> %lezero, <2 x float> %negx, <2 x float> %x
   ret <2 x float> %fabs
 }
@@ -295,8 +295,8 @@ define fp128 @select_fcmp_nnan_ogt_zero(
 ; CHECK-NEXT:    [[TMP1:%.*]] = call nnan fp128 @llvm.fabs.f128(fp128 [[X:%.*]])
 ; CHECK-NEXT:    ret fp128 [[TMP1]]
 ;
-  %gtzero = fcmp nnan ogt fp128 %x, zeroinitializer
-  %negx = fsub fp128 zeroinitializer, %x
+  %gtzero = fcmp ogt fp128 %x, zeroinitializer
+  %negx = fsub nnan fp128 zeroinitializer, %x
   %fabs = select i1 %gtzero, fp128 %x, fp128 %negx
   ret fp128 %fabs
 }
@@ -308,8 +308,8 @@ define half @select_fcmp_nnan_ogt_negzer
 ; CHECK-NEXT:    [[TMP1:%.*]] = call nnan half @llvm.fabs.f16(half [[X:%.*]])
 ; CHECK-NEXT:    ret half [[TMP1]]
 ;
-  %gtzero = fcmp nnan ogt half %x, -0.0
-  %negx = fsub half 0.0, %x
+  %gtzero = fcmp ogt half %x, -0.0
+  %negx = fsub nnan half 0.0, %x
   %fabs = select i1 %gtzero, half %x, half %negx
   ret half %fabs
 }