Skip to content

Commit

Permalink
[ValueLattice] Steal bits from Tag to track range extensions (NFC).
Browse files Browse the repository at this point in the history
Users of ValueLatticeElement currently have to ensure constant ranges
are not extended indefinitely. For example, in SCCP, mergeIn goes to
overdefined if a constantrange value is repeatedly merged with larger
constantranges. This is a simple form of widening.

In some cases, this leads to an unnecessary loss of information and
things can be improved by allowing a small number of extensions in the
hope that a fixed point is reached after a small number of steps.

To make better decisions about widening, it is helpful to keep track of
the number of range extensions. That state is tied directly to a
concrete ValueLatticeElement and some unused bits in the class can be
used. The current patch preserves the existing behavior by default:
CheckWiden defaults to false and if CheckWiden is true, a single change
to the range is allowed.

Follow-up patches will slightly increase the threshold for widening.

Reviewers: efriedma, davide, mssimpso

Reviewed By: efriedma

Differential Revision: https://reviews.llvm.org/D78145
  • Loading branch information
fhahn committed Apr 17, 2020
1 parent a8e4b7a commit c245d3e
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 15 deletions.
24 changes: 18 additions & 6 deletions llvm/include/llvm/Analysis/ValueLattice.h
Expand Up @@ -75,7 +75,9 @@ class ValueLatticeElement {
overdefined,
};

ValueLatticeElementTy Tag;
ValueLatticeElementTy Tag : 6;
/// Number of times a constant range has been extended with widening enabled.
unsigned NumRangeExtensions : 8;

/// The union either stores a pointer to a constant or a constant range,
/// associated to the lattice element. We have to ensure that Range is
Expand Down Expand Up @@ -133,6 +135,7 @@ class ValueLatticeElement {
new (&Range) ConstantRange(Other.Range);
else
Range = Other.Range;
NumRangeExtensions = Other.NumRangeExtensions;
break;
case constant:
case notconstant:
Expand Down Expand Up @@ -287,7 +290,8 @@ class ValueLatticeElement {
/// range or the object must be undef. The tag is set to
/// constant_range_including_undef if either the existing value or the new
/// range may include undef.
bool markConstantRange(ConstantRange NewR, bool MayIncludeUndef = false) {
bool markConstantRange(ConstantRange NewR, bool MayIncludeUndef = false,
bool CheckWiden = false) {
if (NewR.isFullSet())
return markOverdefined();

Expand All @@ -304,6 +308,11 @@ class ValueLatticeElement {
if (getConstantRange() == NewR)
return Tag != OldTag;

// Simple form of widening. If a range is extended multiple times, go to
// overdefined.
if (CheckWiden && ++NumRangeExtensions == 1)
return markOverdefined();

assert(NewR.contains(getConstantRange()) &&
"Existing range must be a subset of NewR");
Range = std::move(NewR);
Expand All @@ -314,14 +323,15 @@ class ValueLatticeElement {
if (NewR.isEmptySet())
return markOverdefined();

NumRangeExtensions = 0;
Tag = NewTag;
new (&Range) ConstantRange(std::move(NewR));
return true;
}

/// Updates this object to approximate both this object and RHS. Returns
/// true if this object has been changed.
bool mergeIn(const ValueLatticeElement &RHS) {
bool mergeIn(const ValueLatticeElement &RHS, bool CheckWiden = false) {
if (RHS.isUnknown() || isOverdefined())
return false;
if (RHS.isOverdefined()) {
Expand All @@ -337,7 +347,7 @@ class ValueLatticeElement {
return markConstant(RHS.getConstant(), /*MayIncludeUndef=*/true);
if (RHS.isConstantRange())
return markConstantRange(RHS.getConstantRange(true),
/*MayIncludeUndef=*/true);
/*MayIncludeUndef=*/true, CheckWiden);
return markOverdefined();
}

Expand Down Expand Up @@ -380,7 +390,7 @@ class ValueLatticeElement {
ConstantRange NewR = getConstantRange().unionWith(RHS.getConstantRange());
return markConstantRange(
std::move(NewR),
/*MayIncludeUndef=*/RHS.isConstantRangeIncludingUndef());
/*MayIncludeUndef=*/RHS.isConstantRangeIncludingUndef(), CheckWiden);
}

// Compares this symbolic value with Other using Pred and returns either
Expand Down Expand Up @@ -412,7 +422,9 @@ class ValueLatticeElement {
}
};

raw_ostream &operator<<(raw_ostream &OS, const ValueLatticeElement &Val);
static_assert(sizeof(ValueLatticeElement) <= 40,
"size of ValueLatticeElement changed unexpectedly");

raw_ostream &operator<<(raw_ostream &OS, const ValueLatticeElement &Val);
} // end namespace llvm
#endif
10 changes: 1 addition & 9 deletions llvm/lib/Transforms/Scalar/SCCP.cpp
Expand Up @@ -401,15 +401,7 @@ class SCCPSolver : public InstVisitor<SCCPSolver> {

bool mergeInValue(ValueLatticeElement &IV, Value *V,
ValueLatticeElement MergeWithV, bool Widen = true) {
// Do a simple form of widening, to avoid extending a range repeatedly in a
// loop. If IV is a constant range, it means we already set it once. If
// MergeWithV would extend IV, mark V as overdefined.
if (Widen && IV.isConstantRange() && MergeWithV.isConstantRange() &&
!IV.getConstantRange().contains(MergeWithV.getConstantRange())) {
markOverdefined(IV, V);
return true;
}
if (IV.mergeIn(MergeWithV)) {
if (IV.mergeIn(MergeWithV, Widen)) {
pushToWorkList(IV, V);
LLVM_DEBUG(dbgs() << "Merged " << MergeWithV << " into " << *V << " : "
<< IV << "\n");
Expand Down

0 comments on commit c245d3e

Please sign in to comment.