Skip to content

Commit c1943b4

Browse files
committed
[ValueLattice] Update markConstantRange to return false equal ranges.
Currently we always return true, when markConstantRange is used on an object already containing a constant range. If NewR is equal to the existing constant range however, nothing changes and we should return false. I also went ahead and added a clarifying comment and improved the assertion. Reviewers: efriedma, davide, nikic Reviewed By: efriedma Differential Revision: https://reviews.llvm.org/D73240
1 parent e5b3ae4 commit c1943b4

File tree

2 files changed

+29
-5
lines changed

2 files changed

+29
-5
lines changed

llvm/include/llvm/Analysis/ValueLattice.h

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -220,14 +220,21 @@ class ValueLatticeElement {
220220
return true;
221221
}
222222

223+
/// Mark the object as constant range with \p NewR. If the object is already a
224+
/// constant range, nothing changes if the existing range is equal to \p
225+
/// NewR. Otherwise \p NewR must be a superset of the existing range or the
226+
/// object must be undefined.
223227
bool markConstantRange(ConstantRange NewR) {
224228
if (isConstantRange()) {
229+
if (getConstantRange() == NewR)
230+
return false;
231+
225232
if (NewR.isEmptySet())
226-
markOverdefined();
227-
else {
228-
assert(NewR.contains(getConstantRange()) && "Existing range must be a subset of NewR");
229-
Range = std::move(NewR);
230-
}
233+
return markOverdefined();
234+
235+
assert(NewR.contains(getConstantRange()) &&
236+
"Existing range must be a subset of NewR");
237+
Range = std::move(NewR);
231238
return true;
232239
}
233240

llvm/unittests/Analysis/ValueLatticeTest.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,23 @@ TEST_F(ValueLatticeTest, ValueLatticeGetters) {
4343
EXPECT_TRUE(ValueLatticeElement::getNot(C2).isNotConstant());
4444
}
4545

46+
TEST_F(ValueLatticeTest, MarkConstantRange) {
47+
auto LV1 =
48+
ValueLatticeElement::getRange({APInt(32, 10, true), APInt(32, 20, true)});
49+
50+
// Test markConstantRange() with an equal range.
51+
EXPECT_FALSE(
52+
LV1.markConstantRange({APInt(32, 10, true), APInt(32, 20, true)}));
53+
54+
// Test markConstantRange() with supersets of existing range.
55+
EXPECT_TRUE(LV1.markConstantRange({APInt(32, 5, true), APInt(32, 20, true)}));
56+
EXPECT_EQ(LV1.getConstantRange().getLower().getLimitedValue(), 5U);
57+
EXPECT_EQ(LV1.getConstantRange().getUpper().getLimitedValue(), 20U);
58+
EXPECT_TRUE(LV1.markConstantRange({APInt(32, 5, true), APInt(32, 23, true)}));
59+
EXPECT_EQ(LV1.getConstantRange().getLower().getLimitedValue(), 5U);
60+
EXPECT_EQ(LV1.getConstantRange().getUpper().getLimitedValue(), 23U);
61+
}
62+
4663
TEST_F(ValueLatticeTest, MergeIn) {
4764
auto I32Ty = IntegerType::get(Context, 32);
4865
auto *C1 = ConstantInt::get(I32Ty, 1);

0 commit comments

Comments
 (0)