ValueTracking: Identify implied fp classes by general fcmp #66505

arsenm · 2023-09-15T12:36:58Z

Previously we could recognize exact class tests performed by
an fcmp with special values (0s, infs and smallest normal).
Expand this to recognize the implied classes by a compare with a general
constant. e.g. fcmp ogt x, 1 implies positive and non-0.

The API should be better merged with fcmpToClassTest but that
made the diff way bigger, will try to do that in a future
patch.

llvmbot · 2023-09-15T12:40:32Z

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-analysis

Changes

None --

Patch is 119.14 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/66505.diff

5 Files Affected:

(modified) llvm/include/llvm/Analysis/ValueTracking.h (+21)
(modified) llvm/lib/Analysis/ValueTracking.cpp (+167-27)
(modified) llvm/test/Transforms/Attributor/nofpclass-implied-by-fcmp.ll (+160-160)
(modified) llvm/test/Transforms/InstSimplify/assume-fcmp-constant-implies-class.ll (+90-180)
(modified) llvm/unittests/Analysis/ValueTrackingTest.cpp (+4-4)

diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h
index 695f2fecae885b7..6a2fac1961070e4 100644
--- a/llvm/include/llvm/Analysis/ValueTracking.h
+++ b/llvm/include/llvm/Analysis/ValueTracking.h
@@ -235,6 +235,27 @@ std::pair&lt;Value *, FPClassTest&gt; fcmpToClassTest(CmpInst::Predicate Pred,
                                                 const APFloat *ConstRHS,
                                                 bool LookThroughSrc = true);
 
