-
Notifications
You must be signed in to change notification settings - Fork 873
ConstraintAnalysis: Increment constants #8969
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
4ff74fa
9b38ac6
a319bc1
9cd4af1
a9a9405
9d8fe3a
eb4fc78
b368c1c
fa9468f
4ff9da5
c79ac07
7516199
d1b8260
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -506,15 +506,110 @@ void LocalConstraint::flip() { | |
| } | ||
|
|
||
| void BasicBlockConstraintMap::set(Index index, const Constraint& c) { | ||
| set(index, AndedConstraintSet{c}); | ||
| } | ||
|
|
||
| void BasicBlockConstraintMap::set(Index index, | ||
| 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)))) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎉 nice use of |
||
| // The local y must have old constraints that we know how to increment. | ||
| auto old = get(y); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to be making a copy here?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| 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) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
@@ -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(); | ||
|
|
@@ -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) { | ||
|
|
@@ -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); | ||
| } | ||
| } | ||
|
|
@@ -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); | ||
|
|
@@ -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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are seeing the future, for that is in the next PR 😄
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
|
|
@@ -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); | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
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
setin terms of this one.There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.