From 75ed83762ae1855600210655825cc8e9e5ea007f Mon Sep 17 00:00:00 2001 From: dcode Date: Fri, 24 Apr 2020 04:56:11 +0200 Subject: [PATCH 1/5] Refactor ExpressionRunner --- src/binaryen-c.cpp | 10 +- src/passes/Precompute.cpp | 13 +- src/wasm-interpreter.h | 296 +++++++++++++++++++++----------------- 3 files changed, 181 insertions(+), 138 deletions(-) diff --git a/src/binaryen-c.cpp b/src/binaryen-c.cpp index 51a401a3c5f..a9bc9e2b6c9 100644 --- a/src/binaryen-c.cpp +++ b/src/binaryen-c.cpp @@ -4998,16 +4998,18 @@ BinaryenExpressionRef RelooperRenderAndDispose(RelooperRef relooper, namespace wasm { -class CExpressionRunner final : public ExpressionRunner { +// Evaluates a suspected constant expression via the C-API. Inherits most of its +// functionality from ConstantExpressionRunner, which it shares with the +// precompute pass, but must be `final` so we can `delete` its instances. +class CExpressionRunner final + : public ConstantExpressionRunner { public: CExpressionRunner(Module* module, CExpressionRunner::Flags flags, Index maxDepth, Index maxLoopIterations) - : ExpressionRunner( + : ConstantExpressionRunner( module, flags, maxDepth, maxLoopIterations) {} - - void trap(const char* why) override { throw NonconstantException(); } }; } // namespace wasm diff --git a/src/passes/Precompute.cpp b/src/passes/Precompute.cpp index 7d3e691e13a..695b62ab200 100644 --- a/src/passes/Precompute.cpp +++ b/src/passes/Precompute.cpp @@ -42,9 +42,11 @@ namespace wasm { typedef std::unordered_map GetValues; // Precomputes an expression. Errors if we hit anything that can't be -// precomputed. +// precomputed. Inherits most of its functionality from +// ConstantExpressionRunner, which it shares with the C-API, but adds handling +// of GetValues computed during the precompute pass. class PrecomputingExpressionRunner - : public ExpressionRunner { + : public ConstantExpressionRunner { // Concrete values of gets computed during the pass, which the runner does not // know about since it only records values of sets it visits. @@ -66,7 +68,7 @@ class PrecomputingExpressionRunner PrecomputingExpressionRunner(Module* module, GetValues& getValues, bool replaceExpression) - : ExpressionRunner( + : ConstantExpressionRunner( module, replaceExpression ? FlagValues::PRESERVE_SIDEEFFECTS : FlagValues::DEFAULT, @@ -82,10 +84,9 @@ class PrecomputingExpressionRunner return Flow(values); } } - return ExpressionRunner::visitLocalGet(curr); + return ConstantExpressionRunner< + PrecomputingExpressionRunner>::visitLocalGet(curr); } - - void trap(const char* why) override { throw NonconstantException(); } }; struct Precompute diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h index 393c33b7548..0e11b0df8e6 100644 --- a/src/wasm-interpreter.h +++ b/src/wasm-interpreter.h @@ -149,52 +149,13 @@ class Indenter { // Execute an expression template class ExpressionRunner : public OverriddenVisitor { -public: - enum FlagValues { - // By default, just evaluate the expression, i.e. all we want to know is - // whether it computes down to a concrete value, where it is not necessary - // to preserve side effects like those of a `local.tee`. - DEFAULT = 0, - // Be very careful to preserve any side effects. For example, if we are - // intending to replace the expression with a constant afterwards, even if - // we can technically evaluate down to a constant, we still cannot replace - // the expression if it also sets a local, which must be preserved in this - // scenario so subsequent code keeps functioning. - PRESERVE_SIDEEFFECTS = 1 << 0, - // Traverse through function calls, attempting to compute their concrete - // value. Must not be used in function-parallel scenarios, where the called - // function might be concurrently modified, leading to undefined behavior. - TRAVERSE_CALLS = 1 << 1 - }; - - // Flags indicating special requirements, for example whether we are just - // evaluating (default), also going to replace the expression afterwards or - // executing in a function-parallel scenario. See FlagValues. - typedef uint32_t Flags; - - // Indicates no limit of maxDepth or maxLoopIterations. - static const Index NO_LIMIT = 0; - protected: - // Optional module context to search for globals and called functions. NULL if - // we are not interested in any context or if the subclass uses an alternative - // mechanism, like RuntimeExpressionRunner does. - Module* module = nullptr; - - // Flags indicating special requirements. See FlagValues. - Flags flags = FlagValues::DEFAULT; - // Maximum depth before giving up. - Index maxDepth = NO_LIMIT; + Index maxDepth; Index depth = 0; // Maximum iterations before giving up on a loop. - Index maxLoopIterations = NO_LIMIT; - - // Map remembering concrete local values set in the context of this flow. - std::unordered_map localValues; - // Map remembering concrete global values set in the context of this flow. - std::unordered_map globalValues; + Index maxLoopIterations; Flow generateArguments(const ExpressionList& operands, LiteralList& arguments) { @@ -212,32 +173,12 @@ class ExpressionRunner : public OverriddenVisitor { } public: - ExpressionRunner() {} - ExpressionRunner(Index maxDepth) : maxDepth(maxDepth) {} - ExpressionRunner(Module* module, - Flags flags, - Index maxDepth, - Index maxLoopIterations) - : module(module), flags(flags), maxDepth(maxDepth), - maxLoopIterations(maxLoopIterations) {} - - struct NonconstantException { - }; // TODO: use a flow with a special name, as this is likely very slow - - // Gets the module this runner is operating on. - Module* getModule() { return module; } - - // Sets a known local value to use. - void setLocalValue(Index index, Literals& values) { - assert(values.isConcrete()); - localValues[index] = values; - } + // Indicates no limit of maxDepth or maxLoopIterations. + static const Index NO_LIMIT = 0; - // Sets a known global value to use. - void setGlobalValue(Name name, Literals& values) { - assert(values.isConcrete()); - globalValues[name] = values; - } + ExpressionRunner(Index maxDepth = NO_LIMIT, + Index maxLoopIterations = NO_LIMIT) + : maxDepth(maxDepth), maxLoopIterations(maxLoopIterations) {} Flow visit(Expression* curr) { depth++; @@ -1246,6 +1187,153 @@ class ExpressionRunner : public OverriddenVisitor { assert(flow.values.size() > curr->index); return Flow(flow.values[curr->index]); } + Flow visitLocalGet(LocalGet* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitLocalSet(LocalSet* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitGlobalGet(GlobalGet* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitGlobalSet(GlobalSet* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitCall(Call* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitCallIndirect(CallIndirect* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitLoad(Load* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitStore(Store* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitHost(Host* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitMemoryInit(MemoryInit* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitDataDrop(DataDrop* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitMemoryCopy(MemoryCopy* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitMemoryFill(MemoryFill* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitAtomicRMW(AtomicRMW* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitAtomicCmpxchg(AtomicCmpxchg* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitAtomicWait(AtomicWait* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitAtomicNotify(AtomicNotify* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitSIMDLoad(SIMDLoad* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitSIMDLoadSplat(SIMDLoad* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitSIMDLoadExtend(SIMDLoad* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitPush(Push* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitPop(Pop* curr) { WASM_UNREACHABLE("unimp"); } + Flow visitRefNull(RefNull* curr) { + NOTE_ENTER("RefNull"); + return Literal::makeNullref(); + } + Flow visitRefIsNull(RefIsNull* curr) { + NOTE_ENTER("RefIsNull"); + Flow flow = visit(curr->value); + if (flow.breaking()) { + return flow; + } + Literal value = flow.getSingleValue(); + NOTE_EVAL1(value); + return Literal(value.type == Type::nullref); + } + Flow visitRefFunc(RefFunc* curr) { + NOTE_ENTER("RefFunc"); + NOTE_NAME(curr->func); + return Literal::makeFuncref(curr->func); + } + Flow visitTry(Try* curr) { + NOTE_ENTER("Try"); + // FIXME This currently only executes 'try' part. Correctly implement this. + return visit(curr->body); + } + Flow visitThrow(Throw* curr) { + NOTE_ENTER("Throw"); + LiteralList arguments; + Flow flow = generateArguments(curr->operands, arguments); + if (flow.breaking()) { + return flow; + } + NOTE_EVAL1(curr->event); + // FIXME This currently traps. Correctly implement throw. + trap("throw"); + WASM_UNREACHABLE("throw"); + } + Flow visitRethrow(Rethrow* curr) { + NOTE_ENTER("Rethrow"); + Flow flow = visit(curr->exnref); + if (flow.breaking()) { + return flow; + } + if (flow.getType() == Type::nullref) { + trap("rethrow: argument is null"); + } + // FIXME This currently traps. Correctly implement rethrow. + trap("rethrow"); + WASM_UNREACHABLE("rethrow"); + } + Flow visitBrOnExn(BrOnExn* curr) { WASM_UNREACHABLE("unimp"); } + + virtual void trap(const char* why) { WASM_UNREACHABLE("unimp"); } +}; + +// Execute a suspected constant expression (precompute and C-API). +template +class ConstantExpressionRunner : public ExpressionRunner { +public: + enum FlagValues { + // By default, just evaluate the expression, i.e. all we want to know is + // whether it computes down to a concrete value, where it is not necessary + // to preserve side effects like those of a `local.tee`. + DEFAULT = 0, + // Be very careful to preserve any side effects. For example, if we are + // intending to replace the expression with a constant afterwards, even if + // we can technically evaluate down to a constant, we still cannot replace + // the expression if it also sets a local, which must be preserved in this + // scenario so subsequent code keeps functioning. + PRESERVE_SIDEEFFECTS = 1 << 0, + // Traverse through function calls, attempting to compute their concrete + // value. Must not be used in function-parallel scenarios, where the called + // function might be concurrently modified, leading to undefined behavior. + TRAVERSE_CALLS = 1 << 1 + }; + + // Flags indicating special requirements, for example whether we are just + // evaluating (default), also going to replace the expression afterwards or + // executing in a function-parallel scenario. See FlagValues. + typedef uint32_t Flags; + + // Indicates no limit of maxDepth or maxLoopIterations. + static const Index NO_LIMIT = 0; + +protected: + // Optional module context to search for globals and called functions. NULL if + // we are not interested in any context. + Module* module = nullptr; + + // Flags indicating special requirements. See FlagValues. + Flags flags = FlagValues::DEFAULT; + + // Maximum iterations before giving up on a loop. + Index maxLoopIterations = NO_LIMIT; + + // Map remembering concrete local values set in the context of this flow. + std::unordered_map localValues; + // Map remembering concrete global values set in the context of this flow. + std::unordered_map globalValues; + +public: + struct NonconstantException { + }; // TODO: use a flow with a special name, as this is likely very slow + + ConstantExpressionRunner(Module* module, + Flags flags, + Index maxDepth, + Index maxLoopIterations) + : ExpressionRunner(maxDepth, maxLoopIterations), module(module), + flags(flags) {} + + // Gets the module this runner is operating on. + Module* getModule() { return module; } + + // Sets a known local value to use. + void setLocalValue(Index index, Literals& values) { + assert(values.isConcrete()); + localValues[index] = values; + } + + // Sets a known global value to use. + void setGlobalValue(Name name, Literals& values) { + assert(values.isConcrete()); + globalValues[name] = values; + } + Flow visitLocalGet(LocalGet* curr) { NOTE_ENTER("LocalGet"); NOTE_EVAL1(curr->index); @@ -1263,7 +1351,7 @@ class ExpressionRunner : public OverriddenVisitor { // If we are evaluating and not replacing the expression, remember the // constant value set, if any, and see if there is a value flowing through // a tee. - auto setFlow = visit(curr->value); + auto setFlow = ExpressionRunner::visit(curr->value); if (!setFlow.breaking()) { setLocalValue(curr->index, setFlow.values); if (curr->type.isConcrete()) { @@ -1282,7 +1370,7 @@ class ExpressionRunner : public OverriddenVisitor { auto* global = module->getGlobal(curr->name); // Check if the global has an immutable value anyway if (!global->imported() && !global->mutable_) { - return visit(global->init); + return ExpressionRunner::visit(global->init); } } // Check if a constant value has been set in the context of this runner. @@ -1300,7 +1388,7 @@ class ExpressionRunner : public OverriddenVisitor { // constant value set, if any, for subsequent gets. auto* global = module->getGlobal(curr->name); assert(global->mutable_); - auto setFlow = visit(curr->value); + auto setFlow = ExpressionRunner::visit(curr->value); if (!setFlow.breaking()) { setGlobalValue(curr->name, setFlow.values); return Flow(); @@ -1324,13 +1412,13 @@ class ExpressionRunner : public OverriddenVisitor { auto prevLocalValues = localValues; localValues.clear(); for (Index i = 0; i < numOperands; ++i) { - auto argFlow = visit(curr->operands[i]); + auto argFlow = ExpressionRunner::visit(curr->operands[i]); if (!argFlow.breaking()) { assert(argFlow.values.isConcrete()); localValues[i] = argFlow.values; } } - auto retFlow = visit(func->body); + auto retFlow = ExpressionRunner::visit(func->body); localValues = prevLocalValues; if (retFlow.breakTo == RETURN_FLOW) { return Flow(retFlow.values); @@ -1411,72 +1499,24 @@ class ExpressionRunner : public OverriddenVisitor { NOTE_ENTER("Pop"); return Flow(NONCONSTANT_FLOW); } - Flow visitRefNull(RefNull* curr) { - NOTE_ENTER("RefNull"); - return Literal::makeNullref(); - } - Flow visitRefIsNull(RefIsNull* curr) { - NOTE_ENTER("RefIsNull"); - Flow flow = visit(curr->value); - if (flow.breaking()) { - return flow; - } - Literal value = flow.getSingleValue(); - NOTE_EVAL1(value); - return Literal(value.type == Type::nullref); - } - Flow visitRefFunc(RefFunc* curr) { - NOTE_ENTER("RefFunc"); - NOTE_NAME(curr->func); - return Literal::makeFuncref(curr->func); - } - Flow visitTry(Try* curr) { - NOTE_ENTER("Try"); - // FIXME This currently only executes 'try' part. Correctly implement this. - return visit(curr->body); - } - Flow visitThrow(Throw* curr) { - NOTE_ENTER("Throw"); - LiteralList arguments; - Flow flow = generateArguments(curr->operands, arguments); - if (flow.breaking()) { - return flow; - } - NOTE_EVAL1(curr->event); - // FIXME This currently traps. Correctly implement throw. - trap("throw"); - WASM_UNREACHABLE("throw"); - } - Flow visitRethrow(Rethrow* curr) { - NOTE_ENTER("Rethrow"); - Flow flow = visit(curr->exnref); - if (flow.breaking()) { - return flow; - } - if (flow.getType() == Type::nullref) { - trap("rethrow: argument is null"); - } - // FIXME This currently traps. Correctly implement rethrow. - trap("rethrow"); - WASM_UNREACHABLE("rethrow"); - } Flow visitBrOnExn(BrOnExn* curr) { NOTE_ENTER("BrOnExn"); return Flow(NONCONSTANT_FLOW); } - virtual void trap(const char* why) { WASM_UNREACHABLE(why); } + void trap(const char* why) override { throw NonconstantException(); } }; -// Execute an constant expression in a global init or memory offset. +// Execute an initializer expression of a global, data or element segment. +// see: https://webassembly.org/docs/modules/#initializer-expression template -class ConstantExpressionRunner - : public ExpressionRunner> { +class InitializerExpressionRunner + : public ExpressionRunner> { GlobalManager& globals; public: - ConstantExpressionRunner(GlobalManager& globals, Index maxDepth) - : ExpressionRunner>(maxDepth), + InitializerExpressionRunner(GlobalManager& globals, Index maxDepth) + : ExpressionRunner>(maxDepth), globals(globals) {} Flow visitGlobalGet(GlobalGet* curr) { return Flow(globals[curr->name]); } @@ -1674,7 +1714,7 @@ template class ModuleInstanceBase { // generate internal (non-imported) globals ModuleUtils::iterDefinedGlobals(wasm, [&](Global* global) { globals[global->name] = - ConstantExpressionRunner(globals, maxDepth) + InitializerExpressionRunner(globals, maxDepth) .visit(global->init) .values; }); @@ -1739,7 +1779,7 @@ template class ModuleInstanceBase { void initializeTableContents() { for (auto& segment : wasm.table.segments) { Address offset = - (uint32_t)ConstantExpressionRunner(globals, maxDepth) + (uint32_t)InitializerExpressionRunner(globals, maxDepth) .visit(segment.offset) .getSingleValue() .geti32(); From c7f2fb2063fee990fc225d0792c1ba25a7c705f8 Mon Sep 17 00:00:00 2001 From: dcode Date: Fri, 24 Apr 2020 05:15:10 +0200 Subject: [PATCH 2/5] would you be so kind, CI From e616f1061d626c527371861f168e76ae0191169e Mon Sep 17 00:00:00 2001 From: dcode Date: Fri, 24 Apr 2020 05:16:08 +0200 Subject: [PATCH 3/5] no? From f26d932d59fef40ccee298edf74e064311d16f6f Mon Sep 17 00:00:00 2001 From: dcode Date: Fri, 24 Apr 2020 05:37:30 +0200 Subject: [PATCH 4/5] remove duplicate field (CI?) --- src/wasm-interpreter.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h index 0e11b0df8e6..0124055ad2b 100644 --- a/src/wasm-interpreter.h +++ b/src/wasm-interpreter.h @@ -1300,9 +1300,6 @@ class ConstantExpressionRunner : public ExpressionRunner { // Flags indicating special requirements. See FlagValues. Flags flags = FlagValues::DEFAULT; - // Maximum iterations before giving up on a loop. - Index maxLoopIterations = NO_LIMIT; - // Map remembering concrete local values set in the context of this flow. std::unordered_map localValues; // Map remembering concrete global values set in the context of this flow. From 35b60af10f08f560268a512edbfdbe8d09e21093 Mon Sep 17 00:00:00 2001 From: dcode Date: Fri, 24 Apr 2020 06:45:44 +0200 Subject: [PATCH 5/5] what if... --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eb321e9d2c6..4bf4099d684 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,6 +3,7 @@ name: CI # Trigger on all branches, even before a PR is opened. on: push: + pull_request: jobs: