[InstCombine] canonicalize fcmp+select to fabs

[spatel]

Summary

This is based on the DAG patterns as shown in D44091. I'm hoping that we can just remove those DAG folds and always rely on IR canonicalization to handle the matching to fabs.

We would still need to delete the broken code from DAGCombiner to fix PR36600:
https://bugs.llvm.org/show_bug.cgi?id=36600

[mike.dvoretsky]

lib/Transforms/InstCombine/InstCombineSelect.cpp

1577

I think there should be an nnan check here. The patterns in DAGCombiner check it implicitly by requiring condition codes that only occur in FP math under nnan, but this code currently doesn't require that, and all of the patterns would direct NaNs of either sign to the same option, creating the same kind of issue as the current patterns have with zero signs (and 0.0 - NaN shouldn't even do anything).

[spatel]

lib/Transforms/InstCombine/InstCombineSelect.cpp

1577

Can you show an example of how this goes wrong with a NaN input? Is there a requirement that we preserve the sign bit of a NaN value in any of the original code sequences?

[mike.dvoretsky]

lib/Transforms/InstCombine/InstCombineSelect.cpp

1577

Per IEEE 754, bit operations (including fabs and copysign) consider sign bits of NaNs as if they were normal FP values. To use a modified example from PR36600:

#include <stdio.h>
#include <math.h>

int main(int argc, char **argv)
{
  double zero;
  sscanf(argv[1], "%lf", &zero);
  zero = 0.0 / zero; // produces a NaN if we have a zero input
  zero = copysign(zero, -1.0); // sets NaN to be negative
  zero = (zero < 0.0) ? (0.0 - zero) : zero; // NaN sign preserved
  zero = copysign(-1.0, zero); // reading NaN sign
  printf("%f\n", zero);
  return 0;
}

Without nsz, this produces -1.0 on zero inputs, since the NaN with the set sign bit goes through the pattern unchanged. With nsz, this patch folds it to fabs, which unsets the sign bit and leads to a 1.0 result.

[spatel]

lib/Transforms/InstCombine/InstCombineSelect.cpp

1577

Thanks. Yes, I think we should try to follow IEEE requirements here. Let me update the patch. I think this also means we have to try harder in D44521.

[spatel]

Patch updated:

1. Require nnan for all folds.
2. Add negative test to show that we won't fold without nnan.

lib/Transforms/InstCombine/InstCombineSelect.cpp

if (FCI->getOperand(0) == TrueVal && FCI->getOperand(1) == FalseVal) {
                                             FCI->getName() + ".inv");
 
         return SelectInst::Create(NewCond, FalseVal, TrueVal,
                                   SI.getName() + ".p");
       }
 
       // NOTE: if we wanted to, this is where to detect MIN/MAX
     }
-    // NOTE: if we wanted to, this is where to detect ABS
+
+    // 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.
+    Value *X = FCI->getOperand(0);
+    FCmpInst::Predicate Pred = FCI->getPredicate();
+    if (match(FCI->getOperand(1), m_AnyZeroFP())) {
+      // (X <= +/-0.0) ? (0.0 - X) : X --> fabs(X)
+      // (X >  +/-0.0) ? X : (0.0 - X) --> fabs(X)
+      if ((X == FalseVal && match(TrueVal, m_FSub(m_Zero(), m_Specific(X))) &&
+          Pred == FCmpInst::FCMP_OLE) ||
+          (X == TrueVal && match(FalseVal, m_FSub(m_Zero(), m_Specific(X))) &&
+          Pred == FCmpInst::FCMP_OGT)) {
+        Value *Fabs = Builder.CreateIntrinsic(Intrinsic::fabs, { X }, FCI);
+        return replaceInstUsesWith(SI, Fabs);
+      }
+      // With nsz:
+      // (X <  +/-0.0) ? -X : X --> fabs(X)
+      // (X <= +/-0.0) ? -X : X --> fabs(X)
+      // (X >  +/-0.0) ? X : -X --> fabs(X)
+      // (X >= +/-0.0) ? X : -X --> fabs(X)
+      if (FCI->hasNoSignedZeros() &&
+          ((X == FalseVal && match(TrueVal, m_FNeg(m_Specific(X))) &&
+            (Pred == FCmpInst::FCMP_OLT || Pred == FCmpInst::FCMP_OLE)) ||
+           (X == TrueVal && match(FalseVal, m_FNeg(m_Specific(X))) &&
+            (Pred == FCmpInst::FCMP_OGT || Pred == FCmpInst::FCMP_OGE)))) {
+        Value *Fabs = Builder.CreateIntrinsic(Intrinsic::fabs, { X }, FCI);
+        return replaceInstUsesWith(SI, Fabs);
+      }
+    }
   }
 
   // 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))

