[ConstantFPRange] Address review comments. NFC.

[dtcxzyw]

1. Address post-commit review comments [LLVM][IR] Add constant range support for floating-point types #86483 (comment).
2. Move some NFC changes from [ConstantFPRange] Implement ConstantFPRange::makeAllowedFCmpRegion #110082 to this patch.

[llvmbot]

@llvm/pr-subscribers-llvm-ir

Author: Yingwei Zheng (dtcxzyw)

Changes

1. Address post-commit review comments [LLVM][IR] Add constant range support for floating-point types #86483 (comment).
2. Move some NFC changes from [ConstantFPRange] Implement ConstantFPRange::makeAllowedFCmpRegion #110082 to this patch.

---

Full diff: https://github.com/llvm/llvm-project/pull/110793.diff

3 Files Affected:

• (modified) llvm/include/llvm/IR/ConstantFPRange.h (+17-9)
• (modified) llvm/lib/IR/ConstantFPRange.cpp (+17-11)
• (modified) llvm/unittests/IR/ConstantFPRangeTest.cpp (+4)

diff --git a/llvm/include/llvm/IR/ConstantFPRange.h b/llvm/include/llvm/IR/ConstantFPRange.h
index 67f9f945d748ba..e240671296479b 100644
--- a/llvm/include/llvm/IR/ConstantFPRange.h
+++ b/llvm/include/llvm/IR/ConstantFPRange.h
@@ -50,7 +50,6 @@ class [[nodiscard]] ConstantFPRange {
 
   void makeEmpty();
   void makeFull();
-  bool isNaNOnly() const;
 
   /// Initialize a full or empty set for the specified semantics.
   explicit ConstantFPRange(const fltSemantics &Sem, bool IsFullSet);
@@ -78,6 +77,9 @@ class [[nodiscard]] ConstantFPRange {
   /// Helper for (-inf, inf) to represent all finite values.
   static ConstantFPRange getFinite(const fltSemantics &Sem);
 
+  /// Helper for [-inf, inf] to represent all non-NaN values.
+  static ConstantFPRange getNonNaN(const fltSemantics &Sem);
+
   /// Create a range which doesn't contain NaNs.
   static ConstantFPRange getNonNaN(APFloat LowerVal, APFloat UpperVal) {
     return ConstantFPRange(std::move(LowerVal), std::move(UpperVal),
@@ -118,13 +120,13 @@ class [[nodiscard]] ConstantFPRange {
 
   /// Produce the exact range such that all values in the returned range satisfy
   /// the given predicate with any value contained within Other. Formally, this
-  /// returns the exact answer when the superset of 'union over all y in Other
-  /// is exactly same as the subset of intersection over all y in Other.
-  /// { x : fcmp op x y is true}'.
+  /// returns { x : fcmp op x Other is true }.
   ///
   /// Example: Pred = olt and Other = float 3 returns [-inf, 3)
-  static ConstantFPRange makeExactFCmpRegion(FCmpInst::Predicate Pred,
-                                             const APFloat &Other);
+  /// If the exact answer is not representable as a ConstantFPRange, returns
+  /// std::nullopt.
+  static std::optional<ConstantFPRange>
+  makeExactFCmpRegion(FCmpInst::Predicate Pred, const APFloat &Other);
 
   /// Does the predicate \p Pred hold between ranges this and \p Other?
   /// NOTE: false does not mean that inverse predicate holds!
@@ -139,6 +141,7 @@ class [[nodiscard]] ConstantFPRange {
   bool containsNaN() const { return MayBeQNaN || MayBeSNaN; }
   bool containsQNaN() const { return MayBeQNaN; }
   bool containsSNaN() const { return MayBeSNaN; }
+  bool isNaNOnly() const;
 
   /// Get the semantics of this ConstantFPRange.
   const fltSemantics &getSemantics() const { return Lower.getSemantics(); }
@@ -157,10 +160,15 @@ class [[nodiscard]] ConstantFPRange {
   bool contains(const ConstantFPRange &CR) const;
 
   /// If this set contains a single element, return it, otherwise return null.
-  const APFloat *getSingleElement() const;
+  /// If \p ExcludesNaN is true, return the non-NaN single element.
+  const APFloat *getSingleElement(bool ExcludesNaN = false) const;
 
   /// Return true if this set contains exactly one member.
-  bool isSingleElement() const { return getSingleElement() != nullptr; }
+  /// If \p ExcludesNaN is true, return true if this set contains exactly one
+  /// non-NaN member.
+  bool isSingleElement(bool ExcludesNaN = false) const {
+    return getSingleElement(ExcludesNaN) != nullptr;
+  }
 
   /// Return true if the sign bit of all values in this range is 1.
   /// Return false if the sign bit of all values in this range is 0.
@@ -185,7 +193,7 @@ class [[nodiscard]] ConstantFPRange {
   /// another range.
   ConstantFPRange intersectWith(const ConstantFPRange &CR) const;
 
-  /// Return the range that results from the union of this range
+  /// Return the smallest range that results from the union of this range
   /// with another range.  The resultant range is guaranteed to include the
   /// elements of both sets, but may contain more.
   ConstantFPRange unionWith(const ConstantFPRange &CR) const;
diff --git a/llvm/lib/IR/ConstantFPRange.cpp b/llvm/lib/IR/ConstantFPRange.cpp
index 957701891c8f37..3f63ca8e62c258 100644
--- a/llvm/lib/IR/ConstantFPRange.cpp
+++ b/llvm/lib/IR/ConstantFPRange.cpp
@@ -81,13 +81,12 @@ static void canonicalizeRange(APFloat &Lower, APFloat &Upper) {
 }
 
 ConstantFPRange::ConstantFPRange(APFloat LowerVal, APFloat UpperVal,
-                                 bool MayBeQNaN, bool MayBeSNaN)
-    : Lower(std::move(LowerVal)), Upper(std::move(UpperVal)) {
+                                 bool MayBeQNaNVal, bool MayBeSNaNVal)
+    : Lower(std::move(LowerVal)), Upper(std::move(UpperVal)),
+      MayBeQNaN(MayBeQNaNVal), MayBeSNaN(MayBeSNaNVal) {
   assert(&Lower.getSemantics() == &Upper.getSemantics() &&
          "Should only use the same semantics");
   assert(!isNonCanonicalEmptySet(Lower, Upper) && "Non-canonical form");
-  this->MayBeQNaN = MayBeQNaN;
-  this->MayBeSNaN = MayBeSNaN;
 }
 
 ConstantFPRange ConstantFPRange::getFinite(const fltSemantics &Sem) {
@@ -103,6 +102,12 @@ ConstantFPRange ConstantFPRange::getNaNOnly(const fltSemantics &Sem,
                          MayBeSNaN);
 }
 
+ConstantFPRange ConstantFPRange::getNonNaN(const fltSemantics &Sem) {
+  return ConstantFPRange(APFloat::getInf(Sem, /*Negative=*/true),
+                         APFloat::getInf(Sem, /*Negative=*/false),
+                         /*MayBeQNaN=*/false, /*MayBeSNaN=*/false);
+}
+
 ConstantFPRange
 ConstantFPRange::makeAllowedFCmpRegion(FCmpInst::Predicate Pred,
                                        const ConstantFPRange &Other) {
@@ -117,9 +122,10 @@ ConstantFPRange::makeSatisfyingFCmpRegion(FCmpInst::Predicate Pred,
   return getEmpty(Other.getSemantics());
 }
 
-ConstantFPRange ConstantFPRange::makeExactFCmpRegion(FCmpInst::Predicate Pred,
-                                                     const APFloat &Other) {
-  return makeAllowedFCmpRegion(Pred, ConstantFPRange(Other));
+std::optional<ConstantFPRange>
+ConstantFPRange::makeExactFCmpRegion(FCmpInst::Predicate Pred,
+                                     const APFloat &Other) {
+  return std::nullopt;
 }
 
 bool ConstantFPRange::fcmp(FCmpInst::Predicate Pred,
@@ -161,8 +167,8 @@ bool ConstantFPRange::contains(const ConstantFPRange &CR) const {
          strictCompare(CR.Upper, Upper) != APFloat::cmpGreaterThan;
 }
 
-const APFloat *ConstantFPRange::getSingleElement() const {
-  if (MayBeSNaN || MayBeQNaN)
+const APFloat *ConstantFPRange::getSingleElement(bool ExcludesNaN) const {
+  if (!ExcludesNaN && (MayBeSNaN || MayBeQNaN))
     return nullptr;
   return Lower.bitwiseIsEqual(Upper) ? &Lower : nullptr;
 }
@@ -189,8 +195,8 @@ FPClassTest ConstantFPRange::classify() const {
     FPClassTest LowerMask = Lower.classify();
     FPClassTest UpperMask = Upper.classify();
     assert(LowerMask <= UpperMask && "Range is nan-only.");
-    for (uint32_t I = LowerMask; I <= UpperMask; I <<= 1)
-      Mask |= I;
+    // Set all bits from log2(LowerMask) to log2(UpperMask).
+    Mask |= (UpperMask << 1) - LowerMask;
   }
   return static_cast<FPClassTest>(Mask);
 }
diff --git a/llvm/unittests/IR/ConstantFPRangeTest.cpp b/llvm/unittests/IR/ConstantFPRangeTest.cpp
index 722e6566730da5..d228651b5129cc 100644
--- a/llvm/unittests/IR/ConstantFPRangeTest.cpp
+++ b/llvm/unittests/IR/ConstantFPRangeTest.cpp
@@ -263,12 +263,16 @@ TEST_F(ConstantFPRangeTest, SingleElement) {
   EXPECT_EQ(*One.getSingleElement(), APFloat(1.0));
   EXPECT_EQ(*PosZero.getSingleElement(), APFloat::getZero(Sem));
   EXPECT_EQ(*PosInf.getSingleElement(), APFloat::getInf(Sem));
+  ConstantFPRange PosZeroOrNaN = PosZero.unionWith(NaN);
+  EXPECT_EQ(*PosZeroOrNaN.getSingleElement(/*ExcludesNaN=*/true),
+            APFloat::getZero(Sem));
 
   EXPECT_FALSE(Full.isSingleElement());
   EXPECT_FALSE(Empty.isSingleElement());
   EXPECT_TRUE(One.isSingleElement());
   EXPECT_FALSE(Some.isSingleElement());
   EXPECT_FALSE(Zero.isSingleElement());
+  EXPECT_TRUE(PosZeroOrNaN.isSingleElement(/*ExcludesNaN=*/true));
 }
 
 TEST_F(ConstantFPRangeTest, ExhaustivelyEnumerate) {