Skip to content

Commit

Permalink
[ValueLattice] Update markConstantRange to return false equal ranges.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
fhahn committed Feb 15, 2020
1 parent e5b3ae4 commit c1943b4
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 5 deletions.
17 changes: 12 additions & 5 deletions llvm/include/llvm/Analysis/ValueLattice.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,14 +220,21 @@ class ValueLatticeElement {
return true;
}

/// Mark the object as constant range with \p NewR. If the object is already a
/// constant range, nothing changes if the existing range is equal to \p
/// NewR. Otherwise \p NewR must be a superset of the existing range or the
/// object must be undefined.
bool markConstantRange(ConstantRange NewR) {
if (isConstantRange()) {
if (getConstantRange() == NewR)
return false;

if (NewR.isEmptySet())
markOverdefined();
else {
assert(NewR.contains(getConstantRange()) && "Existing range must be a subset of NewR");
Range = std::move(NewR);
}
return markOverdefined();

assert(NewR.contains(getConstantRange()) &&
"Existing range must be a subset of NewR");
Range = std::move(NewR);
return true;
}

Expand Down
17 changes: 17 additions & 0 deletions llvm/unittests/Analysis/ValueLatticeTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,23 @@ TEST_F(ValueLatticeTest, ValueLatticeGetters) {
EXPECT_TRUE(ValueLatticeElement::getNot(C2).isNotConstant());
}

TEST_F(ValueLatticeTest, MarkConstantRange) {
auto LV1 =
ValueLatticeElement::getRange({APInt(32, 10, true), APInt(32, 20, true)});

// Test markConstantRange() with an equal range.
EXPECT_FALSE(
LV1.markConstantRange({APInt(32, 10, true), APInt(32, 20, true)}));

// Test markConstantRange() with supersets of existing range.
EXPECT_TRUE(LV1.markConstantRange({APInt(32, 5, true), APInt(32, 20, true)}));
EXPECT_EQ(LV1.getConstantRange().getLower().getLimitedValue(), 5U);
EXPECT_EQ(LV1.getConstantRange().getUpper().getLimitedValue(), 20U);
EXPECT_TRUE(LV1.markConstantRange({APInt(32, 5, true), APInt(32, 23, true)}));
EXPECT_EQ(LV1.getConstantRange().getLower().getLimitedValue(), 5U);
EXPECT_EQ(LV1.getConstantRange().getUpper().getLimitedValue(), 23U);
}

TEST_F(ValueLatticeTest, MergeIn) {
auto I32Ty = IntegerType::get(Context, 32);
auto *C1 = ConstantInt::get(I32Ty, 1);
Expand Down

0 comments on commit c1943b4

Please sign in to comment.