Skip to content

Refactor expression runner so it can be used via the C and JS APIs #2702

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

Merged
merged 30 commits into from
Apr 20, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
132cb1d
Derive standalone expression runner from precompute pass
dcodeIO Mar 18, 2020
bbe1dc5
handle traps, format
dcodeIO Mar 18, 2020
3f3c02d
fix?
dcodeIO Mar 18, 2020
461576d
update tests
dcodeIO Mar 18, 2020
f5e3837
refactor replaceExpresion to an enum
dcodeIO Mar 19, 2020
2ccc81d
fix expressions[0] in tracing, track temporary local values
dcodeIO Mar 19, 2020
30bd10c
simplify
dcodeIO Mar 20, 2020
ca3ed47
implement preset local/global values
dcodeIO Mar 20, 2020
a853db9
could need some format on save
dcodeIO Mar 20, 2020
66d23f1
address comments
dcodeIO Mar 20, 2020
35d09c2
more documentation for getValues
dcodeIO Mar 20, 2020
f57cbdc
refactor runner to its own cpp file
dcodeIO Mar 21, 2020
63d7520
traverse into simple functions
dcodeIO Mar 21, 2020
5432156
deal with non-determinism
dcodeIO Mar 21, 2020
ec0a93d
update API, add test
dcodeIO Mar 21, 2020
1ceb5b5
retrigger CI
dcodeIO Mar 21, 2020
d145cfc
fix comment
dcodeIO Mar 23, 2020
153d10f
address review comments
dcodeIO Mar 24, 2020
7d51f30
address review comments
dcodeIO Mar 25, 2020
cabf038
Merge branch 'master' into expressionrunner
dcodeIO Apr 1, 2020
ed62e30
refactor
dcodeIO Apr 1, 2020
803cd22
reuse existing runner
dcodeIO Apr 2, 2020
597c5fa
revert trap on invalid
dcodeIO Apr 2, 2020
b4ca977
address comments
dcodeIO Apr 3, 2020
dcbab6a
address comments
dcodeIO Apr 3, 2020
4c8fc7c
address (most) comments
dcodeIO Apr 7, 2020
8586c57
mention interaction between TraverseCalls and PreserveSideEffects
dcodeIO Apr 7, 2020
1184f58
Merge branch 'master' into expressionrunner
dcodeIO Apr 15, 2020
17e5de0
address comments
dcodeIO Apr 16, 2020
84c27ac
Merge branch 'master' into expressionrunner
dcodeIO Apr 17, 2020
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
48 changes: 47 additions & 1 deletion src/binaryen-c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4812,6 +4812,51 @@ BinaryenExpressionRef RelooperRenderAndDispose(RelooperRef relooper,
return BinaryenExpressionRef(ret);
}

//
// ========= ExpressionRunner =========
//

ExpressionRunnerIntent ExpressionRunnerIntentEvaluate() {
return StandaloneExpressionRunner::Intent::EVALUATE;
}

ExpressionRunnerIntent ExpressionRunnerIntentReplaceExpression() {
return StandaloneExpressionRunner::Intent::REPLACE_EXPRESSION;
}

ExpressionRunnerRef ExpressionRunnerCreate(BinaryenModuleRef module,
ExpressionRunnerIntent intent,
BinaryenIndex maxDepth) {
if (tracing) {
std::cout << " the_runner = ExpressionRunnerCreate(the_module, " << intent
<< ", " << maxDepth << ");\n";
}
auto* wasm = (Module*)module;
GetValues getValues;
return ExpressionRunnerRef(new StandaloneExpressionRunner(
wasm, getValues, StandaloneExpressionRunner::Intent(intent), maxDepth));
}

BinaryenExpressionRef
ExpressionRunnerRunAndDispose(ExpressionRunnerRef runner,
Copy link
Member

Choose a reason for hiding this comment

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

If the runner fails, it seems like it would be useful to expose more information to the caller about why it failed. That way the user could choose to increase the depth or loop count, if applicable. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, good question. Seems like this might be a bit too much, considering how it complicates the API. For instance, on the AssemblyScript side I expect to always use a reasonable maxDepth (or none) and give up otherwise as there is no reason to make an exception using larger limits. Would have used that limit right away then.

BinaryenExpressionRef expr) {
if (tracing) {
std::cout << " ExpressionRunnerRunAndDispose(the_runner, expressions["
<< expressions[expr] << "]);\n";
}
auto* R = (StandaloneExpressionRunner*)runner;
Expression* ret = nullptr;
try {
auto flow = R->visit(expr);
if (!flow.breaking() && !flow.values.empty()) {
ret = flow.getConstExpression(*R->getModule());
}
} catch (StandaloneExpressionRunner::NonstandaloneException&) {
}
delete R;
return ret;
}