+/// Compute the possible floating-point classes that \p LHS could be based on an
+/// fcmp returning true. Returns { TestedValue, ClassesIfTrue, ClassesIfFalse }
+///
+/// If the compare returns an exact class test, ClassesIfTrue == ~ClassesIfFalse
+///
+/// This is a less exact version of fcmpToClassTest (e.g. fcmpToClassTest will
+/// only succeed for a test of x &gt; 0 implies positive, but not x &gt; 1).
+///
+/// If \p LookThroughSrc is true, consider the input value when computing the
+/// mask. This may look through sign bit operations.
+///
+/// If \p LookThroughSrc is false, ignore the source value (i.e. the first pair
+/// element will always be LHS.
+///
+std::tuple&lt;Value *, FPClassTest, FPClassTest&gt;
+fcmpImpliesClass(CmpInst::Predicate Pred, const Function &amp;F, Value *LHS,
+                 const APFloat *ConstRHS, bool LookThroughSrc = true);
+std::tuple&lt;Value *, FPClassTest, FPClassTest&gt;
+fcmpImpliesClass(CmpInst::Predicate Pred, const Function &amp;F, Value *LHS,
+                 Value *RHS, bool LookThroughSrc = true);
+
 struct KnownFPClass {
   /// Floating-point classes the value could be one of.
   FPClassTest KnownFPClasses = fcAllFlags;
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index c4153b824c37e0a..99be7c0c9e7e9c9 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -4008,7 +4008,7 @@ std::pair&lt;Value *, FPClassTest&gt; llvm::fcmpToClassTest(FCmpInst::Predicate Pred,
                                                       bool LookThroughSrc) {
   const APFloat *ConstRHS;
   if (!match(RHS, m_APFloatAllowUndef(ConstRHS)))
-    return {nullptr, fcNone};
+    return {nullptr, fcAllFlags};
 
   return fcmpToClassTest(Pred, F, LHS, ConstRHS, LookThroughSrc);
 }
@@ -4030,7 +4030,7 @@ llvm::fcmpToClassTest(FCmpInst::Predicate Pred, const Function &amp;F, Value *LHS,
     // TODO: Handle DAZ by expanding masks to cover subnormal cases.
     if (Pred != FCmpInst::FCMP_ORD &amp;&amp; Pred != FCmpInst::FCMP_UNO &amp;&amp;
         !inputDenormalIsIEEE(F, LHS-&gt;getType()))
-      return {nullptr, fcNone};
+      return {nullptr, fcAllFlags};
 
     switch (Pred) {
     case FCmpInst::FCMP_OEQ: // Match x == 0.0
@@ -4067,7 +4067,7 @@ llvm::fcmpToClassTest(FCmpInst::Predicate Pred, const Function &amp;F, Value *LHS,
       break;
     }
 
-    return {nullptr, fcNone};
+    return {nullptr, fcAllFlags};
   }
 
   Value *Src = LHS;
@@ -4151,7 +4151,7 @@ llvm::fcmpToClassTest(FCmpInst::Predicate Pred, const Function &amp;F, Value *LHS,
     case FCmpInst::FCMP_OGE:
     case FCmpInst::FCMP_ULT: {
       if (ConstRHS-&gt;isNegative()) // TODO
-        return {nullptr, fcNone};
+        return {nullptr, fcAllFlags};
 
       // fcmp oge fabs(x), +inf -&gt; fcInf
       // fcmp oge x, +inf -&gt; fcPosInf
@@ -4165,14 +4165,14 @@ llvm::fcmpToClassTest(FCmpInst::Predicate Pred, const Function &amp;F, Value *LHS,
     case FCmpInst::FCMP_OGT:
     case FCmpInst::FCMP_ULE: {
       if (ConstRHS-&gt;isNegative())
-        return {nullptr, fcNone};
+        return {nullptr, fcAllFlags};
 
       // No value is ordered and greater than infinity.
       Mask = fcNone;
       break;
     }
     default:
-      return {nullptr, fcNone};
+      return {nullptr, fcAllFlags};
     }
   } else if (ConstRHS-&gt;isSmallestNormalized() &amp;&amp; !ConstRHS-&gt;isNegative()) {
     // Match pattern that&#x27;s used in __builtin_isnormal.
@@ -4201,14 +4201,14 @@ llvm::fcmpToClassTest(FCmpInst::Predicate Pred, const Function &amp;F, Value *LHS,
       break;
     }
     default:
-      return {nullptr, fcNone};
+      return {nullptr, fcAllFlags};
     }
   } else if (ConstRHS-&gt;isNaN()) {
     // fcmp o__ x, nan -&gt; false
     // fcmp u__ x, nan -&gt; true
     Mask = fcNone;
   } else
-    return {nullptr, fcNone};
+    return {nullptr, fcAllFlags};
 
   // Invert the comparison for the unordered cases.
   if (FCmpInst::isUnordered(Pred))
@@ -4217,6 +4217,140 @@ llvm::fcmpToClassTest(FCmpInst::Predicate Pred, const Function &amp;F, Value *LHS,
   return {Src, Mask};
 }
 
+std::tuple&lt;Value *, FPClassTest, FPClassTest&gt;
+llvm::fcmpImpliesClass(CmpInst::Predicate Pred, const Function &amp;F, Value *LHS,
+                       const APFloat *ConstRHS, bool LookThroughSrc) {
+  auto [Val, ClassMask] =
+      fcmpToClassTest(Pred, F, LHS, ConstRHS, LookThroughSrc);
+  if (Val)
+    return {Val, ClassMask, ~ClassMask};
+
+  FPClassTest RHSClass = ConstRHS-&gt;classify();
+  assert((RHSClass == fcPosNormal || RHSClass == fcNegNormal ||
+          RHSClass == fcPosSubnormal || RHSClass == fcNegSubnormal) &amp;&amp;
+         &quot;should have been recognized as an exact class test&quot;);
+
+  const bool IsNegativeRHS = (RHSClass &amp; fcNegative) == RHSClass;
+  const bool IsPositiveRHS = (RHSClass &amp; fcPositive) == RHSClass;
+
+  assert(IsNegativeRHS == ConstRHS-&gt;isNegative());
+  assert(IsPositiveRHS == !ConstRHS-&gt;isNegative());
+
+  Value *Src = LHS;
+  const bool IsFabs = LookThroughSrc &amp;&amp; match(LHS, m_FAbs(m_Value(Src)));
+
+  if (IsFabs)
+    RHSClass = llvm::inverse_fabs(RHSClass);
+
+  if (Pred == FCmpInst::FCMP_OEQ)
+    return {Src, RHSClass, fcAllFlags};
+
+  if (Pred == FCmpInst::FCMP_UEQ) {
+    FPClassTest Class = RHSClass | fcNan;
+    return {Src, Class, ~fcNan};
+  }
+
+  if (Pred == FCmpInst::FCMP_ONE)
+    return {Src, ~fcNan, RHSClass};
+
+  if (Pred == FCmpInst::FCMP_UNE)
+    return {Src, fcAllFlags, RHSClass};
+
+  if (IsNegativeRHS) {
+    // TODO: Handle fneg(fabs)
+    if (IsFabs) {
+      // fabs(x) o&gt; -k -&gt; fcmp ord x, x
+      // fabs(x) u&gt; -k -&gt; true
+      // fabs(x) o&lt; -k -&gt; false
+      // fabs(x) u&lt; -k -&gt; fcmp uno x, x
+      switch (Pred) {
+      case FCmpInst::FCMP_OGT:
+      case FCmpInst::FCMP_OGE:
+        return {Src, ~fcNan, fcNan};
+      case FCmpInst::FCMP_UGT:
+      case FCmpInst::FCMP_UGE:
+        return {Src, fcAllFlags, fcNone};
+      case FCmpInst::FCMP_OLT:
+      case FCmpInst::FCMP_OLE:
+        return {Src, fcNone, fcAllFlags};
+      case FCmpInst::FCMP_ULT:
+      case FCmpInst::FCMP_ULE:
+        return {Src, fcNan, ~fcNan};
+      default:
+        break;
+      }
+
+      return {nullptr, fcAllFlags, fcAllFlags};
+    }
+
+    FPClassTest ClassesLE = fcNegInf | fcNegNormal;
+    FPClassTest ClassesGE = fcPositive | fcNegZero | fcNegSubnormal;
+
+    if (ConstRHS-&gt;isDenormal())
+      ClassesLE |= fcNegSubnormal;
+    else
+      ClassesGE |= fcNegNormal;
+
+    switch (Pred) {
+    case FCmpInst::FCMP_OGT:
+    case FCmpInst::FCMP_OGE:
+      return {Src, ClassesGE, ~ClassesGE | RHSClass};
+    case FCmpInst::FCMP_UGT:
+    case FCmpInst::FCMP_UGE:
+      return {Src, ClassesGE | fcNan, ~(ClassesGE | fcNan) | RHSClass};
+    case FCmpInst::FCMP_OLT:
+    case FCmpInst::FCMP_OLE:
+      return {Src, ClassesLE, ~ClassesLE | RHSClass};
+    case FCmpInst::FCMP_ULT:
+    case FCmpInst::FCMP_ULE:
+      return {Src, ClassesLE | fcNan, ~(ClassesLE | fcNan) | RHSClass};
+    default:
+      break;
+    }
+  } else if (IsPositiveRHS) {
+    FPClassTest ClassesGE = fcPosNormal | fcPosInf;
+    FPClassTest ClassesLE = fcNegative | fcPosZero | fcPosNormal;
+    if (ConstRHS-&gt;isDenormal())
+      ClassesGE |= fcPosNormal;
+    else
+      ClassesLE |= fcPosSubnormal;
+
+    FPClassTest FalseClasses = RHSClass;
+    if (IsFabs) {
+      ClassesGE = llvm::inverse_fabs(ClassesGE);
+      ClassesLE = llvm::inverse_fabs(ClassesLE);
+    }
+
+    switch (Pred) {
+    case FCmpInst::FCMP_OGT:
+    case FCmpInst::FCMP_OGE:
+      return {Src, ClassesGE, ~ClassesGE | FalseClasses};
+    case FCmpInst::FCMP_UGT:
+    case FCmpInst::FCMP_UGE:
+      return {Src, ClassesGE | fcNan, ~(ClassesGE | fcNan) | FalseClasses};
+    case FCmpInst::FCMP_OLT:
+    case FCmpInst::FCMP_OLE:
+      return {Src, ClassesLE, ~ClassesLE | FalseClasses};
+    case FCmpInst::FCMP_ULT:
+    case FCmpInst::FCMP_ULE:
+      return {Src, ClassesLE | fcNan, ~(ClassesLE | fcNan) | FalseClasses};
+    default:
+      break;
+    }
+  }
+
+  return {nullptr, fcAllFlags, fcAllFlags};
+}
+
+std::tuple&lt;Value *, FPClassTest, FPClassTest&gt;
+llvm::fcmpImpliesClass(CmpInst::Predicate Pred, const Function &amp;F, Value *LHS,
+                       Value *RHS, bool LookThroughSrc) {
+  const APFloat *ConstRHS;
+  if (!match(RHS, m_APFloatAllowUndef(ConstRHS)))
+    return {nullptr, fcAllFlags, fcNone};
+  return fcmpImpliesClass(Pred, F, LHS, ConstRHS, LookThroughSrc);
+}
+
 static FPClassTest computeKnownFPClassFromAssumes(const Value *V,
                                                   const SimplifyQuery &amp;Q) {
   FPClassTest KnownFromAssume = fcAllFlags;
@@ -4241,18 +4375,21 @@ static FPClassTest computeKnownFPClassFromAssumes(const Value *V,
     Value *LHS, *RHS;
     uint64_t ClassVal = 0;
     if (match(I-&gt;getArgOperand(0), m_FCmp(Pred, m_Value(LHS), m_Value(RHS)))) {
-      auto [TestedValue, TestedMask] =
-          fcmpToClassTest(Pred, *F, LHS, RHS, true);
-      // First see if we can fold in fabs/fneg into the test.
-      if (TestedValue == V)
-        KnownFromAssume &amp;= TestedMask;
-      else {
-        // Try again without the lookthrough if we found a different source
-        // value.
-        auto [TestedValue, TestedMask] =
-            fcmpToClassTest(Pred, *F, LHS, RHS, false);
-        if (TestedValue == V)
-          KnownFromAssume &amp;= TestedMask;
+      const APFloat *CRHS;
+      if (match(RHS, m_APFloat(CRHS))) {
+        // First see if we can fold in fabs/fneg into the test.
+        auto [CmpVal, MaskIfTrue, MaskIfFalse] =
+            fcmpImpliesClass(Pred, *F, LHS, CRHS, true);
+        if (CmpVal == V)
+          KnownFromAssume &amp;= MaskIfTrue;
+        else {
+          // Try again without the lookthrough if we found a different source
+          // value.
+          auto [CmpVal, MaskIfTrue, MaskIfFalse] =
+              fcmpImpliesClass(Pred, *F, LHS, CRHS, false);
+          if (CmpVal == V)
+            KnownFromAssume &amp;= MaskIfTrue;
+        }
       }
     } else if (match(I-&gt;getArgOperand(0),
                      m_Intrinsic&lt;Intrinsic::is_fpclass&gt;(
@@ -4400,7 +4537,8 @@ void computeKnownFPClass(const Value *V, const APInt &amp;DemandedElts,
     FPClassTest FilterRHS = fcAllFlags;
 
     Value *TestedValue = nullptr;
-    FPClassTest TestedMask = fcNone;
+    FPClassTest MaskIfTrue = fcAllFlags;
+    FPClassTest MaskIfFalse = fcAllFlags;
     uint64_t ClassVal = 0;
     const Function *F = cast&lt;Instruction&gt;(Op)-&gt;getFunction();
     CmpInst::Predicate Pred;
@@ -4412,20 +4550,22 @@ void computeKnownFPClass(const Value *V, const APInt &amp;DemandedElts,
       // TODO: In some degenerate cases we can infer something if we try again
       // without looking through sign operations.
       bool LookThroughFAbsFNeg = CmpLHS != LHS &amp;&amp; CmpLHS != RHS;
-      std::tie(TestedValue, TestedMask) =
-          fcmpToClassTest(Pred, *F, CmpLHS, CmpRHS, LookThroughFAbsFNeg);
+      std::tie(TestedValue, MaskIfTrue, MaskIfFalse) =
+          fcmpImpliesClass(Pred, *F, CmpLHS, CmpRHS, LookThroughFAbsFNeg);
     } else if (match(Cond,
                      m_Intrinsic&lt;Intrinsic::is_fpclass&gt;(
                          m_Value(TestedValue), m_ConstantInt(ClassVal)))) {
-      TestedMask = static_cast&lt;FPClassTest&gt;(ClassVal);
+      FPClassTest TestedMask = static_cast&lt;FPClassTest&gt;(ClassVal);
+      MaskIfTrue = TestedMask;
+      MaskIfFalse = ~TestedMask;
     }
 
     if (TestedValue == LHS) {
       // match !isnan(x) ? x : y
-      FilterLHS = TestedMask;
-    } else if (TestedValue == RHS) {
+      FilterLHS = MaskIfTrue;
+    } else if (TestedValue == RHS) { // &amp;&amp; IsExactClass
       // match !isnan(x) ? y : x
-      FilterRHS = ~TestedMask;
+      FilterRHS = MaskIfFalse;
     }
 
     KnownFPClass Known2;
diff --git a/llvm/test/Transforms/Attributor/nofpclass-implied-by-fcmp.ll b/llvm/test/Transforms/Attributor/nofpclass-implied-by-fcmp.ll
index 396b8c84fc898c9..212a8eb2f2451f7 100644
--- a/llvm/test/Transforms/Attributor/nofpclass-implied-by-fcmp.ll
+++ b/llvm/test/Transforms/Attributor/nofpclass-implied-by-fcmp.ll
@@ -11,7 +11,7 @@ declare void @llvm.assume(i1 noundef)
 
 ; can&#x27;t be +inf
 define float @clamp_is_ogt_1_to_1(float %arg) {
-; CHECK-LABEL: define float @clamp_is_ogt_1_to_1(
+; CHECK-LABEL: define nofpclass(pinf) float @clamp_is_ogt_1_to_1(
 ; CHECK-SAME: float [[ARG:%.*]]) #[[ATTR2:[0-9]+]] {
 ; CHECK-NEXT:    [[IS_OGT_1:%.*]] = fcmp ogt float [[ARG]], 1.000000e+00
 ; CHECK-NEXT:    [[SELECT:%.*]] = select i1 [[IS_OGT_1]], float 1.000000e+00, float [[ARG]]
@@ -23,7 +23,7 @@ define float @clamp_is_ogt_1_to_1(float %arg) {
 }
 
 define float @clamp_is_ogt_1_to_1_commute(float %arg) {
-; CHECK-LABEL: define float @clamp_is_ogt_1_to_1_commute(
+; CHECK-LABEL: define nofpclass(pinf) float @clamp_is_ogt_1_to_1_commute(
 ; CHECK-SAME: float [[ARG:%.*]]) #[[ATTR2]] {
 ; CHECK-NEXT:    [[IS_ULE_1:%.*]] = fcmp ule float [[ARG]], 1.000000e+00
 ; CHECK-NEXT:    [[SELECT:%.*]] = select i1 [[IS_ULE_1]], float [[ARG]], float 1.000000e+00
@@ -36,7 +36,7 @@ define float @clamp_is_ogt_1_to_1_commute(float %arg) {
 
 ; can&#x27;t be +inf or nan
 define float @clamp_is_ugt_1_to_1(float %arg) {
-; CHECK-LABEL: define float @clamp_is_ugt_1_to_1(
+; CHECK-LABEL: define nofpclass(nan pinf) float @clamp_is_ugt_1_to_1(
 ; CHECK-SAME: float [[ARG:%.*]]) #[[ATTR2]] {
 ; CHECK-NEXT:    [[IS_UGT_1:%.*]] = fcmp ugt float [[ARG]], 1.000000e+00
 ; CHECK-NEXT:    [[SELECT:%.*]] = select i1 [[IS_UGT_1]], float 1.000000e+00, float [[ARG]]
@@ -49,7 +49,7 @@ define float @clamp_is_ugt_1_to_1(float %arg) {
 
 ; can&#x27;t be +inf or nan
 define float @clamp_is_ugt_1_to_1_commute(float %arg) {
-; CHECK-LABEL: define float @clamp_is_ugt_1_to_1_commute(
+; CHECK-LABEL: define nofpclass(nan pinf) float @clamp_is_ugt_1_to_1_commute(
 ; CHECK-SAME: float [[ARG:%.*]]) #[[ATTR2]] {
 ; CHECK-NEXT:    [[IS_OLE_1:%.*]] = fcmp ole float [[ARG]], 1.000000e+00
 ; CHECK-NEXT:    [[SELECT:%.*]] = select i1 [[IS_OLE_1]], float [[ARG]], float 1.000000e+00
@@ -62,7 +62,7 @@ define float @clamp_is_ugt_1_to_1_commute(float %arg) {
 
 ; can&#x27;t be +inf
 define float @clamp_is_oge_1_to_1(float %arg) {
-; CHECK-LABEL: define float @clamp_is_oge_1_to_1(
+; CHECK-LABEL: define nofpclass(pinf) float @clamp_is_oge_1_to_1(
 ; CHECK-SAME: float [[ARG:%.*]]) #[[ATTR2]] {
 ; CHECK-NEXT:    [[IS_OGE_1:%.*]] = fcmp oge float [[ARG]], 1.000000e+00
 ; CHECK-NEXT:    [[SELECT:%.*]] = select i1 [[IS_OGE_1]], float 1.000000e+00, float [[ARG]]
@@ -74,7 +74,7 @@ define float @clamp_is_oge_1_to_1(float %arg) {
 }
 
 define float @clamp_is_oge_1_to_1_commute(float %arg) {
-; CHECK-LABEL: define float @clamp_is_oge_1_to_1_commute(
+; CHECK-LABEL: define nofpclass(pinf) float @clamp_is_oge_1_to_1_commute(
 ; CHECK-SAME: float [[ARG:%.*]]) #[[ATTR2]] {
 ; CHECK-NEXT:    [[IS_ULT_1:%.*]] = fcmp ult float [[ARG]], 1.000000e+00
 ; CHECK-NEXT:    [[SELECT:%.*]] = select i1 [[IS_ULT_1]], float [[ARG]], float 1.000000e+00
@@ -87,7 +87,7 @@ define float @clamp_is_oge_1_to_1_commute(float %arg) {
 
 ; can&#x27;t be +inf or nan
 define float @clamp_is_uge_1_to_1(float %arg) {
-; CHECK-LABEL: define float @clamp_is_uge_1_to_1(
+; CHECK-LABEL: define nofpclass(nan pinf) float @clamp_is_uge_1_to_1(
 ; CHECK-SAME: float [[ARG:%.*]]) #[[ATTR2]] {
 ; CHECK-NEXT:    [[IS_UGT_1:%.*]] = fcmp uge float [[ARG]], 1.000000e+00
 ; CHECK-NEXT:    [[SELECT:%.*]] = select i1 [[IS_UGT_1]], float 1.000000e+00, float [[ARG]]
@@ -100,7 +100,7 @@ define float @clamp_is_uge_1_to_1(float %arg) {
 
 ; can&#x27;t be negative, zero, or denormal
 define float @clamp_is_olt_1_to_1(float %arg) {
-; CHECK-LABEL: define float @clamp_is_olt_1_to_1(
+; CHECK-LABEL: define nofpclass(ninf zero sub nnorm) float @clamp_is_olt_1_to_1(
 ; CHECK-SAME: float [[ARG:%.*]]) #[[ATTR2]] {
 ; CHECK-NEXT:    [[IS_OLT_1:%.*]] = fcmp olt float [[ARG]], 1.000000e+00
 ; CHECK-NEXT:    [[SELECT:%.*]] = select i1 [[IS_OLT_1]], float 1.000000e+00, float [[ARG]]
@@ -113,7 +113,7 @@ define float @clamp_is_olt_1_to_1(float %arg) {
 
 ; can&#x27;t be negative, zero, or denormal
 define float @clamp_is_olt_1_to_1_commute(float %arg) {
-; CHECK-LABEL: define float @clamp_is_olt_1_to_1_commute(
+; CHECK-LABEL: define nofpclass(ninf zero sub nnorm) float @clamp_is_olt_1_to_1_commute(
 ; CHECK-SAME: float [[ARG:%.*]]) #[[ATTR2]] {
 ; CHECK-NEXT:    [[IS_UGE_1:%.*]] = fcmp uge float [[ARG]], 1.000000e+00
 ; CHECK-NEXT:    [[SELECT:%.*]] = select i1 [[IS_UGE_1]], float [[ARG]], float 1.000000e+00
@@ -126,7 +126,7 @@ define float @clamp_is_olt_1_to_1_commute(float %arg) {
 
 ; can&#x27;t be negative or zero, nan or denormal
 define float @clamp_is_ult_1_to_1(float %arg) {
-; CHECK-LABEL: define float @clamp_is_ult_1_to_1(
+; CHECK-LABEL: define nofpclass(nan ninf zero sub nnorm) float @clamp_is_ult_1_to_1(
 ; CHECK-SAME: float [[ARG:%.*]]) #[[ATTR2]] {
 ; CHECK-NEXT:    [[IS_ULT_1:%.*]] = fcmp ult float [[ARG]], 1.000000e+00
 ; CHECK-NEXT:    [[SELECT:%.*]] = select i1 [[IS_ULT_1]], float 1.000000e+00, float [[ARG]]
@@ -139,7 +139,7 @@ define float @clamp_is_ult_1_to_1(float %arg) {
 
 ; can&#x27;t be negative or zero, nan or denormal
 define float @clamp_is_ult_1_to_1_commute(float %arg) {
-; CHECK-LABEL: define float @clamp_is_ult_1_to_1_commute(
+; CHECK-LABEL: define nofpclass(nan ninf zero sub nnorm) float @clamp_is_ult_1_to_1_commute(
 ; CHECK-SAME: float [[ARG:%.*]]) #[[ATTR2]] {
 ; CHECK-NEXT:    [[IS_OGE_1:%.*]] = fcmp oge float [[ARG]], 1.000000e+00
 ; CHECK-NEXT:    [[SELECT:%.*]] = select i1 [[IS_OGE_1]], float [[ARG]], float 1.000000e+00
@@ -152,7 +152,7 @@ define float @clamp_is_ult_1_to_1_commute(float %arg) {
 
 ; can&#x27;t be negative, zero or denormal
 define float @clamp_is_ole_1_to_1(float %arg) {
-; CHECK-LABEL: define float @clamp_is_ole_1_to_1(
+; CHECK-LABEL: define nofpclass(ninf zero sub nnorm) float @clamp_is_ole_1_to_1(
 ; CHECK-SAME: float [[ARG:%.*]]) #[[ATTR2]] {
 ; CHECK-NEXT:    [[IS_OLE_1:%.*]] = fcmp ole float [[ARG]], 1.000000e+00
 ; CHECK-NEXT:    [[SELECT:%.*]] = select i1 [[IS_OLE_1]], float 1.000000e+00, float [[ARG]]
@@ -165,7 +165,7 @@ define float @clamp_is_ole_1_to_1(float %arg) {
 
 ; can&#x27;t be negative or zero, nan or denormal
 define float @clamp_is_ule_1_to_1(float %arg) {
-; CHECK-LABEL: define float @clamp_is_ule_1_to_1(
+; CHECK-LABEL: define nofpclass(nan ninf zero sub nnorm) float @clamp_is_ule_1_to_1(
 ; CHECK-SAME: float [[ARG:%.*]]) #[[ATTR2]] {
 ; CHECK-NEXT:    [[IS_ULE_1:%.*]] = fcmp ule float [[ARG]], 1.000000e+00
 ; CHECK-NEXT:    [[SELECT:%.*]] = select i1 [[IS_ULE_1]], float 1.000000e+00, float [[ARG]]
@@ -178,7 +178,7 @@ define float @clamp_is_ule_1_to_1(float %arg) {
 
 ; can&#x27;t be negative or denormal
 define float @clamp_is_olt_1_to_0(float %arg) {
-; CHECK-LABEL: define float @clamp_is_olt_1_to_0(
+; CHECK-LABEL: define nofpclass(ninf nzero sub nnorm) float @clamp_is_olt_1_to_0(
 ; CHECK-SAME: float [[ARG:%.*]]) #[[ATTR2]] {
 ; ...

Previously we could recognize exact class tests performed by an fcmp with special values (0s, infs and smallest normal). Expand this to recognize the implied classes by a compare with a general constant. e.g. fcmp ogt x, 1 implies positive and non-0. The API should be better merged with fcmpToClassTest but that made the diff way bigger, will try to do that in a future patch.

arsenm · 2023-11-03T05:40:51Z

ping

spavloff

LGTM.

spavloff · 2023-11-06T18:00:13Z

llvm/lib/Analysis/ValueTracking.cpp

+    else
+      ClassesLE |= fcPosSubnormal;
+
+    FPClassTest FalseClasses = RHSClass;


IMHO RHSClass is more clear than FalseClass.

I think originally this was supposed to be a mutable copy while preserving the original argument value, but I don't see it being mutated now

spavloff · 2023-11-06T18:02:34Z

llvm/lib/Analysis/ValueTracking.cpp

+    if (IsFabs) {
+      ClassesGE = llvm::inverse_fabs(ClassesGE);
+      ClassesLE = llvm::inverse_fabs(ClassesLE);
+    }


One can expect that IsPositiveRHS should be symmetrical to IsNegativeRHS but here their implementations are different.

It's different because this is the case that isn't rooted at 0. We need to consider the values between 0 and the absolute value of the constant

ronlieb · 2023-11-10T12:52:23Z

build of libdevice running into issue

[ 67%] Generating acosD.bc

Assertion `(RHSClass == fcPosNormal || RHSClass == fcNegNormal || RHSClass == fcPosSubnormal || RHSClass == fcNegSubnormal) && "should have been recognized as an exact class test"' failed.

zmodem · 2023-11-10T13:45:44Z

Chromium is hitting the same assert. See the attached reproducer at https://bugs.chromium.org/p/chromium/issues/detail?id=1501322#c3

I'll revert to green for now.

…66505)" This causes asserts to fire: llvm/lib/Analysis/ValueTracking.cpp:4262: std::tuple<Value *, FPClassTest, FPClassTest> llvm::fcmpImpliesClass(CmpInst::Predicate, const Function &, Value *, const APFloat *, bool): Assertion `(RHSClass == fcPosNormal || RHSClass == fcNegNormal || RHSClass == fcPosSubnormal || RHSClass == fcNegSubnormal) && "should have been recognized as an exact class test"' failed. See comments on the PR. > Previously we could recognize exact class tests performed by > an fcmp with special values (0s, infs and smallest normal). > Expand this to recognize the implied classes by a compare with a general > constant. e.g. fcmp ogt x, 1 implies positive and non-0. > > The API should be better merged with fcmpToClassTest but that > made the diff way bigger, will try to do that in a future > patch. This reverts commit dc3faf0.

arsenm · 2023-11-10T13:47:53Z

Chromium is hitting the same assert. See the attached reproducer at https://bugs.chromium.org/p/chromium/issues/detail?id=1501322#c3

I'll revert to green for now.

I'm pushing the fix once ninja check completes

zmodem · 2023-11-10T13:51:10Z

I'm pushing the fix once ninja check completes

Sorry, I wouldn't have reverted if I knew you had a fix already.

Previously we could recognize exact class tests performed by an fcmp with special values (0s, infs and smallest normal). Expand this to recognize the implied classes by a compare with a general constant. e.g. fcmp ogt x, 1 implies positive and non-0. The API should be better merged with fcmpToClassTest but that made the diff way bigger, will try to do that in a future patch.

…lvm#66505)" This causes asserts to fire: llvm/lib/Analysis/ValueTracking.cpp:4262: std::tuple<Value *, FPClassTest, FPClassTest> llvm::fcmpImpliesClass(CmpInst::Predicate, const Function &, Value *, const APFloat *, bool): Assertion `(RHSClass == fcPosNormal || RHSClass == fcNegNormal || RHSClass == fcPosSubnormal || RHSClass == fcNegSubnormal) && "should have been recognized as an exact class test"' failed. See comments on the PR. > Previously we could recognize exact class tests performed by > an fcmp with special values (0s, infs and smallest normal). > Expand this to recognize the implied classes by a compare with a general > constant. e.g. fcmp ogt x, 1 implies positive and non-0. > > The API should be better merged with fcmpToClassTest but that > made the diff way bigger, will try to do that in a future > patch. This reverts commit dc3faf0.

…66505)" This reverts commit 96a0d71. Avoid assert with dynamic denormal-fp-math We don't recognize compares with 0 as an exact class test if we don't know the denormal mode. We could try to do better here, but it's probably not worth it. Fixes asserts reported after 1adce7d8e47e2438f99f91607760b825e5e3cc37

slackito · 2023-12-05T01:23:28Z

FYI I'm seeing the same assertion being triggered after d55692d in some Tensorflow tests.

assert.h assertion failed at [third_party/llvm/llvm-project/llvm/lib/Analysis/ValueTracking.cpp:4186](https://cs.corp.google.com/piper///depot/google3/third_party/llvm/llvm-project/llvm/lib/Analysis/ValueTracking.cpp?l=4186&ws=jgorbe/61988&snapshot=467) in std::tuple<Value *, FPClassTest, FPClassTest> llvm::fcmpImpliesClass(CmpInst::Predicate, const Function &, Value *, const APFloat *, bool): (RHSClass == fcPosNormal || RHSClass == fcNegNormal || RHSClass == fcPosSubnormal || RHSClass == fcNegSubnormal) && "should have been recognized as an exact class test"

I don't have a standalone repro yet.

metaflow · 2023-12-05T09:04:28Z

namely

tensorflow/core/kernels/mlir_generated:floor_mod_gpu_floor_mod_kernels_gpu_f32_f32_gen_test and
tensorflow/core/kernels/mlir_generated:floor_mod_gpu_floor_mod_kernels_gpu_f16_f16_gen_test

started to fail after this change with the assertiong above. I have contacted TF asking them to give a more detailed instructions. I will likely revert this again, sorry.

metaflow · 2023-12-05T09:56:48Z

here is a repro from TF (I have not checked by hand, looks like some issues with fp ops)
tf_ir.txt

…l fcmp (#66505)"" This reverts commit d55692d. See discussion in #66505: assertion fires in OSS build of TensorFlow.

arsenm · 2023-12-07T05:39:46Z

tf_ir.txt

How do I reproduce with this? Just running through opt with different levels and obvious passes isn't doing anything. Can you provide a pure opt reproducer?

…l fcmp (`llvm#66505`)"" This reverts commit `d55692d60d218f402ce107520daabed15f2d9ef6`.

arsenm · 2023-12-12T05:53:48Z

tf_ir.txt

How do I reproduce with this? Just running through opt with different levels and obvious passes isn't doing anything. Can you provide a pure opt reproducer?

Ping, this reproducer is not useful as-is

metaflow · 2023-12-12T06:52:55Z

@akuegel remember this test failure in TF last week? maybe you have additional information here?

akuegel · 2023-12-12T13:03:26Z

@akuegel remember this test failure in TF last week? maybe you have additional information here?

I have debugged now which class is detected, and it is FPClassTest::fcPosInf. Does that help?
I see that this code was added before relanding:

if (RHSClass == fcPosZero || RHSClass == fcNegZero)
    return {nullptr, fcAllFlags, fcAllFlags};

Any reason why fcPosInf, fcNegInf, etc. is not just treated in the same way? It seems the original assert was written with some assumption that doesn't hold. I also verified that changing it to:

if (RHSClass == fcPosZero || RHSClass == fcNegZero || RHSClass == fcPosInf)

still makes the generated FloorMod kernel work correctly. But of course I have no idea about ValueTracking logic, so whether that would be the right fix, I don't know. Also in case it is the right fix, what about the other FPClassTest classes that the assert would trigger on? Is the assert even needed if for any unexpected class we can just return {nullptr, fcAllFlags, fcAllFlags} ?

arsenm · 2023-12-19T04:06:31Z

@akuegel remember this test failure in TF last week? maybe you have additional information here?

I have debugged now which class is detected, and it is FPClassTest::fcPosInf. Does that help?

Not really, I want a test case which reproduces the failure. I don't want to spend unbounded time guessing on what fell out of this test case that managed hit this

if (RHSClass == fcPosZero || RHSClass == fcNegZero)
    return {nullptr, fcAllFlags, fcAllFlags};
Any reason why fcPosInf, fcNegInf, etc. is not just treated in the same way? It seems the original assert was written with some assumption that doesn't hold. I also verified that changing it to:

Infinities are simpler than 0 as they do not depend on the denormal mode. This will be a trivial fix if I can just get a reproducer

llvm/lib/Analysis/ValueTracking.cpp

jayfoad · 2023-12-19T11:26:35Z

llvm/lib/Analysis/ValueTracking.cpp

+         "should have been recognized as an exact class test");
+
+  const bool IsNegativeRHS = (RHSClass & fcNegative) == RHSClass;
+  const bool IsPositiveRHS = (RHSClass & fcPositive) == RHSClass;


Having two flags seems overly general. RHS is always either known positive or known negative.

Not for a nan

Then the asserts immediately below must be broken since they effectively assert that IsNegativeRHS == !IsPositiveRHS.

Turns out nans can't reach here and I wrote this this way for the benefit of a future patch to generalize the RHS handling to non-constants

llvm/lib/Analysis/ValueTracking.cpp

jayfoad · 2023-12-19T11:41:53Z

llvm/lib/Analysis/ValueTracking.cpp

+      if (match(RHS, m_APFloat(CRHS))) {
+        // First see if we can fold in fabs/fneg into the test.
+        auto [CmpVal, MaskIfTrue, MaskIfFalse] =
+            fcmpImpliesClass(Pred, *F, LHS, CRHS, true);


Could you make just one call to fcmpImpliesClass, passing in LHS != V for the LookThroughSrc argument?

I tried this in an earlier version of fcmpToClassTest but it missed some cases. Seems to not if I make this change, but I think it will still miss something

akuegel · 2023-12-19T13:51:54Z

@akuegel remember this test failure in TF last week? maybe you have additional information here?

I have debugged now which class is detected, and it is FPClassTest::fcPosInf. Does that help?

Not really, I want a test case which reproduces the failure. I don't want to spend unbounded time guessing on what fell out of this test case that managed hit this
if (RHSClass == fcPosZero || RHSClass == fcNegZero)
    return {nullptr, fcAllFlags, fcAllFlags};
Any reason why fcPosInf, fcNegInf, etc. is not just treated in the same way? It seems the original assert was written with some assumption that doesn't hold. I also verified that changing it to:
Infinities are simpler than 0 as they do not depend on the denormal mode. This will be a trivial fix if I can just get a reproducer

Unfortunately my knowledge about the LLVM side is quite limited. I don't know how to create a reproducer for this, I tried my best to at least debug with what FPClassTest value we are hitting the assert.

arsenm · 2024-01-22T15:46:54Z

@akuegel remember this test failure in TF last week? maybe you have additional information here?

I have debugged now which class is detected, and it is FPClassTest::fcPosInf. Does that help?

Unfortunately my knowledge about the LLVM side is quite limited. I don't know how to create a reproducer for this, I tried my best to at least debug with what FPClassTest value we are hitting the assert.

Can you try again? My best guess is you haven't linked in __nv_fmodf, so this is just a stub

arsenm · 2024-01-23T05:17:46Z

Can you try again? My best guess is you haven't linked in __nv_fmodf, so this is just a stub

I'm assuming #79095 will fix it

…66505)" This reverts commit 0d0c229. Includes a bug fix for fcmp one handling, as well as for positive constants.

Rushing this one out before vacation starts. Refactoring on top of #66505

arsenm added the floating-point Floating-point math label Sep 15, 2023

arsenm requested review from jayfoad, spavloff, andykaylor, kpneal and jcranmer-intel September 15, 2023 12:36

llvmbot added llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Sep 15, 2023

arsenm changed the title ~~Fcmp implies class 2~~ ValueTracking: Identify implied fp classes by general fcmp Sep 15, 2023

arsenm mentioned this pull request Sep 15, 2023

ValueTracking: Merge fcmpImpliesClass and fcmpToClassTest #66522

Merged

arsenm force-pushed the fcmp-implies-class-2 branch from bc5f154 to 47c1087 Compare October 5, 2023 20:11

arsenm requested a review from nikic as a code owner October 5, 2023 20:11

arsenm force-pushed the fcmp-implies-class-2 branch from 47c1087 to 0375094 Compare November 3, 2023 05:40

spavloff approved these changes Nov 6, 2023

View reviewed changes

Remove FalseClasses variable

7fd9a9c

arsenm merged commit dc3faf0 into llvm:main Nov 10, 2023

arsenm deleted the fcmp-implies-class-2 branch November 10, 2023 02:39

metaflow added a commit that referenced this pull request Dec 5, 2023

Revert "Reapply "ValueTracking: Identify implied fp classes by genera…

0d0c229

…l fcmp (#66505)"" This reverts commit d55692d. See discussion in #66505: assertion fires in OSS build of TensorFlow.

stellaraccident added a commit to iree-org/llvm-project that referenced this pull request Dec 8, 2023

Revert "Reapply "ValueTracking: Identify implied fp classes by genera…

6cdda88

…l fcmp (`llvm#66505`)"" This reverts commit `d55692d60d218f402ce107520daabed15f2d9ef6`.

jayfoad reviewed Dec 19, 2023

View reviewed changes

arsenm added a commit that referenced this pull request Jan 25, 2024

Reapply "ValueTracking: Identify implied fp classes by general fcmp (#…

a46422a

…66505)" This reverts commit 0d0c229. Includes a bug fix for fcmp one handling, as well as for positive constants.

arsenm added a commit that referenced this pull request Jan 27, 2024

ValueTracking: Merge fcmpImpliesClass and fcmpToClassTest (#66522)

e44d3b3

Rushing this one out before vacation starts. Refactoring on top of #66505

ValueTracking: Identify implied fp classes by general fcmp #66505

ValueTracking: Identify implied fp classes by general fcmp #66505

Uh oh!

Conversation

arsenm commented Sep 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Sep 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arsenm commented Nov 3, 2023

Uh oh!

spavloff left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ronlieb commented Nov 10, 2023

Uh oh!

zmodem commented Nov 10, 2023

Uh oh!

arsenm commented Nov 10, 2023

Uh oh!

zmodem commented Nov 10, 2023

Uh oh!

slackito commented Dec 5, 2023

Uh oh!

metaflow commented Dec 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metaflow commented Dec 5, 2023

Uh oh!

arsenm commented Dec 7, 2023

Uh oh!

arsenm commented Dec 12, 2023

Uh oh!

metaflow commented Dec 12, 2023

Uh oh!

akuegel commented Dec 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arsenm commented Dec 19, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akuegel commented Dec 19, 2023

Uh oh!

arsenm commented Jan 22, 2024

Uh oh!

arsenm commented Jan 23, 2024

Uh oh!

Uh oh!

arsenm commented Sep 15, 2023 •

edited

Loading

llvmbot commented Sep 15, 2023 •

edited

Loading

metaflow commented Dec 5, 2023 •

edited

Loading

akuegel commented Dec 12, 2023 •

edited

Loading