[Instcombine] Add patterns to generate fneg(fabs(x)) instead of fcmp/sub/selects (part 1)

[mnadeem]

Summary

https://alive2.llvm.org/ce/z/L5DmiE

TODO: handle <=, >, and >=

[mnadeem]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

2970

<= needs nsz on the select, so I'll just add that condition here and the rest should work fine.

[arsenm]

llvm/test/Transforms/InstCombine/fneg-fabs.ll

28

Missing some cases where the compared value is -0?

[mnadeem]

llvm/test/Transforms/InstCombine/fneg-fabs.ll

28

Any specific cases? Or should I duplicate all tests and set the compared value as -0?

[spatel]

Thanks for splitting this up and creating the Alive link!

I'm still not clear if the FMF propagation is behaving as well as it could. For example, if the select has nsz, then can it always be added to the fneg?
https://alive2.llvm.org/ce/z/2Qq6Pz

It would also help to sprinkle some extra flags like nnan and ninf into the tests, so we can tell how those are modified even though they are not required for the transform.

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

2968

"--> -X" should be "--> -fabs(X)"

llvm/test/Transforms/InstCombine/fneg-fabs.ll

28

We canonicalize fcmp to +0.0, so I think it would be enough to just switch any one of these tests to confirm that.
Using different types (a vector, a float, a half, etc) as we cycle through these would also increase coverage.

[mnadeem]

In D112675#3093663

I'm still not clear if the FMF propagation is behaving as well as it could. For example, if the select has nsz, then can it always be added to the fneg?
https://alive2.llvm.org/ce/z/2Qq6Pz

@efriedma any opinion on this?

[efriedma]

If the select is nsz, that means that if the result is zero, we don't care about its sign. That means we could copy it onto the fneg, sure. Actually, the produced fabs and fneg should always be nsz; the transform doesn't make sense otherwise.

For other flags, you can probably just copy them from the select? So initialize the flags from the select, then unconditionally enable the nsz bit for both instructions, I guess.

[mnadeem]

Removing from reviewer's ready to review list for now. Will come back to this patch when/if time permits.

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {
   // (X >= +/-0.0) ? X : -X --> fabs(X)
   if (match(CondVal, m_FCmp(Pred, m_Specific(TrueVal), m_AnyZeroFP())) &&
       match(FalseVal, m_FNeg(m_Specific(TrueVal))) && SI.hasNoSignedZeros() &&
       (Pred == FCmpInst::FCMP_OGT || Pred == FCmpInst::FCMP_OGE ||
        Pred == FCmpInst::FCMP_UGT || Pred == FCmpInst::FCMP_UGE)) {
     Value *Fabs = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, TrueVal, &SI);
     return replaceInstUsesWith(SI, Fabs);
   }
+  // Negative Fabs:
+  // 1. (X < +/-0.0) ? X : (0.0 - X)  --> (0.0 - fabs(X))
+  // 2. nsz on the sub (canonicalized to FNeg) or on the select:
+  //    (X < +/-0.0) ? X : -X         --> -X
+  if (match(CondVal, m_FCmp(Pred, m_Specific(TrueVal), m_AnyZeroFP())) &&
+      (Pred == FCmpInst::FCMP_OLT || Pred == FCmpInst::FCMP_ULT)) {
+    if (match(FalseVal, m_FSub(m_PosZeroFP(), m_Specific(TrueVal)))) {
+      Value *Fabs = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, TrueVal, &SI);
+      return BinaryOperator::CreateFSubFMF(
+          ConstantFP::get(Fabs->getType(), 0.0), Fabs,
+          cast<Instruction>(FalseVal));
+    } else if (match(FalseVal, m_FNeg(m_Specific(TrueVal))) &&
+               (cast<Instruction>(FalseVal)->hasNoSignedZeros() ||
+                SI.hasNoSignedZeros())) {
+      Value *Fabs = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, TrueVal, &SI);
+      return UnaryOperator::CreateFNegFMF(Fabs, cast<Instruction>(FalseVal));
+    }
+  }
 
   // See if we are selecting two values based on a comparison of the two values.
   if (ICmpInst *ICI = dyn_cast<ICmpInst>(CondVal))
     if (Instruction *Result = foldSelectInstWithICmp(SI, ICI))
       return Result;
 
   if (Instruction *Add = foldAddSubSelect(SI, Builder))
     return Add;

