[LLVM][IR] Add constant range support for floating-point types #86483
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ With the latest revision this PR passed the Python code formatter. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
@tschuett This patch is not ready for review. |
|
Sorry. The GlobalIsel combiner already uses ConstantRange. I would like to use ConstantFPRange as well. |
|
@llvm/pr-subscribers-llvm-support @llvm/pr-subscribers-llvm-ir Author: Yingwei Zheng (dtcxzyw) ChangesRelated issues: #68301 #70985 #82381 Full diff: https://github.com/llvm/llvm-project/pull/86483.diff 1 Files Affected:
diff --git a/llvm/include/llvm/IR/ConstantFPRange.h b/llvm/include/llvm/IR/ConstantFPRange.h
new file mode 100644
index 00000000000000..89b0cd0b7763b5
--- /dev/null
+++ b/llvm/include/llvm/IR/ConstantFPRange.h
@@ -0,0 +1,139 @@
+//===- ConstantFPRange.h - Represent a range for floating-point -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Represent a range of possible values that may occur when the program is run
+// for a floating-point value. This keeps track of a lower and upper bound for
+// the constant.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_IR_CONSTANTFPRANGE_H
+#define LLVM_IR_CONSTANTFPRANGE_H
+
+#include "llvm/ADT/APFloat.h"
+#include "llvm/IR/InstrTypes.h"
+#include "llvm/IR/Instruction.h"
+#include "llvm/Support/Compiler.h"
+#include <optional>
+
+namespace llvm {
+
+class raw_ostream;
+struct KnownFPClass;
+
+/// This class represents a range of floating-point values.
+class [[nodiscard]] ConstantFPRange {
+ APFloat Lower, Upper;
+ bool MaybeQNaN : 1;
+ bool MaybeSNaN : 1;
+ bool SignBitMaybeZero : 1;
+ bool SignBitMaybeOne : 1;
+
+ /// Create empty constant range with same semantics.
+ ConstantFPRange getEmpty() const {
+ return ConstantFPRange(getSemantics(), /*IsFullSet=*/false);
+ }
+
+ /// Create full constant range with same semantics.
+ ConstantFPRange getFull() const {
+ return ConstantFPRange(getSemantics(), /*IsFullSet=*/true);
+ }
+
+public:
+ /// Initialize a full or empty set for the specified semantics.
+ explicit ConstantFPRange(const fltSemantics &FloatSema, bool IsFullSet);
+
+ /// Initialize a range to hold the single specified value.
+ ConstantFPRange(const APFloat &Value);
+
+ /// Initialize a range of values explicitly.
+ ConstantFPRange(APFloat Lower, APFloat Upper, bool MaybeQNaN, bool MaybeSNaN,
+ bool SignBitMaybeZero, bool SignBitMaybeOne);
+
+ /// Create empty constant range with the given semantics.
+ static ConstantFPRange getEmpty(const fltSemantics &FloatSema) {
+ return ConstantFPRange(FloatSema, /*IsFullSet=*/false);
+ }
+
+ /// Create full constant range with the given semantics.
+ static ConstantFPRange getFull(const fltSemantics &FloatSema) {
+ return ConstantFPRange(FloatSema, /*IsFullSet=*/true);
+ }
+
+ /// Initialize a range based on a known floating-point classes constraint.
+ static ConstantFPRange fromKnownFPClass(const KnownFPClass &Known);
+
+ /// 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}'.
+ ///
+ /// Example: Pred = olt and Other = float 3 returns [-inf, 3)
+ static 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!
+ bool fcmp(FCmpInst::Predicate Pred, const ConstantFPRange &Other) const;
+
+ /// Return the lower value for this range.
+ const APFloat &getLower() const { return Lower; }
+
+ /// Return the upper value for this range.
+ const APFloat &getUpper() const { return Upper; }
+
+ /// Get the semantics of this ConstantFPRange.
+ const fltSemantics &getSemantics() const { return Lower.getSemantics(); }
+
+ /// Return true if this set contains all of the elements possible
+ /// for this data-type.
+ bool isFullSet() const;
+
+ /// Return true if this set contains no members.
+ bool isEmptySet() const;
+
+ /// Return true if the specified value is in the set.
+ bool contains(const APFloat &Val) const;
+
+ /// Return true if the other range is a subset of this one.
+ bool contains(const ConstantFPRange &CR) const;
+
+ /// If this set contains a single element, return it, otherwise return null.
+ const APFloat *getSingleElement() const;
+
+ /// Return true if this set contains exactly one member.
+ bool isSingleElement() const { return getSingleElement() != 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.
+ /// Otherwise, return std::nullopt.
+ std::optional<bool> getSignBit();
+
+ /// Return true if this range is equal to another range.
+ bool operator==(const ConstantFPRange &CR) const;
+ bool operator!=(const ConstantFPRange &CR) const { return !operator==(CR); }
+
+ /// Return known floating-point classes for values in this range.
+ KnownFPClass toKnownFPClass();
+
+ /// Print out the bounds to a stream.
+ void print(raw_ostream &OS) const;
+
+ /// Allow printing from a debugger easily.
+ void dump() const;
+};
+
+inline raw_ostream &operator<<(raw_ostream &OS, const ConstantFPRange &CR) {
+ CR.print(OS);
+ return OS;
+}
+
+} // end namespace llvm
+
+#endif // LLVM_IR_CONSTANTFPRANGE_H
|
arsenm
reviewed
Mar 26, 2024
Comment on lines
34
to
35
| bool SignBitMaybeZero : 1; | ||
| bool SignBitMaybeOne : 1; |
There was a problem hiding this comment.
Don't understand why these would be "MaybeZero" or "MaybeOne". Must be? Maybe would need to default to 1
dtcxzyw
force-pushed
the
constant-fp-range
branch
from
496d617 to
9928b14
Compare
Currently we use closed intervals and use |
arsenm
reviewed
Jun 21, 2024
Any thoughts? IMO there will be a significant maintenance burden for us if we plan to support wrapped sets. |
I think it's potentially useful. Some instructions only operate in a range of values. Maybe doesn't need to be in the initial support |
For integers, wrapped ranges are quite important because a non-wrapping signed range is often a wrapping unsigned range and vice versa. For FP, this problem does not exist as the ranges are always signed. I think it's better to leave out wrapped sets, at least for now, as they do make reasoning about operations a good bit harder. |
dtcxzyw
force-pushed
the
constant-fp-range
branch
from
661dc28 to
e2bdba2
Compare
arsenm
approved these changes
Sep 15, 2024
nikic
reviewed
Sep 15, 2024
There was a problem hiding this comment.
We should have exhaustive testing infrastructure. On half, or one of the 8-bit types, if any of them is sufficiently IEEE compliant...
There was a problem hiding this comment.
I think Float8E4M3 covers everything
There was a problem hiding this comment.
isSupportedSemantics has been removed. I will add fcmp support with exhaustive testing infrastructure in follow-up patches.
There was a problem hiding this comment.
We should also have exhaustive tests for classify at least (the correctness of the implementation is not obvious to me) and possibly also intersect/union.
nikic
reviewed
Sep 15, 2024
nikic
reviewed
Sep 15, 2024
dtcxzyw
commented
Sep 16, 2024
|
|
||
| TEST_F(ConstantFPRangeTest, Enumerate) { | ||
| constexpr unsigned NNaNValues = (1 << 8) - 2 * ((1 << 3) - 1); | ||
| constexpr unsigned Expected = 4 * ((NNaNValues + 1) * NNaNValues / 2 + 1); |
There was a problem hiding this comment.
We have 117616 ranges here. It is impossible to exhaustively test binary operations.
There was a problem hiding this comment.
Hm yeah, I didn't think the numbers here through. Probably the best we can do instead is to have a list of interesting values (-inf, -largest_normal, -smallest_normal, -largest_subnormal, -smallest_subnormal, -0, +0, ...) plus a few normal numbers to use as boundary values. Not as good as full exhaustive coverage, but at least it makes sure that all the special cases are treated correctly.
There was a problem hiding this comment.
Should test with a bit in every mantissa position
arsenm
reviewed
Sep 18, 2024
nikic
reviewed
Sep 18, 2024
|
|
||
| TEST_F(ConstantFPRangeTest, Enumerate) { | ||
| constexpr unsigned NNaNValues = (1 << 8) - 2 * ((1 << 3) - 1); | ||
| constexpr unsigned Expected = 4 * ((NNaNValues + 1) * NNaNValues / 2 + 1); |
There was a problem hiding this comment.
Hm yeah, I didn't think the numbers here through. Probably the best we can do instead is to have a list of interesting values (-inf, -largest_normal, -smallest_normal, -largest_subnormal, -smallest_subnormal, -0, +0, ...) plus a few normal numbers to use as boundary values. Not as good as full exhaustive coverage, but at least it makes sure that all the special cases are treated correctly.
dtcxzyw
force-pushed
the
constant-fp-range
branch
from
d3a1fbd to
3237d77
Compare
|
@arsenm Is it ok to land this patch? |
arsenm
approved these changes
Sep 25, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/3735 Here is the relevant piece of the build log for the reference |
|
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/145/builds/2073 Here is the relevant piece of the build log for the reference |
Fixed by #109921. |
chapuni
reviewed
Sep 25, 2024
Comment on lines
+199
to
+204
| KnownFPClass ConstantFPRange::toKnownFPClass() const { | ||
| KnownFPClass Result; | ||
| Result.KnownFPClasses = classify(); | ||
| Result.SignBit = getSignBit(); | ||
| return Result; | ||
| } |
There was a problem hiding this comment.
Could you move this to elsewhere?
|
|
||
| #include "llvm/IR/ConstantFPRange.h" | ||
| #include "llvm/ADT/APFloat.h" | ||
| #include "llvm/Analysis/ValueTracking.h" |
There was a problem hiding this comment.
LLVMCore should not depend on llvm/Analysis.
dtcxzyw
added a commit
that referenced
this pull request
Sep 25, 2024
Addresses comment #86483 (review).
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/1233 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/64/builds/1086 Here is the relevant piece of the build log for the reference |
jayfoad
reviewed
Oct 1, 2024
Comment on lines
+121
to
+123
| /// 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}'. |
There was a problem hiding this comment.
The "Formally" part doesn't seem to make sense - did something go wrong with the single-quoted part? Isn't it just: Formally, this returns { x : fcmp op x Other is true}?
| /// Return true if this range is not equal to another range. | ||
| bool operator!=(const ConstantFPRange &CR) const { return !operator==(CR); } | ||
|
|
||
| /// Return the FPClassTest which will return true for the value. |
There was a problem hiding this comment.
This is a bit vague. Maybe "Return the smallest FPClassTest which includes all values in the range"?
| /// another range. | ||
| ConstantFPRange intersectWith(const ConstantFPRange &CR) const; | ||
|
|
||
| /// Return the range that results from the union of this range |
There was a problem hiding this comment.
Maybe "Return the smallest range ...", just to be clear?
| /// Return true if the other range is a subset of this one. | ||
| bool contains(const ConstantFPRange &CR) const; | ||
|
|
||
| /// If this set contains a single element, return it, otherwise return null. |
There was a problem hiding this comment.
Maybe "a single non-NaN element" would be clearer, here and in isSingleElement below?
Comment on lines
+90
to
+91
| this->MayBeQNaN = MayBeQNaN; | ||
| this->MayBeSNaN = MayBeSNaN; |
There was a problem hiding this comment.
Can you not initialize these in the member initializer list?
Comment on lines
+193
to
+195
| for (uint32_t I = LowerMask; I <= UpperMask; I <<= 1) | ||
| Mask |= I; | ||
| } |
There was a problem hiding this comment.
You can do this without a loop: Mask |= (UpperMask << 1) - LowerMask (with a comment)
dtcxzyw
added a commit
that referenced
this pull request
Oct 2, 2024
1. Address post-commit review comments #86483 (comment). 2. Move some NFC changes from #110082 to this patch.
puja2196
pushed a commit
to puja2196/LLVM-tutorial
that referenced
this pull request
Oct 2, 2024
1. Address post-commit review comments llvm/llvm-project#86483 (comment). 2. Move some NFC changes from llvm/llvm-project#110082 to this patch.
Sterling-Augustine
pushed a commit
to Sterling-Augustine/llvm-project
that referenced
this pull request
Oct 3, 2024
1. Address post-commit review comments llvm#86483 (comment). 2. Move some NFC changes from llvm#110082 to this patch.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related issues: #68301 #70985 #82381