Add ConstantRangeList::subtract(ConstantRange) #97093
Merged
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
|
@llvm/pr-subscribers-llvm-ir Author: Haopeng Liu (haopliu) ChangesAdd ConstantRangeList::subtract(ConstantRange). This API will be used in the "initializes" attribute inference as well (for load instructions). Full diff: https://github.com/llvm/llvm-project/pull/97093.diff 3 Files Affected:
diff --git a/llvm/include/llvm/IR/ConstantRangeList.h b/llvm/include/llvm/IR/ConstantRangeList.h
index 46edaff19e73f..9aae52dac130e 100644
--- a/llvm/include/llvm/IR/ConstantRangeList.h
+++ b/llvm/include/llvm/IR/ConstantRangeList.h
@@ -72,6 +72,12 @@ class [[nodiscard]] ConstantRangeList {
APInt(64, Upper, /*isSigned=*/true)));
}
+ void subtract(const ConstantRange &SubRange);
+ void subtract(int64_t Lower, int64_t Upper) {
+ subtract(ConstantRange(APInt(64, Lower, /*isSigned=*/true),
+ APInt(64, Upper, /*isSigned=*/true)));
+ }
+
/// Return the range list that results from the union of this
/// ConstantRangeList with another ConstantRangeList, "CRL".
ConstantRangeList unionWith(const ConstantRangeList &CRL) const;
diff --git a/llvm/lib/IR/ConstantRangeList.cpp b/llvm/lib/IR/ConstantRangeList.cpp
index 0373524a09f10..2db5de86b3c76 100644
--- a/llvm/lib/IR/ConstantRangeList.cpp
+++ b/llvm/lib/IR/ConstantRangeList.cpp
@@ -81,6 +81,65 @@ void ConstantRangeList::insert(const ConstantRange &NewRange) {
}
}
+void ConstantRangeList::subtract(const ConstantRange &SubRange) {
+ if (SubRange.isEmptySet())
+ return;
+ assert(!SubRange.isFullSet() && "Do not support full set");
+ assert(SubRange.getLower().slt(SubRange.getUpper()));
+ assert(getBitWidth() == SubRange.getBitWidth());
+ // Handle common cases.
+ if (empty() || Ranges.back().getUpper().sle(SubRange.getLower())) {
+ return;
+ }
+ if (SubRange.getUpper().sle(Ranges.front().getLower())) {
+ return;
+ }
+
+ SmallVector<ConstantRange, 2> Result;
+ auto AppendRange = [&Result](APInt Start, APInt End) {
+ if (Start.slt(End))
+ Result.push_back(ConstantRange(Start, End));
+ };
+ for (auto &Range : Ranges) {
+ if (SubRange.getUpper().sle(Range.getLower()) ||
+ Range.getUpper().sle(SubRange.getLower())) {
+ // "Range" and "SubRange" do not overlap.
+ // L---U : Range
+ // L---U : SubRange (Case1)
+ // L---U : SubRange (Case2)
+ Result.push_back(Range);
+ } else if (Range.getLower().sle(SubRange.getLower()) &&
+ SubRange.getUpper().sle(Range.getUpper())) {
+ // "Range" contains "SubRange".
+ // L---U : Range
+ // L-U : SubRange
+ // Note that ConstantRange::contains(ConstantRange) checks unsigned,
+ // but we need signed checking here.
+ AppendRange(Range.getLower(), SubRange.getLower());
+ AppendRange(SubRange.getUpper(), Range.getUpper());
+ } else if (SubRange.getLower().sle(Range.getLower()) &&
+ Range.getUpper().sle(SubRange.getUpper())) {
+ // "SubRange" contains "Range".
+ // L-U : Range
+ // L---U : SubRange
+ continue;
+ } else if (Range.getLower().sge(SubRange.getLower()) &&
+ Range.getLower().sle(SubRange.getUpper())) {
+ // "Range" and "SubRange" overlap at the left.
+ // L---U : Range
+ // L---U : SubRange
+ AppendRange(SubRange.getUpper(), Range.getUpper());
+ } else {
+ // "Range" and "SubRange" overlap at the right.
+ // L---U : Range
+ // L---U : SubRange
+ AppendRange(Range.getLower(), SubRange.getLower());
+ }
+ }
+
+ Ranges.assign(Result.begin(), Result.end());
+}
+
ConstantRangeList
ConstantRangeList::unionWith(const ConstantRangeList &CRL) const {
assert(getBitWidth() == CRL.getBitWidth() &&
diff --git a/llvm/unittests/IR/ConstantRangeListTest.cpp b/llvm/unittests/IR/ConstantRangeListTest.cpp
index b679dd3a33d5d..da3cb330871b3 100644
--- a/llvm/unittests/IR/ConstantRangeListTest.cpp
+++ b/llvm/unittests/IR/ConstantRangeListTest.cpp
@@ -101,6 +101,50 @@ ConstantRangeList GetCRL(ArrayRef<std::pair<APInt, APInt>> Pairs) {
return ConstantRangeList(Ranges);
}
+TEST_F(ConstantRangeListTest, Subtract) {
+ APInt AP0 = APInt(64, 0, /*isSigned=*/true);
+ APInt AP2 = APInt(64, 2, /*isSigned=*/true);
+ APInt AP3 = APInt(64, 3, /*isSigned=*/true);
+ APInt AP4 = APInt(64, 4, /*isSigned=*/true);
+ APInt AP8 = APInt(64, 8, /*isSigned=*/true);
+ APInt AP10 = APInt(64, 10, /*isSigned=*/true);
+ APInt AP11 = APInt(64, 11, /*isSigned=*/true);
+ APInt AP12 = APInt(64, 12, /*isSigned=*/true);
+ ConstantRangeList CRL = GetCRL({{AP0, AP4}, {AP8, AP12}});
+
+ // Execute ConstantRangeList::subtract(ConstantRange) and check the result
+ // is expected. Pass "CRL" by value so that subtract() does not affect the
+ // argument in caller.
+ auto SubtractAndCheck = [](ConstantRangeList CRL,
+ const std::pair<int64_t, int64_t> &Range,
+ const ConstantRangeList &ExpectedCRL) {
+ CRL.subtract(Range.first, Range.second);
+ EXPECT_EQ(CRL, ExpectedCRL);
+ };
+
+ // No overlap
+ SubtractAndCheck(CRL, {-4, 0}, CRL);
+ SubtractAndCheck(CRL, {4, 8}, CRL);
+ SubtractAndCheck(CRL, {12, 16}, CRL);
+
+ // Overlap (left or right)
+ SubtractAndCheck(CRL, {-4, 2}, GetCRL({{AP2, AP4}, {AP8, AP12}}));
+ SubtractAndCheck(CRL, {-4, 4}, GetCRL({{AP8, AP12}}));
+ SubtractAndCheck(CRL, {-4, 8}, GetCRL({{AP8, AP12}}));
+ SubtractAndCheck(CRL, {10, 16}, GetCRL({{AP0, AP4}, {AP8, AP10}}));
+ SubtractAndCheck(CRL, {8, 16}, GetCRL({{AP0, AP4}}));
+ SubtractAndCheck(CRL, {6, 16}, GetCRL({{AP0, AP4}}));
+
+ // Subset
+ SubtractAndCheck(CRL, {2, 3}, GetCRL({{AP0, AP2}, {AP3, AP4}, {AP8, AP12}}));
+ SubtractAndCheck(CRL, {10, 11},
+ GetCRL({{AP0, AP4}, {AP8, AP10}, {AP11, AP12}}));
+
+ // Superset
+ SubtractAndCheck(CRL, {0, 12}, GetCRL({}));
+ SubtractAndCheck(CRL, {-4, 16}, GetCRL({}));
+}
+
TEST_F(ConstantRangeListTest, Union) {
APInt APN4 = APInt(64, -4, /*isSigned=*/true);
APInt APN2 = APInt(64, -2, /*isSigned=*/true);
|
jvoung
reviewed
Jun 28, 2024
| @@ -72,6 +72,12 @@ class [[nodiscard]] ConstantRangeList { | |||
| APInt(64, Upper, /*isSigned=*/true))); | |||
| } | |||
|
|
|||
| void subtract(const ConstantRange &SubRange); | |||
| void subtract(int64_t Lower, int64_t Upper) { | |||
There was a problem hiding this comment.
Is this mostly to simplify the test (line 121 in the test.cpp?)?
It might be better to put this logic in the test file instead (could be inlined into the line 121 area?), so that there is (a) smaller public API (b) less int64_t specific APIs (while the rest seem to be more generic regarding bitwidth)?
There was a problem hiding this comment.
Make sense. Deleted the API.
|
|
||
| SmallVector<ConstantRange, 2> Result; | ||
| auto AppendRange = [&Result](APInt Start, APInt End) { | ||
| if (Start.slt(End)) |
There was a problem hiding this comment.
might be helpful to have brief comment "append new range if not empty" (was initially less clear why the slt check)
There was a problem hiding this comment.
I'd rename this to AppendRangeIfNonEmpty or something like that
| for (auto &Range : Ranges) { | ||
| if (SubRange.getUpper().sle(Range.getLower()) || | ||
| Range.getUpper().sle(SubRange.getLower())) { | ||
| // "Range" and "SubRange" do not overlap. |
| SubtractAndCheck(CRL, {-4, 8}, GetCRL({{AP8, AP12}})); | ||
| SubtractAndCheck(CRL, {10, 16}, GetCRL({{AP0, AP4}, {AP8, AP10}})); | ||
| SubtractAndCheck(CRL, {8, 16}, GetCRL({{AP0, AP4}})); | ||
| SubtractAndCheck(CRL, {6, 16}, GetCRL({{AP0, AP4}})); |
There was a problem hiding this comment.
maybe try touching at the other ends like {0, X} and {X, 12} too
(vs {X, 4} and {8, X})?
There was a problem hiding this comment.
Done! Added another case {2, 10} that overlaps multiple ranges.
aeubanks
reviewed
Jun 28, 2024
llvm/lib/IR/ConstantRangeList.cpp
Outdated
| // Handle common cases. | ||
| if (empty() || Ranges.back().getUpper().sle(SubRange.getLower())) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
llvm/lib/IR/ConstantRangeList.cpp
Outdated
| if (empty() || Ranges.back().getUpper().sle(SubRange.getLower())) { | ||
| return; | ||
| } | ||
| if (SubRange.getUpper().sle(Ranges.front().getLower())) { |
There was a problem hiding this comment.
seems weird to combine the empty plus first range check into one if statement but keep the second range check separate
There was a problem hiding this comment.
Ah, nice catch. Combined the CRL empty check and the "SubRange" empty check together.
llvm/lib/IR/ConstantRangeList.cpp
Outdated
| } | ||
| } | ||
|
|
||
| Ranges.assign(Result.begin(), Result.end()); |
There was a problem hiding this comment.
is Ranges = Result; simpler?
|
|
||
| SmallVector<ConstantRange, 2> Result; | ||
| auto AppendRange = [&Result](APInt Start, APInt End) { | ||
| if (Start.slt(End)) |
There was a problem hiding this comment.
I'd rename this to AppendRangeIfNonEmpty or something like that
aeubanks
approved these changes
Jun 28, 2024
| // L---U : SubRange | ||
| AppendRangeIfNonEmpty(SubRange.getUpper(), Range.getUpper()); | ||
| } else { | ||
| // "Range" and "SubRange" overlap at the right. |
jvoung
approved these changes
Jun 28, 2024
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/961 Here is the relevant piece of the build log for the reference: |
lravenclaw
pushed a commit
to lravenclaw/llvm-project
that referenced
this pull request
Jul 3, 2024
Add ConstantRangeList::subtract(ConstantRange). This API will be used in the "initializes" attribute inference as well (for load instructions).
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.
Add ConstantRangeList::subtract(ConstantRange).
This API will be used in the "initializes" attribute inference as well (for load instructions).