test/Transforms/InstCombine/fabs.ll

;
   store volatile double %fpext, double* undef
   ret double %fabs
 }
 
 ; X <= 0.0 ? (0.0 - X) : X --> fabs(X)
 
 define double @select_fcmp_ole_zero(double %x) {
 ; CHECK-LABEL: @select_fcmp_ole_zero(
-; CHECK-NEXT:    [[LEZERO:%.*]] = fcmp ole double [[X:%.*]], 0.000000e+00
-; CHECK-NEXT:    [[NEGX:%.*]] = fsub double 0.000000e+00, [[X]]
-; CHECK-NEXT:    [[FABS:%.*]] = select i1 [[LEZERO]], double [[NEGX]], double [[X]]
-; CHECK-NEXT:    ret double [[FABS]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call double @llvm.fabs.f64(double [[X:%.*]])
+; CHECK-NEXT:    ret double [[TMP1]]
 ;
   %lezero = fcmp ole double %x, 0.0
   %negx = fsub double 0.0, %x
   %fabs = select i1 %lezero, double %negx, double %x
   ret double %fabs
 }
 
 ; X <= -0.0 ? (0.0 - X) : X --> fabs(X)
 
 define float @select_fcmp_ole_negzero(float %x) {
 ; CHECK-LABEL: @select_fcmp_ole_negzero(
-; CHECK-NEXT:    [[LEZERO:%.*]] = fcmp ole float [[X:%.*]], -0.000000e+00
-; CHECK-NEXT:    [[NEGX:%.*]] = fsub float 0.000000e+00, [[X]]
-; CHECK-NEXT:    [[FABS:%.*]] = select i1 [[LEZERO]], float [[NEGX]], float [[X]]
-; CHECK-NEXT:    ret float [[FABS]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call float @llvm.fabs.f32(float [[X:%.*]])
+; CHECK-NEXT:    ret float [[TMP1]]
 ;
   %lezero = fcmp ole float %x, -0.0
   %negx = fsub float 0.0, %x
   %fabs = select i1 %lezero, float %negx, float %x
   ret float %fabs
 }
 
 ; X > 0.0 ? X : (0.0 - X) --> fabs(X)
 
 define fp128 @select_fcmp_ogt_zero(fp128 %x) {
 ; CHECK-LABEL: @select_fcmp_ogt_zero(
-; CHECK-NEXT:    [[GTZERO:%.*]] = fcmp ogt fp128 [[X:%.*]], 0xL00000000000000000000000000000000
-; CHECK-NEXT:    [[NEGX:%.*]] = fsub fp128 0xL00000000000000000000000000000000, [[X]]
-; CHECK-NEXT:    [[FABS:%.*]] = select i1 [[GTZERO]], fp128 [[X]], fp128 [[NEGX]]
-; CHECK-NEXT:    ret fp128 [[FABS]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call fp128 @llvm.fabs.f128(fp128 [[X:%.*]])
+; CHECK-NEXT:    ret fp128 [[TMP1]]
 ;
   %gtzero = fcmp ogt fp128 %x, zeroinitializer
   %negx = fsub fp128 zeroinitializer, %x
   %fabs = select i1 %gtzero, fp128 %x, fp128 %negx
   ret fp128 %fabs
 }
 
 ; X > -0.0 ? X : (0.0 - X) --> fabs(X)
 
 define half @select_fcmp_ogt_negzero(half %x) {
 ; CHECK-LABEL: @select_fcmp_ogt_negzero(
-; CHECK-NEXT:    [[GTZERO:%.*]] = fcmp ogt half [[X:%.*]], 0xH8000
-; CHECK-NEXT:    [[NEGX:%.*]] = fsub half 0xH0000, [[X]]
-; CHECK-NEXT:    [[FABS:%.*]] = select i1 [[GTZERO]], half [[X]], half [[NEGX]]
-; CHECK-NEXT:    ret half [[FABS]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call half @llvm.fabs.f16(half [[X:%.*]])
+; CHECK-NEXT:    ret half [[TMP1]]
 ;
   %gtzero = fcmp ogt half %x, -0.0
   %negx = fsub half 0.0, %x
   %fabs = select i1 %gtzero, half %x, half %negx
   ret half %fabs
 }
 
 ; X < 0.0 ? -X : X --> fabs(X)
 
 define double @select_fcmp_nsz_olt_zero(double %x) {
 ; CHECK-LABEL: @select_fcmp_nsz_olt_zero(
-; CHECK-NEXT:    [[LTZERO:%.*]] = fcmp nsz olt double [[X:%.*]], 0.000000e+00
-; CHECK-NEXT:    [[NEGX:%.*]] = fsub double -0.000000e+00, [[X]]
-; CHECK-NEXT:    [[FABS:%.*]] = select i1 [[LTZERO]], double [[NEGX]], double [[X]]
-; CHECK-NEXT:    ret double [[FABS]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call nsz double @llvm.fabs.f64(double [[X:%.*]])
+; CHECK-NEXT:    ret double [[TMP1]]
 ;
   %ltzero = fcmp nsz olt double %x, 0.0
   %negx = fsub double -0.0, %x
   %fabs = select i1 %ltzero, double %negx, double %x
   ret double %fabs
 }
 
 ; X < -0.0 ? -X : X --> fabs(X)
 
 define float @select_fcmp_nsz_olt_negzero(float %x) {
 ; CHECK-LABEL: @select_fcmp_nsz_olt_negzero(
-; CHECK-NEXT:    [[LTZERO:%.*]] = fcmp ninf nsz olt float [[X:%.*]], -0.000000e+00
-; CHECK-NEXT:    [[NEGX:%.*]] = fsub float -0.000000e+00, [[X]]
-; CHECK-NEXT:    [[FABS:%.*]] = select i1 [[LTZERO]], float [[NEGX]], float [[X]]
-; CHECK-NEXT:    ret float [[FABS]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call ninf nsz float @llvm.fabs.f32(float [[X:%.*]])
+; CHECK-NEXT:    ret float [[TMP1]]
 ;
   %ltzero = fcmp nsz ninf olt float %x, -0.0
   %negx = fsub float -0.0, %x
   %fabs = select i1 %ltzero, float %negx, float %x
   ret float %fabs
 }
 
 ; X <= 0.0 ? -X : X --> fabs(X)
 
 define double @select_fcmp_nsz_ole_zero(double %x) {
 ; CHECK-LABEL: @select_fcmp_nsz_ole_zero(
-; CHECK-NEXT:    [[LEZERO:%.*]] = fcmp fast ole double [[X:%.*]], 0.000000e+00
-; CHECK-NEXT:    [[NEGX:%.*]] = fsub double -0.000000e+00, [[X]]
-; CHECK-NEXT:    [[FABS:%.*]] = select i1 [[LEZERO]], double [[NEGX]], double [[X]]
-; CHECK-NEXT:    ret double [[FABS]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call fast double @llvm.fabs.f64(double [[X:%.*]])
+; CHECK-NEXT:    ret double [[TMP1]]
 ;
   %lezero = fcmp fast ole double %x, 0.0
   %negx = fsub double -0.0, %x
   %fabs = select i1 %lezero, double %negx, double %x
   ret double %fabs
 }
 
 ; X <= -0.0 ? -X : X --> fabs(X)
 
 define float @select_fcmp_nsz_ole_negzero(float %x) {
 ; CHECK-LABEL: @select_fcmp_nsz_ole_negzero(
-; CHECK-NEXT:    [[LEZERO:%.*]] = fcmp nsz ole float [[X:%.*]], -0.000000e+00
-; CHECK-NEXT:    [[NEGX:%.*]] = fsub float -0.000000e+00, [[X]]
-; CHECK-NEXT:    [[FABS:%.*]] = select i1 [[LEZERO]], float [[NEGX]], float [[X]]
-; CHECK-NEXT:    ret float [[FABS]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call nsz float @llvm.fabs.f32(float [[X:%.*]])
+; CHECK-NEXT:    ret float [[TMP1]]
 ;
   %lezero = fcmp nsz ole float %x, -0.0
   %negx = fsub float -0.0, %x
   %fabs = select i1 %lezero, float %negx, float %x
   ret float %fabs
 }
 
 ; X > 0.0 ? X : (0.0 - X) --> fabs(X)
 
 define <2 x float> @select_fcmp_nsz_ogt_zero(<2 x float> %x) {
 ; CHECK-LABEL: @select_fcmp_nsz_ogt_zero(
-; CHECK-NEXT:    [[GTZERO:%.*]] = fcmp nsz arcp ogt <2 x float> [[X:%.*]], zeroinitializer
-; CHECK-NEXT:    [[NEGX:%.*]] = fsub <2 x float> <float -0.000000e+00, float -0.000000e+00>, [[X]]
-; CHECK-NEXT:    [[FABS:%.*]] = select <2 x i1> [[GTZERO]], <2 x float> [[X]], <2 x float> [[NEGX]]
-; CHECK-NEXT:    ret <2 x float> [[FABS]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call nsz arcp <2 x float> @llvm.fabs.v2f32(<2 x float> [[X:%.*]])
+; CHECK-NEXT:    ret <2 x float> [[TMP1]]
 ;
   %gtzero = fcmp nsz arcp ogt <2 x float> %x, zeroinitializer
   %negx = fsub <2 x float> <float -0.0, float -0.0>, %x
   %fabs = select <2 x i1> %gtzero, <2 x float> %x, <2 x float> %negx
   ret <2 x float> %fabs
 }
 
 ; X > -0.0 ? X : (0.0 - X) --> fabs(X)
 
 define half @select_fcmp_nsz_ogt_negzero(half %x) {
 ; CHECK-LABEL: @select_fcmp_nsz_ogt_negzero(
-; CHECK-NEXT:    [[GTZERO:%.*]] = fcmp fast ogt half [[X:%.*]], 0xH8000
-; CHECK-NEXT:    [[NEGX:%.*]] = fsub half 0xH0000, [[X]]
-; CHECK-NEXT:    [[FABS:%.*]] = select i1 [[GTZERO]], half [[X]], half [[NEGX]]
-; CHECK-NEXT:    ret half [[FABS]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call fast half @llvm.fabs.f16(half [[X:%.*]])
+; CHECK-NEXT:    ret half [[TMP1]]
 ;
   %gtzero = fcmp fast ogt half %x, -0.0
   %negx = fsub half 0.0, %x
   %fabs = select i1 %gtzero, half %x, half %negx
   ret half %fabs
 }
 
 ; X > 0.0 ? X : (0.0 - X) --> fabs(X)
 
 define <2 x double> @select_fcmp_nsz_oge_zero(<2 x double> %x) {
 ; CHECK-LABEL: @select_fcmp_nsz_oge_zero(
-; CHECK-NEXT:    [[GEZERO:%.*]] = fcmp reassoc nsz oge <2 x double> [[X:%.*]], zeroinitializer
-; CHECK-NEXT:    [[NEGX:%.*]] = fsub <2 x double> <double -0.000000e+00, double -0.000000e+00>, [[X]]
-; CHECK-NEXT:    [[FABS:%.*]] = select <2 x i1> [[GEZERO]], <2 x double> [[X]], <2 x double> [[NEGX]]
-; CHECK-NEXT:    ret <2 x double> [[FABS]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call reassoc nsz <2 x double> @llvm.fabs.v2f64(<2 x double> [[X:%.*]])
+; CHECK-NEXT:    ret <2 x double> [[TMP1]]
 ;
   %gezero = fcmp nsz reassoc oge <2 x double> %x, zeroinitializer
   %negx = fsub <2 x double> <double -0.0, double -0.0>, %x
   %fabs = select <2 x i1> %gezero, <2 x double> %x, <2 x double> %negx
   ret <2 x double> %fabs
 }
 
 ; X > -0.0 ? X : (0.0 - X) --> fabs(X)
 
 define half @select_fcmp_nsz_oge_negzero(half %x) {
 ; CHECK-LABEL: @select_fcmp_nsz_oge_negzero(
-; CHECK-NEXT:    [[GEZERO:%.*]] = fcmp nsz oge half [[X:%.*]], 0xH8000
-; CHECK-NEXT:    [[NEGX:%.*]] = fsub half 0xH8000, [[X]]
-; CHECK-NEXT:    [[FABS:%.*]] = select i1 [[GEZERO]], half [[X]], half [[NEGX]]
-; CHECK-NEXT:    ret half [[FABS]]
+; CHECK-NEXT:    [[TMP1:%.*]] = call nsz half @llvm.fabs.f16(half [[X:%.*]])
+; CHECK-NEXT:    ret half [[TMP1]]
 ;
   %gezero = fcmp nsz oge half %x, -0.0
   %negx = fsub half -0.0, %x
   %fabs = select i1 %gezero, half %x, half %negx
   ret half %fabs
 }