Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 97 additions & 2 deletions src/ir/constraint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -506,15 +506,110 @@ void LocalConstraint::flip() {
}

void BasicBlockConstraintMap::set(Index index, const Constraint& c) {
set(index, AndedConstraintSet{c});
}

void BasicBlockConstraintMap::set(Index index,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can redefine the existing set in terms of this one.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be less efficient, though. We can apply a single constraint without a loop, and without checking if the set is empty.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though I guess inlining might make it fast. I'll simplify and then see if it shows up in profiles later.

const AndedConstraintSet& constraints) {
// We should not set values in unreachable code.
assert(!unreachable);

// Clear the old state.
eraseStaleRefs(index);
map.erase(index);

// Apply the constraint.
approximateAnd(index, c);
// Apply the constraints, if there are any.
if (constraints.provesNothing()) {
setProvesNothing(index);
} else {
for (auto& c : constraints) {
approximateAnd(index, c);
}
}
}

void BasicBlockConstraintMap::set(Index index, Expression* value) {
using namespace Match;
using namespace Abstract;

// Apply a constraint to a value, x = C.
if (Properties::isSingleConstantExpression(value)) {
auto c = Properties::getLiteral(value);
set(index, Constraint{Abstract::Eq, {c}});
return;
}

// Apply a constraint to a local, x = y.
if (auto* get = value->dynCast<LocalGet>()) {
set(index, Constraint{Abstract::Eq, {get->index}});
return;
}

// Apply an increment of a local, x = y + 1.
Index y;
if (matches(value, binary(Abstract::Add, local(&y), ival(1)))) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 nice use of matches!

// The local y must have old constraints that we know how to increment.
auto old = get(y);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to be making a copy here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, see how we modify the copy in-place, below.


// Iterate over the old constraints and increment each one.
auto success = true;
for (auto& c : old) {
auto* N = std::get_if<Literal>(&c.term);
if (!N) {
// A non-constant term, which we don't know how to increment.
success = false;
break;
}

switch (c.op) {
// x == N, x++ => x == N+1.
case Eq:
*N = N->add(Literal::makeFromInt32(1, N->type));
continue;
// x >= N, x++ => x > N

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could simplify this by always just updating the constant (by adding one, although it looks like this would be pretty easy to generalize) and leaving the operator alone.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, true. However, that would require checking for overflows in more places, and also make things more complicated later when we have non-constants (when x < y, x++, we can infer x <= y without incrementing anything, and in fact can't increment).

case GeS:
c.op = GtS;
continue;
case GeU:
c.op = GtU;
continue;
// x < N, x++ => x <= N
case LtS:
c.op = LeS;
continue;
case LtU:
c.op = LeU;
continue;
// x <= N, x++ => x <= N+1 if no overflow
case LeS:
if (N->isSignedMax()) {
success = false;
break;
}
*N = N->add(Literal::makeFromInt32(1, N->type));
continue;
case LeU:
if (N->isUnsignedMax()) {
success = false;
break;
}
*N = N->add(Literal::makeFromInt32(1, N->type));
continue;
default:
// Something we don't recognize.
success = false;
break;
}
}

if (success) {
set(index, old);
return;
}
}

// We know and can prove nothing.
setProvesNothing(index);
}

void BasicBlockConstraintMap::setProvesNothing(Index index) {
Expand Down
8 changes: 7 additions & 1 deletion src/ir/constraint.h
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,15 @@ struct BasicBlockConstraintMap {
assert(map.empty());
}

// Apply a constraint to a local.
// Apply a constraint to a local, replacing anything before.
void set(Index index, const Constraint& c);

// Apply a set of constraints to a local, replacing anything before.
void set(Index index, const AndedConstraintSet& constraints);

// Set the value in an expression to a local, replacing anything before.
void set(Index index, Expression* value);

// Mark a local as unknown and able to prove nothing.
void setProvesNothing(Index index);

Expand Down
19 changes: 19 additions & 0 deletions src/ir/match.h
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,18 @@ SelectMatcher(Select** binder, S1&& s1, S2&& s2, S3&& s3) {
return Matcher<Select*, S1, S2, S3>(binder, {}, s1, s2, s3);
}

// LocalGet
template<> struct NumComponents<LocalGet*> {
static constexpr size_t value = 1;
};
template<> struct GetComponent<LocalGet*, 0> {
Index operator()(LocalGet* curr) { return curr->index; }
};
template<class S>
inline decltype(auto) LocalGetMatcher(LocalGet** binder, S&& s) {
return Matcher<LocalGet*, S>(binder, {}, s);
}

} // namespace Internal

// Public matching API
Expand Down Expand Up @@ -878,6 +890,13 @@ inline decltype(auto) select(Select** binder, S1&& s1, S2&& s2, S3&& s3) {
return Internal::SelectMatcher(binder, s1, s2, s3);
}

inline decltype(auto) local() {
return Internal::LocalGetMatcher(nullptr, Internal::Any<Index>(nullptr));
}
inline decltype(auto) local(Index* binder) {
return Internal::LocalGetMatcher(nullptr, Internal::Any(binder));
}

} // namespace wasm::Match

#endif // wasm_ir_match_h
69 changes: 59 additions & 10 deletions src/passes/ConstraintAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@
#include "wasm-builder.h"
#include "wasm.h"

