[InstCombine] canonicalize fcmp ord/uno with constants to null constant

[spatel]

Summary

This is a preliminary step towards solving the remaining part of PR27145 - IR for isfinite():
https://bugs.llvm.org/show_bug.cgi?id=27145

In order to solve that one more generally, we need to add matching for and/or of fcmp ord/uno with a constant operand.

But while looking at those patterns, I realized we were missing a canonicalization for nonzero constants. By transforming everything to 0.0, we can simplify the existing code in foldLogicOfFCmps() and pick up missing vector folds.

[spatel]

Patch updated:
NFC, but use the m_Zero() matcher to further simplify the code and make the formulas in the code comments match.

[efriedma]

lib/IR/Constants.cpp

224 ↗

(On Diff #113711)

This handling of undef makes sense where you're using it, but is a bit weird for a function named "isKnownNeverNaN".

Would it make sense for this to be a utility that operates on arbitrary values, rather than constants? For example, we can assume the result of an sitofp is never a NaN.

[spatel]

lib/IR/Constants.cpp

224 ↗

(On Diff #113711)

Sure - I stole the name from the DAG version:
bool SelectionDAG::isKnownNeverNaN(SDValue Op) const

We'd add it to ValueTracking for the IR version?

[efriedma]

ValueTracking seems fine, yes, alongside CannotBeOrderedLessThanZero etc.

[spatel]

Patch updated:

1. Create a ValueTracking isKnownNeverNaN() to handle NaNs more generally (steal more from the DAG version).
2. Potentially replace both operands of fcmp ord/uno using that analysis.
3. Add tests with FMF to show those folds are working.

[efriedma]

LGTM

llvm/trunk/include/llvm/Analysis/ValueTracking.h

class Value;
   ///
   ///      NaN --> true
   ///       +0 --> true
   ///       -0 --> true
   ///   x > +0 --> true
   ///   x < -0 --> false
   bool CannotBeOrderedLessThanZero(const Value *V, const TargetLibraryInfo *TLI);
 
+  /// Return true if the floating-point scalar value is not a NaN or if the
+  /// floating-point vector value has no NaN elements. Return false if a value
+  /// could ever be NaN.
+  bool isKnownNeverNaN(const Value *V);
+
   /// Return true if we can prove that the specified FP value's sign bit is 0.
   ///
   ///      NaN --> true/false (depending on the NaN's sign bit)
   ///       +0 --> true
   ///       -0 --> false
   ///   x > +0 --> true
   ///   x < -0 --> false
   bool SignBitMustBeZero(const Value *V, const TargetLibraryInfo *TLI);

llvm/trunk/lib/Analysis/ValueTracking.cpp

bool llvm::CannotBeOrderedLessThanZero(const Value *V,
                                        const TargetLibraryInfo *TLI) {
   return cannotBeOrderedLessThanZeroImpl(V, TLI, false, 0);
 }
 
 bool llvm::SignBitMustBeZero(const Value *V, const TargetLibraryInfo *TLI) {
   return cannotBeOrderedLessThanZeroImpl(V, TLI, true, 0);
 }
 
+bool llvm::isKnownNeverNaN(const Value *V) {
+  assert(V->getType()->isFPOrFPVectorTy() && "Querying for NaN on non-FP type");
+
+  // If we're told that NaNs won't happen, assume they won't.
+  if (auto *FPMathOp = dyn_cast<FPMathOperator>(V))
+    if (FPMathOp->hasNoNaNs())
+      return true;
+
+  // TODO: Handle instructions and potentially recurse like other 'isKnown'
+  // functions. For example, the result of sitofp is never NaN.
+
+  // Handle scalar constants.
+  if (auto *CFP = dyn_cast<ConstantFP>(V))
+    return !CFP->isNaN();
+
+  // Bail out for constant expressions, but try to handle vector constants.
+  if (!V->getType()->isVectorTy() || !isa<Constant>(V))
+    return false;
+
+  // For vectors, verify that each element is not NaN.
+  unsigned NumElts = V->getType()->getVectorNumElements();
+  for (unsigned i = 0; i != NumElts; ++i) {
+    Constant *Elt = cast<Constant>(V)->getAggregateElement(i);
+    if (!Elt)
+      return false;
+    if (isa<UndefValue>(Elt))
+      continue;
+    auto *CElt = dyn_cast<ConstantFP>(Elt);
+    if (!CElt || CElt->isNaN())
+      return false;
+  }
+  // All elements were confirmed not-NaN or undefined.
+  return true;
+}
+
 /// If the specified value can be set by repeating the same byte in memory,
 /// return the i8 value that it is represented with.  This is
 /// true for all i8 values obviously, but is also true for i32 0, i32 -1,
 /// i16 0xF0F0, double 0.0 etc.  If the value can't be handled with a repeated
 /// byte store (e.g. i16 0x1234), return null.
 Value *llvm::isBytewiseValue(Value *V) {
   // All byte-wide stores are splatable, even of arbitrary variables.
   if (V->getType()->isIntegerTy(8)) return V;

llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

if (LHS0 == RHS0 && LHS1 == RHS1) {
     return getFCmpValue(NewPred, LHS0, LHS1, Builder);
   }
 
   if ((PredL == FCmpInst::FCMP_ORD && PredR == FCmpInst::FCMP_ORD && IsAnd) ||
       (PredL == FCmpInst::FCMP_UNO && PredR == FCmpInst::FCMP_UNO && !IsAnd)) {
     if (LHS0->getType() != RHS0->getType())
       return nullptr;
 
-    auto *LHSC = dyn_cast<ConstantFP>(LHS1);
-    auto *RHSC = dyn_cast<ConstantFP>(RHS1);
-    if (LHSC && RHSC) {
-      assert(!LHSC->getValueAPF().isNaN() && !RHSC->getValueAPF().isNaN() &&
-             "Failed to simplify fcmp ord/uno with NAN operand");
-      // Ignore the constants because they can't be NANs:
-      // (fcmp ord x, c) & (fcmp ord y, c)  -> (fcmp ord x, y)
-      // (fcmp uno x, c) & (fcmp uno y, c)  -> (fcmp uno x, y)
-      return Builder.CreateFCmp(PredL, LHS0, RHS0);
-    }
-
-    // Handle vector zeros. This occurs because the canonical form of
-    // "fcmp ord/uno x,x" is "fcmp ord/uno x, 0".
-    if (isa<ConstantAggregateZero>(LHS1) &&
-        isa<ConstantAggregateZero>(RHS1))
+    // FCmp canonicalization ensures that (fcmp ord/uno X, X) and
+    // (fcmp ord/uno X, C) will be transformed to (fcmp X, 0.0).
+    if (match(LHS1, m_Zero()) && LHS1 == RHS1)
+      // Ignore the constants because they are obviously not NANs:
+      // (fcmp ord x, 0.0) & (fcmp ord y, 0.0)  -> (fcmp ord x, y)
+      // (fcmp uno x, 0.0) | (fcmp uno y, 0.0)  -> (fcmp uno x, y)
       return Builder.CreateFCmp(PredL, LHS0, RHS0);
   }
 
   return nullptr;
 }
 
 /// Match De Morgan's Laws:
 /// (~A & ~B) == (~(A | B))

llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp

if (Op0 == Op1) {
     case FCmpInst::FCMP_OLE:    // True if ordered and less than or equal
       // Canonicalize these to be 'fcmp ord %X, 0.0'.
       I.setPredicate(FCmpInst::FCMP_ORD);
       I.setOperand(1, Constant::getNullValue(Op0->getType()));
       return &I;
     }
   }
 
+  // If we're just checking for a NaN (ORD/UNO) and have a non-NaN operand,
+  // then canonicalize the operand to 0.0.
+  if (Pred == CmpInst::FCMP_ORD || Pred == CmpInst::FCMP_UNO) {
+    if (!match(Op0, m_Zero()) && isKnownNeverNaN(Op0)) {
+      I.setOperand(0, ConstantFP::getNullValue(Op0->getType()));
+      return &I;
+    }
+    if (!match(Op1, m_Zero()) && isKnownNeverNaN(Op1)) {
+      I.setOperand(1, ConstantFP::getNullValue(Op0->getType()));
+      return &I;
+    }
+  }
+
   // Test if the FCmpInst instruction is used exclusively by a select as
   // part of a minimum or maximum operation. If so, refrain from doing
   // any other folding. This helps out other analyses which understand
   // non-obfuscated minimum and maximum idioms, such as ScalarEvolution
   // and CodeGen. And in this case, at least one of the comparison
   // operands has at least one user besides the compare (the select),
   // which would often largely negate the benefit of folding anyway.
   if (I.hasOneUse())

llvm/trunk/test/Transforms/InstCombine/and-fcmp.ll

;
   %cmp1 = fcmp ord float %x, 1.0
   %cmp2 = fcmp ord float %y, 2.0
   %and = and i1 %cmp1, %cmp2
   ret i1 %and
 }
 
 define <3 x i1> @fcmp_ord_nonzero_vec(<3 x float> %x, <3 x float> %y) {
 ; CHECK-LABEL: @fcmp_ord_nonzero_vec(
-; CHECK-NEXT:    [[CMP1:%.*]] = fcmp ord <3 x float> %x, <float 1.000000e+00, float 2.000000e+00, float 3.000000e+00>
-; CHECK-NEXT:    [[CMP2:%.*]] = fcmp ord <3 x float> %y, <float 3.000000e+00, float 2.000000e+00, float 1.000000e+00>
-; CHECK-NEXT:    [[AND:%.*]] = and <3 x i1> [[CMP1]], [[CMP2]]
-; CHECK-NEXT:    ret <3 x i1> [[AND]]
+; CHECK-NEXT:    [[TMP1:%.*]] = fcmp ord <3 x float> %x, %y
+; CHECK-NEXT:    ret <3 x i1> [[TMP1]]
 ;
   %cmp1 = fcmp ord <3 x float> %x, <float 1.0, float 2.0, float 3.0>
   %cmp2 = fcmp ord <3 x float> %y, <float 3.0, float 2.0, float 1.0>
   %and = and <3 x i1> %cmp1, %cmp2
   ret <3 x i1> %and
 }
 
 define i1 @auto_gen_0(double %a, double %b) {

llvm/trunk/test/Transforms/InstCombine/fcmp-special.ll

 ; CHECK-NEXT:    ret i1 [[F]]
 ;
   %f = fcmp ord float %x, 0.0
   ret i1 %f
 }
 
 define i1 @ord_nonzero(double %x) {
 ; CHECK-LABEL: @ord_nonzero(
-; CHECK-NEXT:    [[F:%.*]] = fcmp ord double %x, 3.000000e+00
+; CHECK-NEXT:    [[F:%.*]] = fcmp ord double %x, 0.000000e+00
 ; CHECK-NEXT:    ret i1 [[F]]
 ;
   %f = fcmp ord double %x, 3.0
   ret i1 %f
 }
 
 define i1 @ord_self(float %x) {
 ; CHECK-LABEL: @ord_self(

 ; CHECK-NEXT:    ret i1 [[F]]
 ;
   %f = fcmp uno double %x, 0.0
   ret i1 %f
 }
 
 define i1 @uno_nonzero(float %x) {
 ; CHECK-LABEL: @uno_nonzero(
-; CHECK-NEXT:    [[F:%.*]] = fcmp uno float %x, 3.000000e+00
+; CHECK-NEXT:    [[F:%.*]] = fcmp uno float %x, 0.000000e+00
 ; CHECK-NEXT:    ret i1 [[F]]
 ;
   %f = fcmp uno float %x, 3.0
   ret i1 %f
 }
 
 define i1 @uno_self(double %x) {
 ; CHECK-LABEL: @uno_self(

 ; CHECK-NEXT:    ret <2 x i1> [[F]]
 ;
   %f = fcmp ord <2 x double> %x, zeroinitializer
   ret <2 x i1> %f
 }
 
 define <2 x i1> @ord_nonzero_vec(<2 x float> %x) {
 ; CHECK-LABEL: @ord_nonzero_vec(
-; CHECK-NEXT:    [[F:%.*]] = fcmp ord <2 x float> %x, <float 3.000000e+00, float 5.000000e+00>
+; CHECK-NEXT:    [[F:%.*]] = fcmp ord <2 x float> %x, zeroinitializer
 ; CHECK-NEXT:    ret <2 x i1> [[F]]
 ;
   %f = fcmp ord <2 x float> %x, <float 3.0, float 5.0>
   ret <2 x i1> %f
 }
 
 define <2 x i1> @ord_self_vec(<2 x double> %x) {
 ; CHECK-LABEL: @ord_self_vec(

 ; CHECK-NEXT:    ret <2 x i1> [[F]]
 ;
   %f = fcmp uno <2 x float> %x, zeroinitializer
   ret <2 x i1> %f
 }
 
 define <2 x i1> @uno_nonzero_vec(<2 x double> %x) {
 ; CHECK-LABEL: @uno_nonzero_vec(
-; CHECK-NEXT:    [[F:%.*]] = fcmp uno <2 x double> %x, <double 3.000000e+00, double 5.000000e+00>
+; CHECK-NEXT:    [[F:%.*]] = fcmp uno <2 x double> %x, zeroinitializer
 ; CHECK-NEXT:    ret <2 x i1> [[F]]
 ;
   %f = fcmp uno <2 x double> %x, <double 3.0, double 5.0>
   ret <2 x i1> %f
 }
 
 define <2 x i1> @uno_self_vec(<2 x float> %x) {
 ; CHECK-LABEL: @uno_self_vec(
;
   %f = fcmp uno <2 x double> %x, <double 3.0, double 0x7FF00000FFFFFFFF>
   ret <2 x i1> %f
 }
 
 ; TODO: This could be handled in InstSimplify.
 
 define i1 @nnan_ops_to_fcmp_ord(float %x, float %y) {
 ; CHECK-LABEL: @nnan_ops_to_fcmp_ord(
-; CHECK-NEXT:    [[MUL:%.*]] = fmul nnan float %x, %y
-; CHECK-NEXT:    [[DIV:%.*]] = fdiv nnan float %x, %y
-; CHECK-NEXT:    [[CMP:%.*]] = fcmp ord float [[MUL]], [[DIV]]
-; CHECK-NEXT:    ret i1 [[CMP]]
+; CHECK-NEXT:    ret i1 true
 ;
   %mul = fmul nnan float %x, %y
   %div = fdiv nnan float %x, %y
   %cmp = fcmp ord float %mul, %div
   ret i1 %cmp
 }
 
 ; TODO: This could be handled in InstSimplify.
 
 define i1 @nnan_ops_to_fcmp_uno(float %x, float %y) {
 ; CHECK-LABEL: @nnan_ops_to_fcmp_uno(
-; CHECK-NEXT:    [[MUL:%.*]] = fmul nnan float %x, %y
-; CHECK-NEXT:    [[DIV:%.*]] = fdiv nnan float %x, %y
-; CHECK-NEXT:    [[CMP:%.*]] = fcmp uno float [[MUL]], [[DIV]]
-; CHECK-NEXT:    ret i1 [[CMP]]
+; CHECK-NEXT:    ret i1 false
 ;
   %mul = fmul nnan float %x, %y
   %div = fdiv nnan float %x, %y
   %cmp = fcmp uno float %mul, %div
   ret i1 %cmp
 }
 

llvm/trunk/test/Transforms/InstCombine/or-fcmp.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -instcombine -S | FileCheck %s
 
 define i1 @fcmp_uno_nonzero(float %x, float %y) {
 ; CHECK-LABEL: @fcmp_uno_nonzero(
 ; CHECK-NEXT:    [[TMP1:%.*]] = fcmp uno float %x, %y
 ; CHECK-NEXT:    ret i1 [[TMP1]]
 ;
   %cmp1 = fcmp uno float %x, 1.0
   %cmp2 = fcmp uno float %y, 2.0
   %or = or i1 %cmp1, %cmp2
   ret i1 %or
 }
 
 define <3 x i1> @fcmp_uno_nonzero_vec(<3 x float> %x, <3 x float> %y) {
 ; CHECK-LABEL: @fcmp_uno_nonzero_vec(
-; CHECK-NEXT:    [[CMP1:%.*]] = fcmp uno <3 x float> %x, <float 1.000000e+00, float 2.000000e+00, float 3.000000e+00>
-; CHECK-NEXT:    [[CMP2:%.*]] = fcmp uno <3 x float> %y, <float 3.000000e+00, float 2.000000e+00, float 1.000000e+00>
-; CHECK-NEXT:    [[OR:%.*]] = or <3 x i1> [[CMP1]], [[CMP2]]
-; CHECK-NEXT:    ret <3 x i1> [[OR]]
+; CHECK-NEXT:    [[TMP1:%.*]] = fcmp uno <3 x float> %x, %y
+; CHECK-NEXT:    ret <3 x i1> [[TMP1]]
 ;
   %cmp1 = fcmp uno <3 x float> %x, <float 1.0, float 2.0, float 3.0>
   %cmp2 = fcmp uno <3 x float> %y, <float 3.0, float 2.0, float 1.0>
   %or = or <3 x i1> %cmp1, %cmp2
   ret <3 x i1> %or
 }
 
 define i1 @auto_gen_0(double %a, double %b) {