//
// ========= Other APIs =========
//
Expand All @@ -4832,7 +4877,8 @@ void BinaryenSetAPITracing(int on) {
" std::map<size_t, BinaryenExportRef> exports;\n"
" std::map<size_t, RelooperBlockRef> relooperBlocks;\n"
" BinaryenModuleRef the_module = NULL;\n"
" RelooperRef the_relooper = NULL;\n";
" RelooperRef the_relooper = NULL;\n"
" ExpressionRunnerRef the_runner = NULL;\n";
} else {
std::cout << " return 0;\n";
std::cout << "}\n";
Expand Down
32 changes: 32 additions & 0 deletions src/binaryen-c.h
Original file line number Diff line number Diff line change
Expand Up @@ -1618,6 +1618,38 @@ BINARYEN_API void RelooperAddBranchForSwitch(RelooperBlockRef from,
BINARYEN_API BinaryenExpressionRef RelooperRenderAndDispose(
RelooperRef relooper, RelooperBlockRef entry, BinaryenIndex labelHelper);

//
// ========= ExpressionRunner ==========
//

#ifdef __cplusplus
namespace wasm {
class StandaloneExpressionRunner;
} // namespace wasm
typedef class wasm::StandaloneExpressionRunner* ExpressionRunnerRef;
#else
typedef struct StandaloneExpressionRunner* ExpressionRunnerRef;
#endif

typedef uint32_t ExpressionRunnerIntent;

// Intent is to evaluate the expression, so we can ignore some side effects.
BINARYEN_API ExpressionRunnerIntent ExpressionRunnerIntentEvaluate();

// Intent is to replace the expression, so side effects must be retained.
BINARYEN_API ExpressionRunnerIntent ExpressionRunnerIntentReplaceExpression();

// Creates an ExpressionRunner instance
BINARYEN_API ExpressionRunnerRef
ExpressionRunnerCreate(BinaryenModuleRef module,
ExpressionRunnerIntent intent,
BinaryenIndex maxDepth);

// Runs the expression and returns the constant value expression it evaluates
// to, if any. Otherwise returns `NULL`. Also disposes the runner.
BINARYEN_API BinaryenExpressionRef ExpressionRunnerRunAndDispose(
ExpressionRunnerRef runner, BinaryenExpressionRef expr);

//
// ========= Other APIs =========
//
Expand Down
17 changes: 17 additions & 0 deletions src/js/binaryen.js-post.js
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,12 @@ function initializeConstants() {
].forEach(function(name) {
Module['SideEffects'][name] = Module['_BinaryenSideEffect' + name]();
});

// ExpressionRunner intents
Module['ExpressionRunner']['Intent'] = {
'Evaluate': Module['_ExpressionRunnerIntentEvaluate'](),
'ReplaceExpression': Module['_ExpressionRunnerIntentReplaceExpression']()
};
}

// 'Module' interface
Expand Down Expand Up @@ -2390,6 +2396,17 @@ Module['Relooper'] = function(module) {
};
};

// 'ExpressionRunner' interface
Module['ExpressionRunner'] = function(module, intent, maxDepth) {
if (typeof maxDepth === "undefined") maxDepth = 50; // default used by precompute
var runner = Module['_ExpressionRunnerCreate'](module['ptr'], intent, maxDepth);
this['ptr'] = runner;

this['runAndDispose'] = function(expr) {
return Module['_ExpressionRunnerRunAndDispose'](runner, expr);
};
};