#define CONSTRAINT_DEBUG 0

#ifndef CONSTRAINT_DEBUG
#define CONSTRAINT_DEBUG 0
#endif

namespace wasm {

using namespace wasm::constraint;
Expand Down Expand Up @@ -220,6 +226,10 @@ struct ConstraintAnalysis
// Flow infos around until we have inferred all we can about the constraints
// in each location.
void flow() {
#if CONSTRAINT_DEBUG
dumpCFG("flow");
#endif

// Start from the entry as the only reachable block. That block has incoming
// values - defaults - for each var.
entry->contents.startConstraints.setReachable();
Expand Down Expand Up @@ -247,15 +257,25 @@ struct ConstraintAnalysis
// Starting from the entry, keep going while we find something new.
UniqueDeferredQueue<BasicBlock*> work;
work.push(entry);

while (!work.empty()) {
auto* block = work.pop();

// Start at the top of the block, then go through, applying things.
BasicBlockConstraintMap constraints = block->contents.startConstraints;

#if CONSTRAINT_DEBUG
std::cout << block << " start constraints: " << constraints << '\n';
#endif

for (auto** currp : block->contents.actions) {
applyToConstraints(*currp, constraints);
}

#if CONSTRAINT_DEBUG
std::cout << block << " end constraints: " << constraints << '\n';
#endif

// We now know the values at the end of the block. Flow it onward, and
// where it causes changes, queue more work.
for (auto* out : block->out) {
Expand All @@ -267,14 +287,27 @@ struct ConstraintAnalysis
branch && checkRelevancy(*branch)) {
auto sentConstraints = constraints;
sentConstraints.approximateAnd(branch->local, branch->constraint);
#if CONSTRAINT_DEBUG
std::cout << block << " sending branch to " << out
<< " with sent constraints: " << sentConstraints << '\n';
#endif
// If anything changed at the start of the target block, flow onwards.
if (outStartConstraints.approximateOr(sentConstraints)) {
#if CONSTRAINT_DEBUG
std::cout << "out's start after " << outStartConstraints << '\n';
std::cout << block << " branch-modified " << out
<< " to start with: " << outStartConstraints << '\n';
#endif
work.push(out);
}
} else {
// There are no specific branch constraints, so send the unmodified
// |constraints|, avoiding a copy.
if (outStartConstraints.approximateOr(constraints)) {
#if CONSTRAINT_DEBUG
std::cout << block << " modified " << out
<< " to start with: " << outStartConstraints << '\n';
#endif
work.push(out);
}
}
Expand All @@ -293,6 +326,9 @@ struct ConstraintAnalysis
// of course not needed at this stage.)
auto& constraints = block->contents.startConstraints;
for (auto** currp : block->contents.actions) {
#if CONSTRAINT_DEBUG
std::cout << block << " trying to optimize " << **currp << '\n';
#endif
if (!constraints.unreachable) {
applyToConstraints(*currp, constraints);
optimizeExpression(currp, constraints);
Expand Down Expand Up @@ -423,6 +459,21 @@ struct ConstraintAnalysis
return parsed;
}

// When applying constraints for a binary operation like x = y + 1, we may
// end up with lots of nonlinear work, in a loop: x may go from 0 to 1, then
// branch back to the top and merge, making it in the range [0, 1], then get
// incremented and loop again, leading to [0, 2] and so forth, only stopping
// when it reaches the loop bound, which may be very high. We don't want to
// spend significant time on such constant operations, as other passes will
// propagate them anyhow, so we verify that we don't apply such x = y + 1
// operations too many times.
Comment on lines +466 to +469

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was expecting to see a new "widening" mechanism to prevent unbounded iteration, but I don't see it. Am I missing something?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are seeing the future, for that is in the next PR 😄

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the infinite loop tests are in this PR. How do they not hang?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not have all the rules for combining their constants into ranges yet. I might add an internal Span class for that, as you suggested before. (However, that isn't needed for common loops, so it's not in the next PR.)

Anyhow, for now, this PR asserts on excessive work, so when we add stuff later, we won't silently get very slow.

#ifndef NDEBUG
static const Index MaxBinaryActions = 5;

// How many times we processed each Binary action.
std::unordered_map<Binary*, Index> binaryActionCounts;
#endif

// Given an expression, apply it to the constraints. For example, a local.set
// sets the value for that local.
void applyToConstraints(Expression* curr,
Expand All @@ -432,17 +483,15 @@ struct ConstraintAnalysis
// No point to apply a constraint to an irrelevant local.
return;
}
if (Properties::isSingleConstantExpression(set->value)) {
// Apply a constraint to this value.
auto value = Properties::getLiteral(set->value);
constraints.set(set->index, Constraint{Abstract::Eq, {value}});
} else if (auto* get = set->value->dynCast<LocalGet>()) {
// Apply a constraint to this local.
constraints.set(set->index, Constraint{Abstract::Eq, {get->index}});
} else {
// We know and can prove nothing.
constraints.setProvesNothing(set->index);

#ifndef NDEBUG
// See above on binary action counting limits.
if (auto* binary = set->value->dynCast<Binary>()) {
assert(binaryActionCounts[binary]++ <= MaxBinaryActions);
}
#endif

constraints.set(set->index, set->value);
}
}

Expand Down
Loading
Loading