llvm/test/Transforms/InstCombine/fneg-fabs.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -instcombine -S | FileCheck %s
 
 
 ; ///////////////////////////////
 ; (X < +/-0.0) ? X : (0.0 - X)  --> (0.0 - fabs(X))
 ; (X < +/-0.0) ? X : -X         --> -X FMF on FNEG or on Select
 ; ///////////////////////////////
 
 ; (X < +/-0.0) ? X : (0.0 - X)  --> (0.0 - fabs(X))
 define double @olt_fsub(double %x) {
 ; CHECK-LABEL: @olt_fsub(
-; CHECK-NEXT:    [[CMP:%.*]] = fcmp olt double [[X:%.*]], 0.000000e+00
-; CHECK-NEXT:    [[NEGX:%.*]] = fsub double 0.000000e+00, [[X]]
-; CHECK-NEXT:    [[RETVAL:%.*]] = select i1 [[CMP]], double [[X]], double [[NEGX]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call double @llvm.fabs.f64(double [[X:%.*]])
+; CHECK-NEXT:    [[RETVAL:%.*]] = fsub double 0.000000e+00, [[TMP1]]
 ; CHECK-NEXT:    ret double [[RETVAL]]
 ;
   %cmp = fcmp olt double %x, 0.000000e+00
   %negX = fsub double 0.000000e+00, %x
   %retval = select i1 %cmp, double %x, double %negX
   ret double %retval
 }
 define double @ult_fsub(double %x) {
 ; CHECK-LABEL: @ult_fsub(
-; CHECK-NEXT:    [[CMP:%.*]] = fcmp ult double [[X:%.*]], 0.000000e+00
-; CHECK-NEXT:    [[NEGX:%.*]] = fsub double 0.000000e+00, [[X]]
-; CHECK-NEXT:    [[RETVAL:%.*]] = select i1 [[CMP]], double [[X]], double [[NEGX]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call double @llvm.fabs.f64(double [[X:%.*]])
+; CHECK-NEXT:    [[RETVAL:%.*]] = fsub double 0.000000e+00, [[TMP1]]
 ; CHECK-NEXT:    ret double [[RETVAL]]
 ;
   %cmp = fcmp ult double %x, 0.000000e+00
   %negX = fsub double 0.000000e+00, %x
   %retval = select i1 %cmp, double %x, double %negX
   ret double %retval
 }
 
 ; Negative tests <= needs nsz on select.
 define double @ole_fsub_nofmf(double %x) {
 ; CHECK-LABEL: @ole_fsub_nofmf(
;
   %cmp = fcmp ult double %x, 0.000000e+00
   %negX = fneg double %x
   %retval = select i1 %cmp, double %x, double %negX
   ret double %retval
 }
 
 define double @ult_fneg1(double %x) {
 ; CHECK-LABEL: @ult_fneg1(
-; CHECK-NEXT:    [[CMP:%.*]] = fcmp ult double [[X:%.*]], 0.000000e+00
-; CHECK-NEXT:    [[NEGX:%.*]] = fneg nsz double [[X]]
-; CHECK-NEXT:    [[RETVAL:%.*]] = select i1 [[CMP]], double [[X]], double [[NEGX]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call double @llvm.fabs.f64(double [[X:%.*]])
+; CHECK-NEXT:    [[RETVAL:%.*]] = fneg nsz double [[TMP1]]
 ; CHECK-NEXT:    ret double [[RETVAL]]
 ;
   %cmp = fcmp ult double %x, 0.000000e+00
   %negX = fneg nsz double %x
   %retval = select i1 %cmp, double %x, double %negX
   ret double %retval
 }
 define double @ult_fneg2(double %x) {
 ; CHECK-LABEL: @ult_fneg2(
-; CHECK-NEXT:    [[CMP:%.*]] = fcmp ult double [[X:%.*]], 0.000000e+00
-; CHECK-NEXT:    [[NEGX:%.*]] = fneg double [[X]]
-; CHECK-NEXT:    [[RETVAL:%.*]] = select nsz i1 [[CMP]], double [[X]], double [[NEGX]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call nsz double @llvm.fabs.f64(double [[X:%.*]])
+; CHECK-NEXT:    [[RETVAL:%.*]] = fneg double [[TMP1]]
 ; CHECK-NEXT:    ret double [[RETVAL]]
 ;
   %cmp = fcmp ult double %x, 0.000000e+00
   %negX = fneg double %x
   %retval = select nsz i1 %cmp, double %x, double %negX
   ret double %retval
 }
 define double @olt_fneg1(double %x) {
 ; CHECK-LABEL: @olt_fneg1(
-; CHECK-NEXT:    [[CMP:%.*]] = fcmp olt double [[X:%.*]], 0.000000e+00
-; CHECK-NEXT:    [[NEGX:%.*]] = fneg nsz double [[X]]
-; CHECK-NEXT:    [[RETVAL:%.*]] = select i1 [[CMP]], double [[X]], double [[NEGX]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call double @llvm.fabs.f64(double [[X:%.*]])
+; CHECK-NEXT:    [[RETVAL:%.*]] = fneg nsz double [[TMP1]]
 ; CHECK-NEXT:    ret double [[RETVAL]]
 ;
   %cmp = fcmp olt double %x, 0.000000e+00
   %negX = fneg nsz double %x
   %retval = select i1 %cmp, double %x, double %negX
   ret double %retval
 }
 define double @olt_fneg2(double %x) {
 ; CHECK-LABEL: @olt_fneg2(
-; CHECK-NEXT:    [[CMP:%.*]] = fcmp olt double [[X:%.*]], 0.000000e+00
-; CHECK-NEXT:    [[NEGX:%.*]] = fneg double [[X]]
-; CHECK-NEXT:    [[RETVAL:%.*]] = select nsz i1 [[CMP]], double [[X]], double [[NEGX]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call nsz double @llvm.fabs.f64(double [[X:%.*]])
+; CHECK-NEXT:    [[RETVAL:%.*]] = fneg double [[TMP1]]
 ; CHECK-NEXT:    ret double [[RETVAL]]
 ;
   %cmp = fcmp olt double %x, 0.000000e+00
   %negX = fneg double %x
   %retval = select nsz i1 %cmp, double %x, double %negX
   ret double %retval
 }