function getAllNested(ref, numFn, getFn) {
var num = numFn(ref);
var ret = new Array(num);
Expand Down
118 changes: 13 additions & 105 deletions src/passes/Precompute.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,108 +39,12 @@

namespace wasm {

static const Name NOTPRECOMPUTABLE_FLOW("Binaryen|notprecomputable");

// Limit evaluation depth for 2 reasons: first, it is highly unlikely
// that we can do anything useful to precompute a hugely nested expression
// (we should succed at smaller parts of it first). Second, a low limit is
// helpful to avoid platform differences in native stack sizes.
static const Index MAX_DEPTH = 50;

typedef std::unordered_map<LocalGet*, Literals> GetValues;

// Precomputes an expression. Errors if we hit anything that can't be
// precomputed.
class PrecomputingExpressionRunner
: public ExpressionRunner<PrecomputingExpressionRunner> {
Module* module;

// map gets to constant values, if they are known to be constant
GetValues& getValues;

// Whether we are trying to precompute down to an expression (which we can do
// on say 5 + 6) or to a value (which we can't do on a local.tee that flows a
// 7 through it). When we want to replace the expression, we can only do so
// when it has no side effects. When we don't care about replacing the
// expression, we just want to know if it will contain a known constant.
bool replaceExpression;

public:
PrecomputingExpressionRunner(Module* module,
GetValues& getValues,
bool replaceExpression)
: ExpressionRunner<PrecomputingExpressionRunner>(MAX_DEPTH), module(module),
getValues(getValues), replaceExpression(replaceExpression) {}

struct NonstandaloneException {
}; // TODO: use a flow with a special name, as this is likely very slow

Flow visitLoop(Loop* curr) {
// loops might be infinite, so must be careful
// but we can't tell if non-infinite, since we don't have state, so loops
// are just impossible to optimize for now
return Flow(NOTPRECOMPUTABLE_FLOW);
}

Flow visitCall(Call* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitCallIndirect(CallIndirect* curr) {
return Flow(NOTPRECOMPUTABLE_FLOW);
}
Flow visitLocalGet(LocalGet* curr) {
auto iter = getValues.find(curr);
if (iter != getValues.end()) {
auto values = iter->second;
if (values.isConcrete()) {
return Flow(std::move(values));
}
}
return Flow(NOTPRECOMPUTABLE_FLOW);
}
Flow visitLocalSet(LocalSet* curr) {
// If we don't need to replace the whole expression, see if there
// is a value flowing through a tee.
if (!replaceExpression) {
if (curr->type.isConcrete()) {
assert(curr->isTee());
return visit(curr->value);
}
}
return Flow(NOTPRECOMPUTABLE_FLOW);
}
Flow visitGlobalGet(GlobalGet* curr) {
auto* global = module->getGlobal(curr->name);
if (!global->imported() && !global->mutable_) {
return visit(global->init);
}
return Flow(NOTPRECOMPUTABLE_FLOW);
}
Flow visitGlobalSet(GlobalSet* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitLoad(Load* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitStore(Store* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitAtomicRMW(AtomicRMW* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitAtomicCmpxchg(AtomicCmpxchg* curr) {
return Flow(NOTPRECOMPUTABLE_FLOW);
}
Flow visitAtomicWait(AtomicWait* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitAtomicNotify(AtomicNotify* curr) {
return Flow(NOTPRECOMPUTABLE_FLOW);
}
Flow visitSIMDLoad(SIMDLoad* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitMemoryInit(MemoryInit* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitDataDrop(DataDrop* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitMemoryCopy(MemoryCopy* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitMemoryFill(MemoryFill* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitHost(Host* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitTry(Try* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitThrow(Throw* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitRethrow(Rethrow* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitBrOnExn(BrOnExn* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitPush(Push* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }
Flow visitPop(Pop* curr) { return Flow(NOTPRECOMPUTABLE_FLOW); }

void trap(const char* why) override { throw NonstandaloneException(); }
};

struct Precompute
: public WalkerPass<
PostWalker<Precompute, UnifiedExpressionVisitor<Precompute>>> {
Expand Down Expand Up @@ -192,7 +96,7 @@ struct Precompute
return;
}
if (flow.breaking()) {
if (flow.breakTo == NOTPRECOMPUTABLE_FLOW) {
if (flow.breakTo == NONSTANDALONE_FLOW) {
return;
}
if (flow.breakTo == RETURN_FLOW) {
Expand Down Expand Up @@ -266,13 +170,16 @@ struct Precompute
private:
// Precompute an expression, returning a flow, which may be a constant
// (that we can replace the expression with if replaceExpression is set).
Flow precomputeExpression(Expression* curr, bool replaceExpression = true) {
Flow precomputeExpression(
Expression* curr,
StandaloneExpressionRunner::Intent intent =
StandaloneExpressionRunner::Intent::REPLACE_EXPRESSION) {
try {
return PrecomputingExpressionRunner(
getModule(), getValues, replaceExpression)
return StandaloneExpressionRunner(
getModule(), getValues, intent, MAX_DEPTH)
.visit(curr);
} catch (PrecomputingExpressionRunner::NonstandaloneException&) {
return Flow(NOTPRECOMPUTABLE_FLOW);
} catch (StandaloneExpressionRunner::NonstandaloneException&) {
return Flow(NONSTANDALONE_FLOW);
}
}

Expand All @@ -284,9 +191,10 @@ struct Precompute
// will have value 1 which we can optimize here, but in precomputeExpression
// we could not do anything.
Literals precomputeValue(Expression* curr) {
// Note that we set replaceExpression to false, as we just care about
// the value here.
Flow flow = precomputeExpression(curr, false /* replaceExpression */);
// Note that we do not intent to replace the expression, as we just care
// about the value here.
Flow flow =
precomputeExpression(curr, StandaloneExpressionRunner::Intent::EVALUATE);
if (flow.breaking()) {
return {};
}
Expand Down
Loading