Skip to content

Commit 4db1056

Browse files
authored
[ConstantFPRange] Address review comments. NFC. (#110793)
1. Address post-commit review comments #86483 (comment). 2. Move some NFC changes from #110082 to this patch.
1 parent bc91f3c commit 4db1056

File tree

3 files changed

+38
-20
lines changed

3 files changed

+38
-20
lines changed

llvm/include/llvm/IR/ConstantFPRange.h

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ class [[nodiscard]] ConstantFPRange {
5050

5151
void makeEmpty();
5252
void makeFull();
53-
bool isNaNOnly() const;
5453

5554
/// Initialize a full or empty set for the specified semantics.
5655
explicit ConstantFPRange(const fltSemantics &Sem, bool IsFullSet);
@@ -78,6 +77,9 @@ class [[nodiscard]] ConstantFPRange {
7877
/// Helper for (-inf, inf) to represent all finite values.
7978
static ConstantFPRange getFinite(const fltSemantics &Sem);
8079

80+
/// Helper for [-inf, inf] to represent all non-NaN values.
81+
static ConstantFPRange getNonNaN(const fltSemantics &Sem);
82+
8183
/// Create a range which doesn't contain NaNs.
8284
static ConstantFPRange getNonNaN(APFloat LowerVal, APFloat UpperVal) {
8385
return ConstantFPRange(std::move(LowerVal), std::move(UpperVal),
@@ -118,13 +120,13 @@ class [[nodiscard]] ConstantFPRange {
118120

119121
/// Produce the exact range such that all values in the returned range satisfy
120122
/// the given predicate with any value contained within Other. Formally, this
121-
/// returns the exact answer when the superset of 'union over all y in Other
122-
/// is exactly same as the subset of intersection over all y in Other.
123-
/// { x : fcmp op x y is true}'.
123+
/// returns { x : fcmp op x Other is true }.
124124
///
125125
/// Example: Pred = olt and Other = float 3 returns [-inf, 3)
126-
static ConstantFPRange makeExactFCmpRegion(FCmpInst::Predicate Pred,
127-
const APFloat &Other);
126+
/// If the exact answer is not representable as a ConstantFPRange, returns
127+
/// std::nullopt.
128+
static std::optional<ConstantFPRange>
129+
makeExactFCmpRegion(FCmpInst::Predicate Pred, const APFloat &Other);
128130

129131
/// Does the predicate \p Pred hold between ranges this and \p Other?
130132
/// NOTE: false does not mean that inverse predicate holds!
@@ -139,6 +141,7 @@ class [[nodiscard]] ConstantFPRange {
139141
bool containsNaN() const { return MayBeQNaN || MayBeSNaN; }
140142
bool containsQNaN() const { return MayBeQNaN; }
141143
bool containsSNaN() const { return MayBeSNaN; }
144+
bool isNaNOnly() const;
142145

143146
/// Get the semantics of this ConstantFPRange.
144147
const fltSemantics &getSemantics() const { return Lower.getSemantics(); }
@@ -157,10 +160,15 @@ class [[nodiscard]] ConstantFPRange {
157160
bool contains(const ConstantFPRange &CR) const;
158161

159162
/// If this set contains a single element, return it, otherwise return null.
160-
const APFloat *getSingleElement() const;
163+
/// If \p ExcludesNaN is true, return the non-NaN single element.
164+
const APFloat *getSingleElement(bool ExcludesNaN = false) const;
161165

162166
/// Return true if this set contains exactly one member.
163-
bool isSingleElement() const { return getSingleElement() != nullptr; }
167+
/// If \p ExcludesNaN is true, return true if this set contains exactly one
168+
/// non-NaN member.
169+
bool isSingleElement(bool ExcludesNaN = false) const {
170+
return getSingleElement(ExcludesNaN) != nullptr;
171+
}
164172

165173
/// Return true if the sign bit of all values in this range is 1.
166174
/// Return false if the sign bit of all values in this range is 0.
@@ -185,7 +193,7 @@ class [[nodiscard]] ConstantFPRange {
185193
/// another range.
186194
ConstantFPRange intersectWith(const ConstantFPRange &CR) const;
187195

188-
/// Return the range that results from the union of this range
196+
/// Return the smallest range that results from the union of this range
189197
/// with another range. The resultant range is guaranteed to include the
190198
/// elements of both sets, but may contain more.
191199
ConstantFPRange unionWith(const ConstantFPRange &CR) const;

llvm/lib/IR/ConstantFPRange.cpp

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,12 @@ static void canonicalizeRange(APFloat &Lower, APFloat &Upper) {
8181
}
8282

8383
ConstantFPRange::ConstantFPRange(APFloat LowerVal, APFloat UpperVal,
84-
bool MayBeQNaN, bool MayBeSNaN)
85-
: Lower(std::move(LowerVal)), Upper(std::move(UpperVal)) {
84+
bool MayBeQNaNVal, bool MayBeSNaNVal)
85+
: Lower(std::move(LowerVal)), Upper(std::move(UpperVal)),
86+
MayBeQNaN(MayBeQNaNVal), MayBeSNaN(MayBeSNaNVal) {
8687
assert(&Lower.getSemantics() == &Upper.getSemantics() &&
8788
"Should only use the same semantics");
8889
assert(!isNonCanonicalEmptySet(Lower, Upper) && "Non-canonical form");
89-
this->MayBeQNaN = MayBeQNaN;
90-
this->MayBeSNaN = MayBeSNaN;
9190
}
9291

9392
ConstantFPRange ConstantFPRange::getFinite(const fltSemantics &Sem) {
@@ -103,6 +102,12 @@ ConstantFPRange ConstantFPRange::getNaNOnly(const fltSemantics &Sem,
103102
MayBeSNaN);
104103
}
105104

105+
ConstantFPRange ConstantFPRange::getNonNaN(const fltSemantics &Sem) {
106+
return ConstantFPRange(APFloat::getInf(Sem, /*Negative=*/true),
107+
APFloat::getInf(Sem, /*Negative=*/false),
108+
/*MayBeQNaN=*/false, /*MayBeSNaN=*/false);
109+
}
110+
106111
ConstantFPRange
107112
ConstantFPRange::makeAllowedFCmpRegion(FCmpInst::Predicate Pred,
108113
const ConstantFPRange &Other) {
@@ -117,9 +122,10 @@ ConstantFPRange::makeSatisfyingFCmpRegion(FCmpInst::Predicate Pred,
117122
return getEmpty(Other.getSemantics());
118123
}
119124

120-
ConstantFPRange ConstantFPRange::makeExactFCmpRegion(FCmpInst::Predicate Pred,
121-
const APFloat &Other) {
122-
return makeAllowedFCmpRegion(Pred, ConstantFPRange(Other));
125+
std::optional<ConstantFPRange>
126+
ConstantFPRange::makeExactFCmpRegion(FCmpInst::Predicate Pred,
127+
const APFloat &Other) {
128+
return std::nullopt;
123129
}
124130

125131
bool ConstantFPRange::fcmp(FCmpInst::Predicate Pred,
@@ -161,8 +167,8 @@ bool ConstantFPRange::contains(const ConstantFPRange &CR) const {
161167
strictCompare(CR.Upper, Upper) != APFloat::cmpGreaterThan;
162168
}
163169

164-
const APFloat *ConstantFPRange::getSingleElement() const {
165-
if (MayBeSNaN || MayBeQNaN)
170+
const APFloat *ConstantFPRange::getSingleElement(bool ExcludesNaN) const {
171+
if (!ExcludesNaN && (MayBeSNaN || MayBeQNaN))
166172
return nullptr;
167173
return Lower.bitwiseIsEqual(Upper) ? &Lower : nullptr;
168174
}
@@ -189,8 +195,8 @@ FPClassTest ConstantFPRange::classify() const {
189195
FPClassTest LowerMask = Lower.classify();
190196
FPClassTest UpperMask = Upper.classify();
191197
assert(LowerMask <= UpperMask && "Range is nan-only.");
192-
for (uint32_t I = LowerMask; I <= UpperMask; I <<= 1)
193-
Mask |= I;
198+
// Set all bits from log2(LowerMask) to log2(UpperMask).
199+
Mask |= (UpperMask << 1) - LowerMask;
194200
}
195201
return static_cast<FPClassTest>(Mask);
196202
}

llvm/unittests/IR/ConstantFPRangeTest.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,12 +263,16 @@ TEST_F(ConstantFPRangeTest, SingleElement) {
263263
EXPECT_EQ(*One.getSingleElement(), APFloat(1.0));
264264
EXPECT_EQ(*PosZero.getSingleElement(), APFloat::getZero(Sem));
265265
EXPECT_EQ(*PosInf.getSingleElement(), APFloat::getInf(Sem));
266+
ConstantFPRange PosZeroOrNaN = PosZero.unionWith(NaN);
267+
EXPECT_EQ(*PosZeroOrNaN.getSingleElement(/*ExcludesNaN=*/true),
268+
APFloat::getZero(Sem));
266269

267270
EXPECT_FALSE(Full.isSingleElement());
268271
EXPECT_FALSE(Empty.isSingleElement());
269272
EXPECT_TRUE(One.isSingleElement());
270273
EXPECT_FALSE(Some.isSingleElement());
271274
EXPECT_FALSE(Zero.isSingleElement());
275+
EXPECT_TRUE(PosZeroOrNaN.isSingleElement(/*ExcludesNaN=*/true));
272276
}
273277

274278
TEST_F(ConstantFPRangeTest, ExhaustivelyEnumerate) {

0 commit comments

Comments
 (0)