Skip to content

Commit

Permalink
[ValueLattice] Add struct for merge options.
Browse files Browse the repository at this point in the history
This makes it easier to extend the merge options in the future and also
reduces the risk of accidentally setting a wrong option.

Reviewers: efriedma, nikic, reames, davide

Reviewed By: efriedma

Differential Revision: https://reviews.llvm.org/D78368
  • Loading branch information
fhahn committed Apr 19, 2020
1 parent 9412e4c commit 6ba0695
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 15 deletions.
49 changes: 39 additions & 10 deletions llvm/include/llvm/Analysis/ValueLattice.h
Expand Up @@ -88,6 +88,31 @@ class ValueLatticeElement {
};

public:
/// Struct to control some aspects related to merging constant ranges.
struct MergeOptions {
/// The merge value may include undef.
bool MayIncludeUndef;

/// Handle repeatedly extending a range by going to overdefined after a
/// number of steps.
bool CheckWiden;

MergeOptions() : MergeOptions(false, false) {}

MergeOptions(bool MayIncludeUndef, bool CheckWiden)
: MayIncludeUndef(MayIncludeUndef), CheckWiden(CheckWiden) {}

MergeOptions &setMayIncludeUndef(bool V = true) {
MayIncludeUndef = V;
return *this;
}

MergeOptions &setCheckWiden(bool V = true) {
CheckWiden = V;
return *this;
}
};

// Const and Range are initialized on-demand.
ValueLatticeElement() : Tag(unknown) {}

Expand Down Expand Up @@ -164,7 +189,8 @@ class ValueLatticeElement {
return getOverdefined();

ValueLatticeElement Res;
Res.markConstantRange(std::move(CR), MayIncludeUndef);
Res.markConstantRange(std::move(CR),
MergeOptions().setMayIncludeUndef(MayIncludeUndef));
return Res;
}
static ValueLatticeElement getOverdefined() {
Expand Down Expand Up @@ -248,7 +274,9 @@ class ValueLatticeElement {
}

if (ConstantInt *CI = dyn_cast<ConstantInt>(V))
return markConstantRange(ConstantRange(CI->getValue()), MayIncludeUndef);
return markConstantRange(
ConstantRange(CI->getValue()),
MergeOptions().setMayIncludeUndef(MayIncludeUndef));

assert(isUnknown() || isUndef());
Tag = constant;
Expand Down Expand Up @@ -282,14 +310,14 @@ 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 CheckWiden = false) {
bool markConstantRange(ConstantRange NewR,
MergeOptions Opts = MergeOptions()) {
if (NewR.isFullSet())
return markOverdefined();

ValueLatticeElementTy OldTag = Tag;
ValueLatticeElementTy NewTag =
(isUndef() || isConstantRangeIncludingUndef() || MayIncludeUndef)
(isUndef() || isConstantRangeIncludingUndef() || Opts.MayIncludeUndef)
? constantrange_including_undef
: constantrange;
if (isConstantRange()) {
Expand All @@ -302,7 +330,7 @@ class ValueLatticeElement {

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

assert(NewR.contains(getConstantRange()) &&
Expand All @@ -323,7 +351,8 @@ class ValueLatticeElement {

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

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

// Compares this symbolic value with Other using Pred and returns either
Expand Down
16 changes: 11 additions & 5 deletions llvm/lib/Transforms/Scalar/SCCP.cpp
Expand Up @@ -399,9 +399,13 @@ class SCCPSolver : public InstVisitor<SCCPSolver> {
return true;
}

/// Merge \p MergeWithV into \p IV and push \p V to the worklist, if \p IV
/// changes.
bool mergeInValue(ValueLatticeElement &IV, Value *V,
ValueLatticeElement MergeWithV, bool Widen = true) {
if (IV.mergeIn(MergeWithV, Widen)) {
ValueLatticeElement MergeWithV,
ValueLatticeElement::MergeOptions Opts = {
/*MayIncludeUndef=*/false, /*CheckWiden=*/true}) {
if (IV.mergeIn(MergeWithV, Opts)) {
pushToWorkList(IV, V);
LLVM_DEBUG(dbgs() << "Merged " << MergeWithV << " into " << *V << " : "
<< IV << "\n");
Expand All @@ -411,10 +415,11 @@ class SCCPSolver : public InstVisitor<SCCPSolver> {
}

bool mergeInValue(Value *V, ValueLatticeElement MergeWithV,
bool Widen = true) {
ValueLatticeElement::MergeOptions Opts = {
/*MayIncludeUndef=*/false, /*CheckWiden=*/true}) {
assert(!V->getType()->isStructTy() &&
"non-structs should use markConstant");
return mergeInValue(ValueState[V], V, MergeWithV, Widen);
return mergeInValue(ValueState[V], V, MergeWithV, Opts);
}

/// getValueState - Return the ValueLatticeElement object that corresponds to
Expand Down Expand Up @@ -1203,7 +1208,8 @@ void SCCPSolver::handleCallArguments(CallSite CS) {
mergeInValue(getStructValueState(&*AI, i), &*AI, CallArg);
}
} else
mergeInValue(&*AI, getValueState(*CAI), false);
mergeInValue(&*AI, getValueState(*CAI),
ValueLatticeElement::MergeOptions().setCheckWiden(false));
}
}
}
Expand Down

0 comments on commit 6ba0695

Please sign in to comment.