[llvm] [ConstantFPRange] Address review comments. NFC. (PR #110793)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 1 22:33:02 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-ir
Author: Yingwei Zheng (dtcxzyw)
<details>
<summary>Changes</summary>
1. Address post-commit review comments https://github.com/llvm/llvm-project/pull/86483#discussion_r1782305961.
2. Move some NFC changes from https://github.com/llvm/llvm-project/pull/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
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) {
``````````
</details>
https://github.com/llvm/llvm-project/pull/110793
More information about the llvm-commits
mailing list