From d1979b25f3969c7b48943acbf1a8f3854f124bbd Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 23 May 2017 11:42:53 -0700 Subject: [PATCH 01/44] start ssa pass --- src/passes/CMakeLists.txt | 1 + src/passes/CoalesceLocals.cpp | 30 +---- src/passes/SSAify.cpp | 243 ++++++++++++++++++++++++++++++++++ src/passes/pass.cpp | 1 + src/passes/passes.h | 1 + src/support/permutations.h | 55 ++++++++ 6 files changed, 302 insertions(+), 29 deletions(-) create mode 100644 src/passes/SSAify.cpp create mode 100644 src/support/permutations.h diff --git a/src/passes/CMakeLists.txt b/src/passes/CMakeLists.txt index 66f86a02f51..b774ebd2d98 100644 --- a/src/passes/CMakeLists.txt +++ b/src/passes/CMakeLists.txt @@ -32,6 +32,7 @@ SET(passes_SOURCES ReorderLocals.cpp ReorderFunctions.cpp SimplifyLocals.cpp + SSAify.cpp Vacuum.cpp ) ADD_LIBRARY(passes STATIC ${passes_SOURCES}) diff --git a/src/passes/CoalesceLocals.cpp b/src/passes/CoalesceLocals.cpp index 2828c9955fa..fb81a5981d1 100644 --- a/src/passes/CoalesceLocals.cpp +++ b/src/passes/CoalesceLocals.cpp @@ -32,6 +32,7 @@ #include "cfg/cfg-traversal.h" #include "wasm-builder.h" #include "support/learning.h" +#include "support/permutations.h" #ifdef CFG_PROFILE #include "support/timing.h" #endif @@ -533,35 +534,6 @@ void CoalesceLocals::pickIndicesFromOrder(std::vector& order, std::vector } } -// Utilities for operating on permutation vectors - -static std::vector makeIdentity(Index num) { - std::vector ret; - ret.resize(num); - for (Index i = 0; i < num; i++) { - ret[i] = i; - } - return ret; -} - -static void setIdentity(std::vector& ret) { - auto num = ret.size(); - assert(num > 0); // must already be of the right size - for (Index i = 0; i < num; i++) { - ret[i] = i; - } -} - -static std::vector makeReversed(std::vector& original) { - std::vector ret; - auto num = original.size(); - ret.resize(num); - for (Index i = 0; i < num; i++) { - ret[original[i]] = i; - } - return ret; -} - // given a baseline order, adjust it based on an important order of priorities (higher values // are higher priority). The priorities take precedence, unless they are equal and then // the original order should be kept. diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp new file mode 100644 index 00000000000..95c9736f8e3 --- /dev/null +++ b/src/passes/SSAify.cpp @@ -0,0 +1,243 @@ +/* + * Copyright 2016 WebAssembly Community Group participants + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// +// Transforms code into SSA form. That ensures each variable has a +// single assignment. For phis, we do not add a new node to the AST, +// so the result is multiple assignments but with the guarantee that +// they all travel directly to the same basic block, i.e., they are +// a way to represent a phi in our AST. +// + +#include "wasm.h" +#include "pass.h" +#include "support/permutations.h" + +namespace wasm { + +// Tracks assignments to locals, assuming single-assignment form, i.e., +// each assignment creates a new variable. + +template +struct SetTrackingWalker : public PostWalker { + typedef std::vector NameMapping; // old index (in original code) => new index (in SSA form, new variables) + + struct BreakInfo { + NameMapping mapping; + Expression** origin; // the origin of a node where a phi would go. note that *origin can be nullptr, in which case we can just fill it + enum PhiType { + Before = 0, // a phi should go right before the origin (e.g., origin is a loop and this is the entrance) + After = 1, // a phi should go right after the origin (e.g. this is an if body) + Internal = 2 // origin is the breaking instruction itself, we must add the phi internally (depending on if the break is condition or has a value, etc., + // or for a block as the last instruction) + } type; + // TODO: move semantics? + BreakInfo(NameMapping mapping, Expression** origin, PhiType type) : mapping(mapping), origin(origin), type(type) {} + }; + + Index numLocals; + NameMapping currMapping; + Index nextIndex; + std::vector mappingStack; // used in ifs, loops + std::map> breakInfos; // break target => infos that reach it + + SetTrackingWalker(Function* func) { + numLocals = func->getNumLocals(); + if (numLocals == 0) return; // nothing to do + // We begin with each param being assigned from the incoming value, and the zero-init for the locals, + // so the initial state is the identity permutation + currMapping.resize(numLocals); + setIdentity(currMapping); + nextIndex = numLocals; + PostWalker::walk(func->body); + } + + // control flow + + static void doVisitBlock(SubType* self, Expression** currp) { + auto* curr = (*currp)->cast(); + if (curr->name.is() && self->breakInfos.find(curr->name) != self->breakInfos.end()) { + auto& infos = self->breakInfos[curr->name]; + infos.emplace_back(std::move(self->currMapping), currp, BreakInfo::Internal); + self->currMapping = std::move(self->merge(infos), curr->name); + } + } + static void doIfCondition(SubType* self, Expression** currp) { + auto* curr = (*currp)->cast(); + if (!curr->ifFalse) { + self->mappingStack.push_back(self->currMapping); + } + } + static void doIfTrue(SubType* self, Expression** currp) { + auto* curr = (*currp)->cast(); + if (curr->ifFalse) { + self->mappingStack.push_back(self->currMapping); + } else { + // that's it for this if, merge + std::vector breaks; + breaks.emplace_back(std::move(self->currMapping), &curr->ifFalse, BreakInfo::After); + breaks.emplace_back(self->mappingStack.back(), &curr->condition, BreakInfo::After); + self->mappingStack.pop_back(); + self->currMapping = std::move(merge(breaks)); + } + } + static void doIfFalse(SubType* self, Expression** currp) { + auto* curr = (*currp)->cast(); + std::vector breaks; + breaks.emplace_back(std::move(self->currMapping), &curr->ifFalse, BreakInfo::After); + breaks.emplace_back(self->mappingStack.back(), &curr->ifTrue, BreakInfo::After); + self->mappingStack.pop_back(); + self->currMapping = std::move(merge(breaks)); + } + static void doPreLoop(SubType* self, Expression** currp) { + // save the state before entering the loop, for calculation later of the merge at the loop top + self->mappingStack.push_back(self->currMapping); + } + static void doVisitLoop(SubType* self, Expression** currp) { + auto* curr = (*currp)->cast(); + if (curr->name.is() && self->breakInfos.find(curr->name) != self->breakInfos.end()) { + auto& infos = self->breakInfos[curr->name]; + infos.emplace_back(self->mappingStack.back(), currp, BreakInfo::Before); + self->merge(infos, curr->name); // output is not assigned anywhere, this is an interesting code path + } + self->mappingStack.pop_back(); + } + static void visitBreak(SubType* self, Expression** currp) { + auto* curr = (*currp)->cast(); + self->breakInfos[curr->name].emplace_back(std::move(self->currMapping), currp, BreakInfo::Internal); + if (!(*currp)->cast()->condition) { + setUnreachable(self->currMapping); + } + } + static void visitSwitch(SubType* self, Expression** currp) { + auto* curr = (*currp)->cast(); + std::set all; + for (auto target : curr->targets) { + all.insert(target); + } + all.insert(curr->default_); + for (auto target : all) { + self->breakInfos[curr->default_].emplace_back(self->currMapping, currp, BreakInfo::Switch); + } + setUnreachable(self->currMapping); + } + void visitReturn(Return *curr) { + setUnreachable(currMapping); + } + void visitUnreachable(Unreachable *curr) { + setUnreachable(currMapping); + } + + // assignments + + void visitSetLocal(SetLocal *curr) { + currMapping[curr->index] = nextIndex++; // a new assignment, trample the old + } + + // traversal + + static void scan(SubType* self, Expression** currp) { + if (auto* iff = (*currp)->dynCast()) { + // if needs special handling + if (iff->ifFalse) { + self->pushTask(SubType::doIfFalse, currp); + self->pushTask(SubType::scan, iff->ifFalse); + } + self->pushTask(SubType::doIfTrue, currp); + self->pushTask(SubType::scan, iff->ifTrue); + self->pushTask(SubType::doIfCondition, currp); + self->pushTask(SubType::scan, iff->condition); + } else { + PostWalker::scan(self, currp); + } + + // loops need pre-order visiting too + if ((*currp)->is()) { + self->pushTask(SubType::doPreLoop, currp); + } + } + + // helpers + + void setUnreachable(NameMapping& mapping) { + mapping.resize(numLocals); // may have been emptied by a move + mapping[0] = Index(-1); + } + + bool isUnreachable(NameMapping& mapping) { + return mapping[0] == Index(-1); + } + + // merges a bunch of infos into one. where necessary calls a phi hook. + NameMapping& merge(std::vector& infos, Name name = Name()) { + auto& out = infos[0]; + for (Index i = 0; i < numLocals; i++) { + Index seen = -1; + for (auto& info : infos) { + if (isUnreachable(info.mapping)) continue; + if (seen == -1) { + seen = info.mapping[i]; + } else { + if (info.mapping[i] != seen) { + // we need a phi here + seen = nextIndex++; + createPhi(infos, i, seen, name); + break; + } + } + } + out.mapping[i] = seen; + } + return out.mapping; + } + + void createPhi(std::vector& infos, Index old, Index new_, Name name) { + abort(); // override this in child classes + } +}; + +struct LoopPhiFinder : public SetTrackingWalker> { + std::map> loopPhis; // loop name => list of source indexes that need a phi + + LoopPhiFinder(Function* func) : SetTrackingWalker(func) {} + + void createPhi(std::vector& infos, Index old, Index new_, Name name) { + for (auto& info : infos) { + if (info.type == BreakInfo::Before) { + auto* loop = (*info.origin)->cast(); + loopPhis[loop->name].push_back(old); + } + } + } +}; + +struct SSAify : public Pass { + bool isFunctionParallel() override { return true; } + + Pass* create() override { return new SSAify; } + + void runFunction(PassRunner* runner, Module* module, Function* function) override { + // count how many set_locals each local has. if it already has just 1, we can ignore it. + LoopPhiFinder finder(&numSetLocals, func); + } +}; + +Pass *createSSAifyPass() { + return new SSAify(); +} + +} // namespace wasm + diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp index 2fb0d5c3b3c..acc5f17bf51 100644 --- a/src/passes/pass.cpp +++ b/src/passes/pass.cpp @@ -101,6 +101,7 @@ void PassRegistry::registerPasses() { registerPass("simplify-locals-notee", "miscellaneous locals-related optimizations", createSimplifyLocalsNoTeePass); registerPass("simplify-locals-nostructure", "miscellaneous locals-related optimizations", createSimplifyLocalsNoStructurePass); registerPass("simplify-locals-notee-nostructure", "miscellaneous locals-related optimizations", createSimplifyLocalsNoTeeNoStructurePass); + registerPass("ssa", "ssa-ify variables so that they have a single assignment", createSSAifyPass); registerPass("vacuum", "removes obviously unneeded code", createVacuumPass); registerPass("precompute", "computes compile-time evaluatable expressions", createPrecomputePass); // registerPass("lower-i64", "lowers i64 into pairs of i32s", createLowerInt64Pass); diff --git a/src/passes/passes.h b/src/passes/passes.h index 6322aad3cf3..806ce0c2b18 100644 --- a/src/passes/passes.h +++ b/src/passes/passes.h @@ -59,6 +59,7 @@ Pass *createSimplifyLocalsPass(); Pass *createSimplifyLocalsNoTeePass(); Pass *createSimplifyLocalsNoStructurePass(); Pass *createSimplifyLocalsNoTeeNoStructurePass(); +Pass *createSSAifyPass(); Pass *createVacuumPass(); Pass *createPrecomputePass(); //Pass *createLowerInt64Pass(); diff --git a/src/support/permutations.h b/src/support/permutations.h new file mode 100644 index 00000000000..214063058f2 --- /dev/null +++ b/src/support/permutations.h @@ -0,0 +1,55 @@ +/* + * Copyright 2016 WebAssembly Community Group participants + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Utilities for operating on permutation vectors + +#ifndef wasm_support_permutations_h +#define wasm_support_permutations_h + +#include "wasm.h" + +namespace wasm { + +inline std::vector makeIdentity(Index num) { + std::vector ret; + ret.resize(num); + for (Index i = 0; i < num; i++) { + ret[i] = i; + } + return ret; +} + +inline void setIdentity(std::vector& ret) { + auto num = ret.size(); + assert(num > 0); // must already be of the right size + for (Index i = 0; i < num; i++) { + ret[i] = i; + } +} + +inline std::vector makeReversed(std::vector& original) { + std::vector ret; + auto num = original.size(); + ret.resize(num); + for (Index i = 0; i < num; i++) { + ret[original[i]] = i; + } + return ret; +} + +} // namespace wasm + +#endif // permutations From 9563cd2787006f25fcf24e2b18e2671dfbbf3d0f Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 23 May 2017 14:09:27 -0700 Subject: [PATCH 02/44] wip --- src/passes/SSAify.cpp | 194 +++++++++++++++++++++++------------------- 1 file changed, 107 insertions(+), 87 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 95c9736f8e3..8d28b60aa5d 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -20,49 +20,45 @@ // so the result is multiple assignments but with the guarantee that // they all travel directly to the same basic block, i.e., they are // a way to represent a phi in our AST. +// TODO: consider adding a "proper" phi node to the AST, that passes +// can utilize // #include "wasm.h" #include "pass.h" +#include "wasm-builder.h" #include "support/permutations.h" +#include "ast/manipulation.h" namespace wasm { // Tracks assignments to locals, assuming single-assignment form, i.e., // each assignment creates a new variable. -template -struct SetTrackingWalker : public PostWalker { - typedef std::vector NameMapping; // old index (in original code) => new index (in SSA form, new variables) - - struct BreakInfo { - NameMapping mapping; - Expression** origin; // the origin of a node where a phi would go. note that *origin can be nullptr, in which case we can just fill it - enum PhiType { - Before = 0, // a phi should go right before the origin (e.g., origin is a loop and this is the entrance) - After = 1, // a phi should go right after the origin (e.g. this is an if body) - Internal = 2 // origin is the breaking instruction itself, we must add the phi internally (depending on if the break is condition or has a value, etc., - // or for a block as the last instruction) - } type; - // TODO: move semantics? - BreakInfo(NameMapping mapping, Expression** origin, PhiType type) : mapping(mapping), origin(origin), type(type) {} - }; +struct SSAify : public PostWalker { + // we map (old local index) => the set_local for that index. The new + // index for the local can be seen in that set local. + // this can be nullptr if there is no set_local, and instead the value + // is a parameter, or the zero init. + typedef std::vector Mapping; Index numLocals; - NameMapping currMapping; + Mapping currMapping; Index nextIndex; - std::vector mappingStack; // used in ifs, loops - std::map> breakInfos; // break target => infos that reach it + std::vector mappingStack; // used in ifs, loops + std::map> breakInfos; // break target => infos that reach it + std::vector> loopGetStack; // stack of loops, all the gets in each, so we can update them for phis + std::map originalGetIndexes; - SetTrackingWalker(Function* func) { + void doWalkFunction(Function* func) { numLocals = func->getNumLocals(); if (numLocals == 0) return; // nothing to do // We begin with each param being assigned from the incoming value, and the zero-init for the locals, // so the initial state is the identity permutation currMapping.resize(numLocals); - setIdentity(currMapping); + for (auto& set : currMapping) set = nullptr; nextIndex = numLocals; - PostWalker::walk(func->body); + PostWalker::walk(func->body); } // control flow @@ -75,45 +71,55 @@ struct SetTrackingWalker : public PostWalker { self->currMapping = std::move(self->merge(infos), curr->name); } } - static void doIfCondition(SubType* self, Expression** currp) { + + void finishIf() { + // that's it for this if, merge + std::vector breaks; + breaks.emplace_back(std::move(currMapping)); + breaks.emplace_back(std::move(mappingStack.back())); + mappingStack.pop_back(); + currMapping = std::move(merge(breaks)); + } + + static void afterIfCondition(SubType* self, Expression** currp) { auto* curr = (*currp)->cast(); - if (!curr->ifFalse) { - self->mappingStack.push_back(self->currMapping); - } + self->mappingStack.push_back(self->currMapping); } - static void doIfTrue(SubType* self, Expression** currp) { + static void afterIfTrue(SubType* self, Expression** currp) { auto* curr = (*currp)->cast(); if (curr->ifFalse) { - self->mappingStack.push_back(self->currMapping); + auto afterCondition = std::move(self.mappingStack.back()); + self->mappingStack.back() = std::move(self->currMapping); + self->currMapping = std::move(afterCondition); } else { - // that's it for this if, merge - std::vector breaks; - breaks.emplace_back(std::move(self->currMapping), &curr->ifFalse, BreakInfo::After); - breaks.emplace_back(self->mappingStack.back(), &curr->condition, BreakInfo::After); - self->mappingStack.pop_back(); - self->currMapping = std::move(merge(breaks)); + self->finishIf(); } } - static void doIfFalse(SubType* self, Expression** currp) { - auto* curr = (*currp)->cast(); - std::vector breaks; - breaks.emplace_back(std::move(self->currMapping), &curr->ifFalse, BreakInfo::After); - breaks.emplace_back(self->mappingStack.back(), &curr->ifTrue, BreakInfo::After); - self->mappingStack.pop_back(); - self->currMapping = std::move(merge(breaks)); + static void afterIfFalse(SubType* self, Expression** currp) { + self->finishIf(); } - static void doPreLoop(SubType* self, Expression** currp) { + static void beforeLoop(SubType* self, Expression** currp) { // save the state before entering the loop, for calculation later of the merge at the loop top self->mappingStack.push_back(self->currMapping); + self->loopGetStack.push_back({}); } static void doVisitLoop(SubType* self, Expression** currp) { auto* curr = (*currp)->cast(); if (curr->name.is() && self->breakInfos.find(curr->name) != self->breakInfos.end()) { auto& infos = self->breakInfos[curr->name]; - infos.emplace_back(self->mappingStack.back(), currp, BreakInfo::Before); - self->merge(infos, curr->name); // output is not assigned anywhere, this is an interesting code path + infos.emplace_back(std::move(self->mappingStack.back())); + auto before = infos.back(); + auto& merged = self->merge(infos); + // every local we created a phi for requires us to update get_local operations in + // the loop + auto& gets = self->loopGetStack.back(); + for (auto* get : gets) { + auto original = self->originalGetIndexes[get]; + get->index = merged[original]; + } } self->mappingStack.pop_back(); + self->loopGetStack.pop_back(); } static void visitBreak(SubType* self, Expression** currp) { auto* curr = (*currp)->cast(); @@ -141,6 +147,18 @@ struct SetTrackingWalker : public PostWalker { setUnreachable(currMapping); } + void visitGetLocal(GetLocal* curr) { + for (auto& loopGets : loopGetStack) { + loopGets.push_back(curr); + } + originalGetIndexes[curr] = curr->index; + curr->index = currMapping[curr->index]; + } + void visitSetLocal(SetLocal* curr) { + currMapping[curr->index] = curr; + curr->index = nextIndex++; + } + // assignments void visitSetLocal(SetLocal *curr) { @@ -153,12 +171,12 @@ struct SetTrackingWalker : public PostWalker { if (auto* iff = (*currp)->dynCast()) { // if needs special handling if (iff->ifFalse) { - self->pushTask(SubType::doIfFalse, currp); + self->pushTask(SubType::afterIfFalse, currp); self->pushTask(SubType::scan, iff->ifFalse); } - self->pushTask(SubType::doIfTrue, currp); + self->pushTask(SubType::afterIfTrue, currp); self->pushTask(SubType::scan, iff->ifTrue); - self->pushTask(SubType::doIfCondition, currp); + self->pushTask(SubType::afterIfCondition, currp); self->pushTask(SubType::scan, iff->condition); } else { PostWalker::scan(self, currp); @@ -166,75 +184,77 @@ struct SetTrackingWalker : public PostWalker { // loops need pre-order visiting too if ((*currp)->is()) { - self->pushTask(SubType::doPreLoop, currp); + self->pushTask(SubType::beforeLoop, currp); } } // helpers - void setUnreachable(NameMapping& mapping) { + void setUnreachable(Mapping& mapping) { mapping.resize(numLocals); // may have been emptied by a move mapping[0] = Index(-1); } - bool isUnreachable(NameMapping& mapping) { + bool isUnreachable(Mapping& mapping) { return mapping[0] == Index(-1); } - // merges a bunch of infos into one. where necessary calls a phi hook. - NameMapping& merge(std::vector& infos, Name name = Name()) { + // merges a bunch of infos into one + Mapping& merge(std::vector& infos) { + assert(infos.size() > 0); auto& out = infos[0]; + if (infos.size() == 1) { + return infos[0]; + } for (Index i = 0; i < numLocals; i++) { - Index seen = -1; + SetLocal* merged; + bool seen = false; for (auto& info : infos) { - if (isUnreachable(info.mapping)) continue; - if (seen == -1) { - seen = info.mapping[i]; + if (isUnreachable(info)) continue; + if (!seen) { + merged = info[i]; + seen = true; } else { - if (info.mapping[i] != seen) { - // we need a phi here - seen = nextIndex++; - createPhi(infos, i, seen, name); + if (info[i] != merged) { + // we need a phi for this local + merged = nextIndex++; + createPhi(infos, i, merged); break; } } } - out.mapping[i] = seen; + if (seen) { + out[i] = merged; + } } - return out.mapping; + return out; } - void createPhi(std::vector& infos, Index old, Index new_, Name name) { - abort(); // override this in child classes - } -}; - -struct LoopPhiFinder : public SetTrackingWalker> { - std::map> loopPhis; // loop name => list of source indexes that need a phi - - LoopPhiFinder(Function* func) : SetTrackingWalker(func) {} - - void createPhi(std::vector& infos, Index old, Index new_, Name name) { + void createPhi(std::vector& infos, Index old, Index new_) { + // assign the set value to the phi value as well + Builder builder(*getModule()); for (auto& info : infos) { - if (info.type == BreakInfo::Before) { - auto* loop = (*info.origin)->cast(); - loopPhis[loop->name].push_back(old); + if (!info[index]) { + // this is a param or the zero init value. add a set first thing in + // the function + auto* block = builder.blockify(getFunction()->body); + getFunction()->body = block; + auto* set = builder.makeSetLocal( + old_, + builder.makeGetLocal(new_, getFunction()->getLocalType(old)) + ); + info[old] = set; + ExpressionManipulator::spliceIntoBlock(block, 0, set); } + // now a set exists, just add a tee of its value, so we also set the phi merge var + info[old]->value = builder.makeTeeLocal( + new_, + info[old]->value + ); } } }; -struct SSAify : public Pass { - bool isFunctionParallel() override { return true; } - - Pass* create() override { return new SSAify; } - - void runFunction(PassRunner* runner, Module* module, Function* function) override { - // count how many set_locals each local has. if it already has just 1, we can ignore it. - LoopPhiFinder finder(&numSetLocals, func); - } -}; - Pass *createSSAifyPass() { return new SSAify(); } From 80613202c93e66a460cd7766f64bcc3f31f3fefa Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 23 May 2017 14:53:46 -0700 Subject: [PATCH 03/44] more --- src/passes/SSAify.cpp | 45 ++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 8d28b60aa5d..4946234f7d1 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -63,7 +63,7 @@ struct SSAify : public PostWalker { // control flow - static void doVisitBlock(SubType* self, Expression** currp) { + static void doVisitBlock(SSAify* self, Expression** currp) { auto* curr = (*currp)->cast(); if (curr->name.is() && self->breakInfos.find(curr->name) != self->breakInfos.end()) { auto& infos = self->breakInfos[curr->name]; @@ -81,11 +81,11 @@ struct SSAify : public PostWalker { currMapping = std::move(merge(breaks)); } - static void afterIfCondition(SubType* self, Expression** currp) { + static void afterIfCondition(SSAify* self, Expression** currp) { auto* curr = (*currp)->cast(); self->mappingStack.push_back(self->currMapping); } - static void afterIfTrue(SubType* self, Expression** currp) { + static void afterIfTrue(SSAify* self, Expression** currp) { auto* curr = (*currp)->cast(); if (curr->ifFalse) { auto afterCondition = std::move(self.mappingStack.back()); @@ -95,15 +95,15 @@ struct SSAify : public PostWalker { self->finishIf(); } } - static void afterIfFalse(SubType* self, Expression** currp) { + static void afterIfFalse(SSAify* self, Expression** currp) { self->finishIf(); } - static void beforeLoop(SubType* self, Expression** currp) { + static void beforeLoop(SSAify* self, Expression** currp) { // save the state before entering the loop, for calculation later of the merge at the loop top self->mappingStack.push_back(self->currMapping); self->loopGetStack.push_back({}); } - static void doVisitLoop(SubType* self, Expression** currp) { + static void doVisitLoop(SSAify* self, Expression** currp) { auto* curr = (*currp)->cast(); if (curr->name.is() && self->breakInfos.find(curr->name) != self->breakInfos.end()) { auto& infos = self->breakInfos[curr->name]; @@ -121,14 +121,14 @@ struct SSAify : public PostWalker { self->mappingStack.pop_back(); self->loopGetStack.pop_back(); } - static void visitBreak(SubType* self, Expression** currp) { + static void visitBreak(SSAify* self, Expression** currp) { auto* curr = (*currp)->cast(); self->breakInfos[curr->name].emplace_back(std::move(self->currMapping), currp, BreakInfo::Internal); if (!(*currp)->cast()->condition) { setUnreachable(self->currMapping); } } - static void visitSwitch(SubType* self, Expression** currp) { + static void visitSwitch(SSAify* self, Expression** currp) { auto* curr = (*currp)->cast(); std::set all; for (auto target : curr->targets) { @@ -147,44 +147,40 @@ struct SSAify : public PostWalker { setUnreachable(currMapping); } + // local usage + void visitGetLocal(GetLocal* curr) { for (auto& loopGets : loopGetStack) { loopGets.push_back(curr); } originalGetIndexes[curr] = curr->index; - curr->index = currMapping[curr->index]; + curr->index = currMapping[curr->index]->index; } void visitSetLocal(SetLocal* curr) { currMapping[curr->index] = curr; curr->index = nextIndex++; } - // assignments - - void visitSetLocal(SetLocal *curr) { - currMapping[curr->index] = nextIndex++; // a new assignment, trample the old - } - // traversal - static void scan(SubType* self, Expression** currp) { + static void scan(SSAify* self, Expression** currp) { if (auto* iff = (*currp)->dynCast()) { // if needs special handling if (iff->ifFalse) { - self->pushTask(SubType::afterIfFalse, currp); - self->pushTask(SubType::scan, iff->ifFalse); + self->pushTask(SSAify::afterIfFalse, currp); + self->pushTask(SSAify::scan, iff->ifFalse); } - self->pushTask(SubType::afterIfTrue, currp); - self->pushTask(SubType::scan, iff->ifTrue); - self->pushTask(SubType::afterIfCondition, currp); - self->pushTask(SubType::scan, iff->condition); + self->pushTask(SSAify::afterIfTrue, currp); + self->pushTask(SSAify::scan, iff->ifTrue); + self->pushTask(SSAify::afterIfCondition, currp); + self->pushTask(SSAify::scan, iff->condition); } else { - PostWalker::scan(self, currp); + PostWalker::scan(self, currp); } // loops need pre-order visiting too if ((*currp)->is()) { - self->pushTask(SubType::beforeLoop, currp); + self->pushTask(SSAify::beforeLoop, currp); } } @@ -217,6 +213,7 @@ struct SSAify : public PostWalker { } else { if (info[i] != merged) { // we need a phi for this local +// XXX merge to a single set_local how? merged = nextIndex++; createPhi(infos, i, merged); break; From d4a94f2f3da939be2778f9c69a607ecd49236903 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 24 May 2017 13:44:22 -0700 Subject: [PATCH 04/44] fixes --- src/ast/literal-utils.h | 46 +++++++++++++++++++++++++++++ src/passes/SSAify.cpp | 65 +++++++++++++++++++++++++++-------------- 2 files changed, 89 insertions(+), 22 deletions(-) create mode 100644 src/ast/literal-utils.h diff --git a/src/ast/literal-utils.h b/src/ast/literal-utils.h new file mode 100644 index 00000000000..7e75e8bc811 --- /dev/null +++ b/src/ast/literal-utils.h @@ -0,0 +1,46 @@ +/* + * Copyright 2017 WebAssembly Community Group participants + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef wasm_ast_literl_utils_h +#define wasm_ast_literl_utils_h + +#include "wasm.h" + +namespace wasm { + +namespace LiteralUtils { + +inline Expression* makeZero(WasmType type, Module& wasm) { + Literal value; + switch (type) { + case i32: value = Literal(int32_t(0)); break; + case i64: value = Literal(int64_t(0)); break; + case f32: value = Literal(float(0)); break; + case f64: value = Literal(double(0)); break; + default: WASM_UNREACHABLE(); + } + auto* ret = wasm.allocator.alloc(); + ret->value = value; + ret->type = value.type; + return ret; +} + +} // namespace LiteralUtils + +} // namespace wasm + +#endif // wasm_ast_literl_utils_h + diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 4946234f7d1..5d8f45e9a60 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -29,6 +29,7 @@ #include "wasm-builder.h" #include "support/permutations.h" #include "ast/manipulation.h" +#include "ast/literal-utils.h" namespace wasm { @@ -154,7 +155,18 @@ struct SSAify : public PostWalker { loopGets.push_back(curr); } originalGetIndexes[curr] = curr->index; - curr->index = currMapping[curr->index]->index; + auto* set = currMapping[curr->index]; + if (set) { + curr->index = set->index; + } else { + // no set, this is either a param or a zero init + if (curr->index < getFunction()->getNumParams()) { + // nothing to do, keep getting that param + } else { + // just replace with zero + replaceCurrent(LiteralUtils::makeZero(curr->type, *getModule()); + } + } } void visitSetLocal(SetLocal* curr) { currMapping[curr->index] = curr; @@ -195,27 +207,36 @@ struct SSAify : public PostWalker { return mapping[0] == Index(-1); } - // merges a bunch of infos into one - Mapping& merge(std::vector& infos) { - assert(infos.size() > 0); - auto& out = infos[0]; - if (infos.size() == 1) { - return infos[0]; + // merges a bunch of infos into one. + // if we need phis, writes them into the provided vector. the caller should + // ensure those are placed in the right location + Mapping& merge(std::vector& mappings) { + assert(mappings.size() > 0); + auto& out = mappings[0]; + if (mappings.size() == 1) { + return mappings[0]; } for (Index i = 0; i < numLocals; i++) { SetLocal* merged; bool seen = false; - for (auto& info : infos) { - if (isUnreachable(info)) continue; + for (auto& mapping : mappings) { + if (isUnreachable(mapping)) continue; if (!seen) { - merged = info[i]; + merged = mapping[i]; seen = true; } else { - if (info[i] != merged) { - // we need a phi for this local -// XXX merge to a single set_local how? - merged = nextIndex++; - createPhi(infos, i, merged); + if (mapping[i] != merged) { + // we need a phi for this local, e.g., imagine + // if (..) { x = 1 } else { x = 2 } ..use x here.. + // create a new local, and also write to it at the old write locations + // if (..) { x = y = 1 } else { x = y = 2 } ..use y here.. + // (not that y seems as "single-assignment" as x here, but the point is + // that x may be merged multiple times, so we need a different phi merge + // local for each) + auto phiLocal = nextIndex++; + addWritesToLocal(mappings, i, phiLocal); + // we can leave |merged| alone here - we basically just + // pick one to represent it, it doesn't matter which break; } } @@ -227,26 +248,26 @@ struct SSAify : public PostWalker { return out; } - void createPhi(std::vector& infos, Index old, Index new_) { + void addWritesToLocal(std::vector& mappings, Index old, Index new_) { // assign the set value to the phi value as well Builder builder(*getModule()); - for (auto& info : infos) { - if (!info[index]) { + for (auto& mapping : mappings) { + if (!mapping[index]) { // this is a param or the zero init value. add a set first thing in // the function auto* block = builder.blockify(getFunction()->body); getFunction()->body = block; auto* set = builder.makeSetLocal( - old_, + old, builder.makeGetLocal(new_, getFunction()->getLocalType(old)) ); - info[old] = set; + mapping[old] = set; ExpressionManipulator::spliceIntoBlock(block, 0, set); } // now a set exists, just add a tee of its value, so we also set the phi merge var - info[old]->value = builder.makeTeeLocal( + mapping[old]->value = builder.makeTeeLocal( new_, - info[old]->value + mapping[old]->value ); } } From 0b217e76fd95e479fb71611aec329295a42fab45 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 24 May 2017 13:59:38 -0700 Subject: [PATCH 05/44] buid --- src/passes/SSAify.cpp | 84 +++++++++++++++++++++---------------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 5d8f45e9a60..2152e7927b2 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -36,7 +36,7 @@ namespace wasm { // Tracks assignments to locals, assuming single-assignment form, i.e., // each assignment creates a new variable. -struct SSAify : public PostWalker { +struct SSAify : public WalkerPass> { // we map (old local index) => the set_local for that index. The new // index for the local can be seen in that set local. // this can be nullptr if there is no set_local, and instead the value @@ -47,7 +47,7 @@ struct SSAify : public PostWalker { Mapping currMapping; Index nextIndex; std::vector mappingStack; // used in ifs, loops - std::map> breakInfos; // break target => infos that reach it + std::map> breakMappings; // break target => infos that reach it std::vector> loopGetStack; // stack of loops, all the gets in each, so we can update them for phis std::map originalGetIndexes; @@ -59,23 +59,23 @@ struct SSAify : public PostWalker { currMapping.resize(numLocals); for (auto& set : currMapping) set = nullptr; nextIndex = numLocals; - PostWalker::walk(func->body); + WalkerPass>::walk(func->body); } // control flow static void doVisitBlock(SSAify* self, Expression** currp) { auto* curr = (*currp)->cast(); - if (curr->name.is() && self->breakInfos.find(curr->name) != self->breakInfos.end()) { - auto& infos = self->breakInfos[curr->name]; - infos.emplace_back(std::move(self->currMapping), currp, BreakInfo::Internal); - self->currMapping = std::move(self->merge(infos), curr->name); + if (curr->name.is() && self->breakMappings.find(curr->name) != self->breakMappings.end()) { + auto& infos = self->breakMappings[curr->name]; + infos.emplace_back(std::move(self->currMapping)); + self->currMapping = std::move(self->merge(infos)); } } void finishIf() { // that's it for this if, merge - std::vector breaks; + std::vector breaks; breaks.emplace_back(std::move(currMapping)); breaks.emplace_back(std::move(mappingStack.back())); mappingStack.pop_back(); @@ -83,13 +83,12 @@ struct SSAify : public PostWalker { } static void afterIfCondition(SSAify* self, Expression** currp) { - auto* curr = (*currp)->cast(); self->mappingStack.push_back(self->currMapping); } static void afterIfTrue(SSAify* self, Expression** currp) { auto* curr = (*currp)->cast(); if (curr->ifFalse) { - auto afterCondition = std::move(self.mappingStack.back()); + auto afterCondition = std::move(self->mappingStack.back()); self->mappingStack.back() = std::move(self->currMapping); self->currMapping = std::move(afterCondition); } else { @@ -106,8 +105,8 @@ struct SSAify : public PostWalker { } static void doVisitLoop(SSAify* self, Expression** currp) { auto* curr = (*currp)->cast(); - if (curr->name.is() && self->breakInfos.find(curr->name) != self->breakInfos.end()) { - auto& infos = self->breakInfos[curr->name]; + if (curr->name.is() && self->breakMappings.find(curr->name) != self->breakMappings.end()) { + auto& infos = self->breakMappings[curr->name]; infos.emplace_back(std::move(self->mappingStack.back())); auto before = infos.back(); auto& merged = self->merge(infos); @@ -115,31 +114,28 @@ struct SSAify : public PostWalker { // the loop auto& gets = self->loopGetStack.back(); for (auto* get : gets) { - auto original = self->originalGetIndexes[get]; - get->index = merged[original]; + self->updateGetLocalIndex(get, merged); } } self->mappingStack.pop_back(); self->loopGetStack.pop_back(); } - static void visitBreak(SSAify* self, Expression** currp) { - auto* curr = (*currp)->cast(); - self->breakInfos[curr->name].emplace_back(std::move(self->currMapping), currp, BreakInfo::Internal); - if (!(*currp)->cast()->condition) { - setUnreachable(self->currMapping); + void visitBreak(Break* curr) { + breakMappings[curr->name].emplace_back(std::move(currMapping)); + if (!curr->condition) { + setUnreachable(currMapping); } } - static void visitSwitch(SSAify* self, Expression** currp) { - auto* curr = (*currp)->cast(); + void visitSwitch(Switch* curr) { std::set all; for (auto target : curr->targets) { all.insert(target); } all.insert(curr->default_); for (auto target : all) { - self->breakInfos[curr->default_].emplace_back(self->currMapping, currp, BreakInfo::Switch); + breakMappings[target].emplace_back(currMapping); } - setUnreachable(self->currMapping); + setUnreachable(currMapping); } void visitReturn(Return *curr) { setUnreachable(currMapping); @@ -150,12 +146,8 @@ struct SSAify : public PostWalker { // local usage - void visitGetLocal(GetLocal* curr) { - for (auto& loopGets : loopGetStack) { - loopGets.push_back(curr); - } - originalGetIndexes[curr] = curr->index; - auto* set = currMapping[curr->index]; + void updateGetLocalIndex(GetLocal* curr, Mapping& mapping) { + auto* set = mapping[curr->index]; if (set) { curr->index = set->index; } else { @@ -164,10 +156,18 @@ struct SSAify : public PostWalker { // nothing to do, keep getting that param } else { // just replace with zero - replaceCurrent(LiteralUtils::makeZero(curr->type, *getModule()); + replaceCurrent(LiteralUtils::makeZero(curr->type, *getModule())); } } } + + void visitGetLocal(GetLocal* curr) { + for (auto& loopGets : loopGetStack) { + loopGets.push_back(curr); + } + originalGetIndexes[curr] = curr->index; + updateGetLocalIndex(curr, currMapping); + } void visitSetLocal(SetLocal* curr) { currMapping[curr->index] = curr; curr->index = nextIndex++; @@ -180,14 +180,14 @@ struct SSAify : public PostWalker { // if needs special handling if (iff->ifFalse) { self->pushTask(SSAify::afterIfFalse, currp); - self->pushTask(SSAify::scan, iff->ifFalse); + self->pushTask(SSAify::scan, &iff->ifFalse); } self->pushTask(SSAify::afterIfTrue, currp); - self->pushTask(SSAify::scan, iff->ifTrue); + self->pushTask(SSAify::scan, &iff->ifTrue); self->pushTask(SSAify::afterIfCondition, currp); - self->pushTask(SSAify::scan, iff->condition); + self->pushTask(SSAify::scan, &iff->condition); } else { - PostWalker::scan(self, currp); + WalkerPass>::scan(self, currp); } // loops need pre-order visiting too @@ -198,13 +198,15 @@ struct SSAify : public PostWalker { // helpers + static SetLocal IMPOSSIBLE_SET; + void setUnreachable(Mapping& mapping) { mapping.resize(numLocals); // may have been emptied by a move - mapping[0] = Index(-1); + mapping[0] = &IMPOSSIBLE_SET; } bool isUnreachable(Mapping& mapping) { - return mapping[0] == Index(-1); + return mapping[0] == &IMPOSSIBLE_SET; } // merges a bunch of infos into one. @@ -217,13 +219,11 @@ struct SSAify : public PostWalker { return mappings[0]; } for (Index i = 0; i < numLocals; i++) { - SetLocal* merged; - bool seen = false; + SetLocal* merged = &IMPOSSIBLE_SET; for (auto& mapping : mappings) { if (isUnreachable(mapping)) continue; - if (!seen) { + if (merged == &IMPOSSIBLE_SET) { merged = mapping[i]; - seen = true; } else { if (mapping[i] != merged) { // we need a phi for this local, e.g., imagine @@ -241,7 +241,7 @@ struct SSAify : public PostWalker { } } } - if (seen) { + if (merged != &IMPOSSIBLE_SET) { out[i] = merged; } } @@ -252,7 +252,7 @@ struct SSAify : public PostWalker { // assign the set value to the phi value as well Builder builder(*getModule()); for (auto& mapping : mappings) { - if (!mapping[index]) { + if (!mapping[old]) { // this is a param or the zero init value. add a set first thing in // the function auto* block = builder.blockify(getFunction()->body); From 91dc4a7fd51974f61bdeb5ff1581f137a835e402 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 24 May 2017 14:18:42 -0700 Subject: [PATCH 06/44] start test --- src/passes/SSAify.cpp | 5 +++-- test/passes/ssa.txt | 13 +++++++++++++ test/passes/ssa.wast | 8 ++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 test/passes/ssa.txt create mode 100644 test/passes/ssa.wast diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 2152e7927b2..e9cadaea7b3 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -33,6 +33,9 @@ namespace wasm { +// A set we know is impossible / not in the ast +SetLocal IMPOSSIBLE_SET; + // Tracks assignments to locals, assuming single-assignment form, i.e., // each assignment creates a new variable. @@ -198,8 +201,6 @@ struct SSAify : public WalkerPass> { // helpers - static SetLocal IMPOSSIBLE_SET; - void setUnreachable(Mapping& mapping) { mapping.resize(numLocals); // may have been emptied by a move mapping[0] = &IMPOSSIBLE_SET; diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt new file mode 100644 index 00000000000..dba6cd3c5d0 --- /dev/null +++ b/test/passes/ssa.txt @@ -0,0 +1,13 @@ +(module + (type $0 (func (param i32))) + (memory $0 0) + (func $basics (type $0) (param $x i32) + (local $y i32) + (drop + (get_local $x) + ) + (drop + (i32.const 0) + ) + ) +) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast new file mode 100644 index 00000000000..5e7b50a2c78 --- /dev/null +++ b/test/passes/ssa.wast @@ -0,0 +1,8 @@ +(module + (func $basics (param $x i32) + (local $y i32) + (drop (get_local $x)) + (drop (get_local $y)) + ) +) + From 9d9dd7c8753ce3c435e156049e0707340fc07131 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 24 May 2017 14:20:02 -0700 Subject: [PATCH 07/44] more --- test/passes/ssa.txt | 12 ++++++++++++ test/passes/ssa.wast | 10 ++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index dba6cd3c5d0..69e9dd7b1e6 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -3,11 +3,23 @@ (memory $0 0) (func $basics (type $0) (param $x i32) (local $y i32) + (local $z f32) + (local $w i64) + (local $t f64) (drop (get_local $x) ) (drop (i32.const 0) ) + (drop + (f32.const 0) + ) + (drop + (i64.const 0) + ) + (drop + (f64.const 0) + ) ) ) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index 5e7b50a2c78..803fdab4572 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -1,8 +1,14 @@ (module (func $basics (param $x i32) (local $y i32) - (drop (get_local $x)) - (drop (get_local $y)) + (local $z f32) + (local $w i64) + (local $t f64) + (drop (get_local $x)) ;; keep as param get + (drop (get_local $y)) ;; turn into get of 0-init + (drop (get_local $z)) + (drop (get_local $w)) + (drop (get_local $t)) ) ) From 84e6dbb269455bc0f1c5a60f2e93f8ec46dd861b Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 24 May 2017 14:25:13 -0700 Subject: [PATCH 08/44] more --- src/passes/SSAify.cpp | 10 +++++++--- test/passes/ssa.txt | 14 ++++++++++++++ test/passes/ssa.wast | 4 ++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index e9cadaea7b3..78d443de0a5 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -173,7 +173,7 @@ struct SSAify : public WalkerPass> { } void visitSetLocal(SetLocal* curr) { currMapping[curr->index] = curr; - curr->index = nextIndex++; + curr->index = addLocal(getFunction()->getLocalType(curr->index)); } // traversal @@ -234,8 +234,8 @@ struct SSAify : public WalkerPass> { // (not that y seems as "single-assignment" as x here, but the point is // that x may be merged multiple times, so we need a different phi merge // local for each) - auto phiLocal = nextIndex++; - addWritesToLocal(mappings, i, phiLocal); + auto phiLocal = addLocal(getFunction()->getLocalType(i)); +; addWritesToLocal(mappings, i, phiLocal); // we can leave |merged| alone here - we basically just // pick one to represent it, it doesn't matter which break; @@ -272,6 +272,10 @@ struct SSAify : public WalkerPass> { ); } } + + Index addLocal(WasmType type) { + return Builder::addVar(getFunction(), type); + } }; Pass *createSSAifyPass() { diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 69e9dd7b1e6..c9fccf34389 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -6,6 +6,8 @@ (local $z f32) (local $w i64) (local $t f64) + (local $5 i32) + (local $6 f64) (drop (get_local $x) ) @@ -21,5 +23,17 @@ (drop (f64.const 0) ) + (set_local $5 + (i32.const 100) + ) + (drop + (get_local $5) + ) + (set_local $6 + (f64.const 2) + ) + (drop + (get_local $6) + ) ) ) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index 803fdab4572..0cd174e4717 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -9,6 +9,10 @@ (drop (get_local $z)) (drop (get_local $w)) (drop (get_local $t)) + (set_local $x (i32.const 100)) ;; overwrite param + (drop (get_local $x)) ;; no longer a param! + (set_local $t (f64.const 2)) ;; overwrite local + (drop (get_local $t)) ) ) From 64346e972e522b47029ecc96fc9827450054f527 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 24 May 2017 14:31:00 -0700 Subject: [PATCH 09/44] more --- test/passes/ssa.txt | 10 ++++++++++ test/passes/ssa.wast | 3 +++ 2 files changed, 13 insertions(+) diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index c9fccf34389..96d8721eefe 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -8,6 +8,7 @@ (local $t f64) (local $5 i32) (local $6 f64) + (local $7 f64) (drop (get_local $x) ) @@ -35,5 +36,14 @@ (drop (get_local $6) ) + (set_local $7 + (f64.const 33) + ) + (drop + (get_local $7) + ) + (drop + (get_local $7) + ) ) ) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index 0cd174e4717..f99d5488c41 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -13,6 +13,9 @@ (drop (get_local $x)) ;; no longer a param! (set_local $t (f64.const 2)) ;; overwrite local (drop (get_local $t)) + (set_local $t (f64.const 33)) ;; overwrite local AGAIN + (drop (get_local $t)) + (drop (get_local $t)) ;; use twice ) ) From 65039dc30661914143454e0a46b4909bef87eb93 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 24 May 2017 14:53:49 -0700 Subject: [PATCH 10/44] debug --- src/passes/SSAify.cpp | 8 ++++++++ test/passes/ssa.txt | 32 ++++++++++++++++++++++++++++++++ test/passes/ssa.wast | 16 ++++++++++++++++ 3 files changed, 56 insertions(+) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 78d443de0a5..6d470e9b735 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -31,6 +31,8 @@ #include "ast/manipulation.h" #include "ast/literal-utils.h" +#include "wasm-printing.h" + namespace wasm { // A set we know is impossible / not in the ast @@ -56,6 +58,7 @@ struct SSAify : public WalkerPass> { void doWalkFunction(Function* func) { numLocals = func->getNumLocals(); +std::cerr << "num locals " << numLocals << '\n'; if (numLocals == 0) return; // nothing to do // We begin with each param being assigned from the incoming value, and the zero-init for the locals, // so the initial state is the identity permutation @@ -165,6 +168,8 @@ struct SSAify : public WalkerPass> { } void visitGetLocal(GetLocal* curr) { + assert(currMapping.size() == numLocals); + assert(curr->index < numLocals); for (auto& loopGets : loopGetStack) { loopGets.push_back(curr); } @@ -172,6 +177,9 @@ struct SSAify : public WalkerPass> { updateGetLocalIndex(curr, currMapping); } void visitSetLocal(SetLocal* curr) { +std::cerr << "visit set local " << curr->index << " : " << numLocals << '\n' << getFunction()->body << '\n' << curr << '\n'; + assert(currMapping.size() == numLocals); + assert(curr->index < numLocals); currMapping[curr->index] = curr; curr->index = addLocal(getFunction()->getLocalType(curr->index)); } diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 96d8721eefe..2c9460e555c 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -1,5 +1,6 @@ (module (type $0 (func (param i32))) + (type $1 (func)) (memory $0 0) (func $basics (type $0) (param $x i32) (local $y i32) @@ -46,4 +47,35 @@ (get_local $7) ) ) + (func $if (type $1) + (local $x i32) + (local $y i32) + (local $2 i32) + (local $3 i32) + (local $4 i32) + (local $5 i32) + (set_local $x + (tee_local $3 + (get_local $3) + ) + ) + (drop + (if i32 + (i32.const 1) + (i32.const 0) + (i32.const 0) + ) + ) + (if + (i32.const 1) + (set_local $5 + (tee_local $4 + (i32.const 1) + ) + ) + ) + (drop + (get_local $x) + ) + ) ) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index f99d5488c41..79a979a6470 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -17,5 +17,21 @@ (drop (get_local $t)) (drop (get_local $t)) ;; use twice ) + (func $if + (local $x i32) + (local $y i32) + (drop + (if i32 + (i32.const 1) + (get_local $x) + (get_local $y) + ) + ) + (if + (i32.const 1) + (set_local $x (i32.const 1)) + ) + (drop (get_local $x)) ;; x merged to here + ) ) From 0a80c1a91ce21208e19177dfd6aabe6fe3d59fe0 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 24 May 2017 15:16:25 -0700 Subject: [PATCH 11/44] fix --- src/passes/SSAify.cpp | 33 +++++++++++++++++++-------------- test/passes/ssa.txt | 10 ++++------ 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 6d470e9b735..888eb9f8633 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -31,8 +31,6 @@ #include "ast/manipulation.h" #include "ast/literal-utils.h" -#include "wasm-printing.h" - namespace wasm { // A set we know is impossible / not in the ast @@ -55,10 +53,10 @@ struct SSAify : public WalkerPass> { std::map> breakMappings; // break target => infos that reach it std::vector> loopGetStack; // stack of loops, all the gets in each, so we can update them for phis std::map originalGetIndexes; + std::vector functionPrepends; // things we add to the function prologue void doWalkFunction(Function* func) { numLocals = func->getNumLocals(); -std::cerr << "num locals " << numLocals << '\n'; if (numLocals == 0) return; // nothing to do // We begin with each param being assigned from the incoming value, and the zero-init for the locals, // so the initial state is the identity permutation @@ -66,6 +64,15 @@ std::cerr << "num locals " << numLocals << '\n'; for (auto& set : currMapping) set = nullptr; nextIndex = numLocals; WalkerPass>::walk(func->body); + if (functionPrepends.size() > 0) { + Builder builder(*getModule()); + auto* block = builder.blockify(func->body); + func->body = block; + // TODO: this is O(toplevel block size^2) + for (auto* pre : functionPrepends) { + ExpressionManipulator::spliceIntoBlock(block, 0, pre); + } + } } // control flow @@ -177,7 +184,6 @@ std::cerr << "num locals " << numLocals << '\n'; updateGetLocalIndex(curr, currMapping); } void visitSetLocal(SetLocal* curr) { -std::cerr << "visit set local " << curr->index << " : " << numLocals << '\n' << getFunction()->body << '\n' << curr << '\n'; assert(currMapping.size() == numLocals); assert(curr->index < numLocals); currMapping[curr->index] = curr; @@ -243,9 +249,7 @@ std::cerr << "visit set local " << curr->index << " : " << numLocals << '\n' << // that x may be merged multiple times, so we need a different phi merge // local for each) auto phiLocal = addLocal(getFunction()->getLocalType(i)); -; addWritesToLocal(mappings, i, phiLocal); - // we can leave |merged| alone here - we basically just - // pick one to represent it, it doesn't matter which +; merged = addWritesToLocal(mappings, i, phiLocal); break; } } @@ -257,28 +261,29 @@ std::cerr << "visit set local " << curr->index << " : " << numLocals << '\n' << return out; } - void addWritesToLocal(std::vector& mappings, Index old, Index new_) { - // assign the set value to the phi value as well + // adds phi-style writes to sets of a local + // returns one of those writes + SetLocal* addWritesToLocal(std::vector& mappings, Index old, Index new_) { + SetLocal* ret = nullptr; Builder builder(*getModule()); for (auto& mapping : mappings) { if (!mapping[old]) { // this is a param or the zero init value. add a set first thing in // the function - auto* block = builder.blockify(getFunction()->body); - getFunction()->body = block; auto* set = builder.makeSetLocal( old, - builder.makeGetLocal(new_, getFunction()->getLocalType(old)) + builder.makeGetLocal(old, getFunction()->getLocalType(old)) ); mapping[old] = set; - ExpressionManipulator::spliceIntoBlock(block, 0, set); + functionPrepends.push_back(set); } // now a set exists, just add a tee of its value, so we also set the phi merge var - mapping[old]->value = builder.makeTeeLocal( + mapping[old]->value = ret = builder.makeTeeLocal( new_, mapping[old]->value ); } + return ret; } Index addLocal(WasmType type) { diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 2c9460e555c..8064a4ca6f9 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -52,11 +52,9 @@ (local $y i32) (local $2 i32) (local $3 i32) - (local $4 i32) - (local $5 i32) (set_local $x (tee_local $3 - (get_local $3) + (get_local $x) ) ) (drop @@ -68,14 +66,14 @@ ) (if (i32.const 1) - (set_local $5 - (tee_local $4 + (set_local $2 + (tee_local $3 (i32.const 1) ) ) ) (drop - (get_local $x) + (get_local $3) ) ) ) From 97e16e28d010f13de6beee5991511fe8200fcf10 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 24 May 2017 15:20:25 -0700 Subject: [PATCH 12/44] zero out preinits of locals --- src/passes/SSAify.cpp | 3 ++- test/passes/ssa.txt | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 888eb9f8633..15390f5385e 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -272,7 +272,8 @@ struct SSAify : public WalkerPass> { // the function auto* set = builder.makeSetLocal( old, - builder.makeGetLocal(old, getFunction()->getLocalType(old)) + getFunction()->isParam(old) ? builder.makeGetLocal(old, getFunction()->getLocalType(old)) + : LiteralUtils::makeZero(getFunction()->getLocalType(old), *getModule()) ); mapping[old] = set; functionPrepends.push_back(set); diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 8064a4ca6f9..5c6af0b8d85 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -54,7 +54,7 @@ (local $3 i32) (set_local $x (tee_local $3 - (get_local $x) + (i32.const 0) ) ) (drop From 9de1e59c751c820eb4e06d3bd509ee88b841cc0d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 24 May 2017 15:22:14 -0700 Subject: [PATCH 13/44] test --- test/passes/ssa.txt | 31 ++++++++++++++++++++++++------- test/passes/ssa.wast | 10 ++++++++-- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 5c6af0b8d85..113c3b791ec 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -1,6 +1,5 @@ (module (type $0 (func (param i32))) - (type $1 (func)) (memory $0 0) (func $basics (type $0) (param $x i32) (local $y i32) @@ -47,13 +46,20 @@ (get_local $7) ) ) - (func $if (type $1) + (func $if (type $0) (param $p i32) (local $x i32) (local $y i32) - (local $2 i32) (local $3 i32) + (local $4 i32) + (local $5 i32) + (local $6 i32) + (set_local $p + (tee_local $6 + (get_local $p) + ) + ) (set_local $x - (tee_local $3 + (tee_local $4 (i32.const 0) ) ) @@ -66,14 +72,25 @@ ) (if (i32.const 1) - (set_local $2 - (tee_local $3 + (set_local $3 + (tee_local $4 + (i32.const 1) + ) + ) + ) + (drop + (get_local $4) + ) + (if + (i32.const 1) + (set_local $5 + (tee_local $6 (i32.const 1) ) ) ) (drop - (get_local $3) + (get_local $6) ) ) ) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index 79a979a6470..dd8bb1f0bb7 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -17,7 +17,7 @@ (drop (get_local $t)) (drop (get_local $t)) ;; use twice ) - (func $if + (func $if (param $p i32) (local $x i32) (local $y i32) (drop @@ -31,7 +31,13 @@ (i32.const 1) (set_local $x (i32.const 1)) ) - (drop (get_local $x)) ;; x merged to here + (drop (get_local $x)) + ;; same but with param + (if + (i32.const 1) + (set_local $p (i32.const 1)) + ) + (drop (get_local $p)) ) ) From eb3d9c8d615bee3a27fd0e9e2ce7eb8f98ac8543 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 24 May 2017 15:26:24 -0700 Subject: [PATCH 14/44] test --- test/passes/ssa.txt | 78 +++++++++++++++++++++++++++++++++++++++++++- test/passes/ssa.wast | 28 ++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 113c3b791ec..deb366e7a74 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -53,6 +53,17 @@ (local $4 i32) (local $5 i32) (local $6 i32) + (local $7 i32) + (local $8 i32) + (local $9 i32) + (local $10 i32) + (local $11 i32) + (local $12 i32) + (local $13 i32) + (local $14 i32) + (local $15 i32) + (local $16 i32) + (local $17 i32) (set_local $p (tee_local $6 (get_local $p) @@ -60,7 +71,9 @@ ) (set_local $x (tee_local $4 - (i32.const 0) + (tee_local $8 + (i32.const 0) + ) ) ) (drop @@ -92,5 +105,68 @@ (drop (get_local $6) ) + (if + (i32.const 1) + (set_local $7 + (tee_local $8 + (tee_local $10 + (i32.const 2) + ) + ) + ) + (nop) + ) + (drop + (get_local $8) + ) + (if + (i32.const 1) + (nop) + (set_local $9 + (tee_local $10 + (i32.const 3) + ) + ) + ) + (drop + (get_local $10) + ) + (if + (i32.const 1) + (set_local $11 + (tee_local $13 + (i32.const 4) + ) + ) + (set_local $12 + (tee_local $13 + (i32.const 5) + ) + ) + ) + (drop + (get_local $13) + ) + (if + (i32.const 1) + (set_local $14 + (tee_local $17 + (i32.const 6) + ) + ) + (block $block + (set_local $15 + (i32.const 7) + ) + (set_local $16 + (tee_local $17 + (i32.const 8) + ) + ) + ) + ) + (drop + (get_local $17) + ) ) ) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index dd8bb1f0bb7..6313fe6d8b2 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -38,6 +38,34 @@ (set_local $p (i32.const 1)) ) (drop (get_local $p)) + ;; if-else + (if + (i32.const 1) + (set_local $x (i32.const 2)) + (nop) + ) + (drop (get_local $x)) + (if + (i32.const 1) + (nop) + (set_local $x (i32.const 3)) + ) + (drop (get_local $x)) + (if + (i32.const 1) + (set_local $x (i32.const 4)) + (set_local $x (i32.const 5)) + ) + (drop (get_local $x)) + (if + (i32.const 1) + (set_local $x (i32.const 6)) + (block + (set_local $x (i32.const 7)) + (set_local $x (i32.const 8)) + ) + ) + (drop (get_local $x)) ) ) From 829065d012705d444267108546c1430017165b64 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 24 May 2017 15:32:46 -0700 Subject: [PATCH 15/44] test --- src/passes/SSAify.cpp | 4 ++++ test/passes/ssa.txt | 25 +++++++++++++++++++++++++ test/passes/ssa.wast | 10 ++++++++++ 3 files changed, 39 insertions(+) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 15390f5385e..8ce37821d83 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -40,6 +40,10 @@ SetLocal IMPOSSIBLE_SET; // each assignment creates a new variable. struct SSAify : public WalkerPass> { + bool isFunctionParallel() override { return true; } + + Pass* create() override { return new SSAify; } + // we map (old local index) => the set_local for that index. The new // index for the local can be seen in that set local. // this can be nullptr if there is no set_local, and instead the value diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index deb366e7a74..f998e89eee2 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -169,4 +169,29 @@ (get_local $17) ) ) + (func $if2 (type $0) (param $x i32) + (local $1 i32) + (local $2 i32) + (set_local $x + (tee_local $2 + (get_local $x) + ) + ) + (if + (i32.const 1) + (block $block + (set_local $1 + (tee_local $2 + (i32.const 1) + ) + ) + (drop + (get_local $1) + ) + ) + ) + (drop + (get_local $2) + ) + ) ) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index 6313fe6d8b2..b9cf714488e 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -67,5 +67,15 @@ ) (drop (get_local $x)) ) + (func $if2 (param $x i32) + (if + (i32.const 1) + (block + (set_local $x (i32.const 1)) + (drop (get_local $x)) ;; use between phi set and use + ) + ) + (drop (get_local $x)) + ) ) From 559385883c7caa7eeeaf9d13c6556d7f066d8492 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 24 May 2017 15:56:55 -0700 Subject: [PATCH 16/44] test --- test/passes/ssa.wast | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index b9cf714488e..8a17413f6ee 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -77,5 +77,38 @@ ) (drop (get_local $x)) ) + (func $block (param $x i32) + (block $out + (set_local $x (i32.const 1)) + (br_if $out (i32.const 2)) + (if (i32.const 3) + (block + (set_local $x (i32.const 1)) + (br $out) + ) + ) + (set_local $x (i32.const 4)) + (if (i32.const 5) + (br $out) + ) + (if (i32.const 6) + (nop) + ) + (if (i32.const 7) + (nop) + (nop) + ) + ;; finally, switching + (block $in + (set_local $x (i32.const 8)) + (br_table $in $out (i32.const 9)) + ) + (block $in2 + (set_local $x (i32.const 10)) + (br_table $out $in2 (i32.const 11)) + ) + ) + (drop (get_local $x)) + ) ) From 8292fe1336c19d5d04ea99025c431218f8ee0fe0 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Thu, 25 May 2017 11:34:39 -0700 Subject: [PATCH 17/44] more --- src/passes/SSAify.cpp | 17 +++++++++-------- test/passes/ssa.txt | 22 ++++++++++++++++++++++ test/passes/ssa.wast | 28 +--------------------------- 3 files changed, 32 insertions(+), 35 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 8ce37821d83..edbcbd59175 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -81,12 +81,11 @@ struct SSAify : public WalkerPass> { // control flow - static void doVisitBlock(SSAify* self, Expression** currp) { - auto* curr = (*currp)->cast(); - if (curr->name.is() && self->breakMappings.find(curr->name) != self->breakMappings.end()) { - auto& infos = self->breakMappings[curr->name]; - infos.emplace_back(std::move(self->currMapping)); - self->currMapping = std::move(self->merge(infos)); + void visitBlock(Block* curr) { + if (curr->name.is() && breakMappings.find(curr->name) != breakMappings.end()) { + auto& infos = breakMappings[curr->name]; + infos.emplace_back(std::move(currMapping)); + currMapping = std::move(merge(infos)); } } @@ -138,8 +137,10 @@ struct SSAify : public WalkerPass> { self->loopGetStack.pop_back(); } void visitBreak(Break* curr) { - breakMappings[curr->name].emplace_back(std::move(currMapping)); - if (!curr->condition) { + if (curr->condition) { + breakMappings[curr->name].emplace_back(currMapping); + } else { + breakMappings[curr->name].emplace_back(std::move(currMapping)); setUnreachable(currMapping); } } diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index f998e89eee2..102688fd7cd 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -194,4 +194,26 @@ (get_local $2) ) ) + (func $block (type $0) (param $x i32) + (local $1 i32) + (local $2 i32) + (set_local $x + (tee_local $2 + (get_local $x) + ) + ) + (block $out + (br_if $out + (i32.const 2) + ) + (set_local $1 + (tee_local $2 + (i32.const 1) + ) + ) + ) + (drop + (get_local $2) + ) + ) ) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index 8a17413f6ee..365ce254034 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -79,34 +79,8 @@ ) (func $block (param $x i32) (block $out - (set_local $x (i32.const 1)) (br_if $out (i32.const 2)) - (if (i32.const 3) - (block - (set_local $x (i32.const 1)) - (br $out) - ) - ) - (set_local $x (i32.const 4)) - (if (i32.const 5) - (br $out) - ) - (if (i32.const 6) - (nop) - ) - (if (i32.const 7) - (nop) - (nop) - ) - ;; finally, switching - (block $in - (set_local $x (i32.const 8)) - (br_table $in $out (i32.const 9)) - ) - (block $in2 - (set_local $x (i32.const 10)) - (br_table $out $in2 (i32.const 11)) - ) + (set_local $x (i32.const 1)) ) (drop (get_local $x)) ) From 8789b3f5250408d7f13e95f4358cb2b80d41cedc Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Thu, 25 May 2017 11:43:50 -0700 Subject: [PATCH 18/44] test --- test/passes/ssa.txt | 72 ++++++++++++++++++++++++++++++++++++++++++++ test/passes/ssa.wast | 33 ++++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 102688fd7cd..75af68da81e 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -216,4 +216,76 @@ (get_local $2) ) ) + (func $block2 (type $0) (param $x i32) + (local $1 i32) + (local $2 i32) + (local $3 i32) + (local $4 i32) + (local $5 i32) + (local $6 i32) + (block $out + (set_local $1 + (tee_local $6 + (i32.const 1) + ) + ) + (br_if $out + (i32.const 2) + ) + (if + (i32.const 3) + (block $block + (set_local $2 + (tee_local $6 + (i32.const 1) + ) + ) + (br $out) + ) + ) + (set_local $3 + (tee_local $6 + (i32.const 4) + ) + ) + (if + (i32.const 5) + (br $out) + ) + (if + (i32.const 6) + (nop) + ) + (if + (i32.const 7) + (nop) + (nop) + ) + (block $in + (set_local $4 + (tee_local $6 + (i32.const 8) + ) + ) + (br_table $in $out + (i32.const 9) + ) + ) + (block $in2 + (set_local $5 + (tee_local $6 + (tee_local $6 + (i32.const 10) + ) + ) + ) + (br_table $out $in2 + (i32.const 11) + ) + ) + ) + (drop + (get_local $6) + ) + ) ) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index 365ce254034..cdc5305fa1f 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -84,5 +84,38 @@ ) (drop (get_local $x)) ) + (func $block2 (param $x i32) + (block $out + (set_local $x (i32.const 1)) + (br_if $out (i32.const 2)) + (if (i32.const 3) + (block + (set_local $x (i32.const 1)) + (br $out) + ) + ) + (set_local $x (i32.const 4)) + (if (i32.const 5) + (br $out) + ) + (if (i32.const 6) + (nop) + ) + (if (i32.const 7) + (nop) + (nop) + ) + ;; finally, switching + (block $in + (set_local $x (i32.const 8)) + (br_table $in $out (i32.const 9)) + ) + (block $in2 + (set_local $x (i32.const 10)) + (br_table $out $in2 (i32.const 11)) + ) + ) + (drop (get_local $x)) + ) ) From b684a3dbbd4ffa28abf1f1e604cf7755e368cd49 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Thu, 25 May 2017 11:57:02 -0700 Subject: [PATCH 19/44] fix --- src/passes/SSAify.cpp | 15 +++++++++++---- test/passes/ssa.txt | 4 +--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index edbcbd59175..0c358fe059f 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -271,11 +271,18 @@ struct SSAify : public WalkerPass> { SetLocal* addWritesToLocal(std::vector& mappings, Index old, Index new_) { SetLocal* ret = nullptr; Builder builder(*getModule()); + // the same set may appear in multiple mappings; we just need one for each + std::set seen; for (auto& mapping : mappings) { - if (!mapping[old]) { + auto* set = mapping[old]; + if (!seen.insert(set).second) { + // seen it already + continue; + } + if (!set) { // this is a param or the zero init value. add a set first thing in // the function - auto* set = builder.makeSetLocal( + set = builder.makeSetLocal( old, getFunction()->isParam(old) ? builder.makeGetLocal(old, getFunction()->getLocalType(old)) : LiteralUtils::makeZero(getFunction()->getLocalType(old), *getModule()) @@ -284,9 +291,9 @@ struct SSAify : public WalkerPass> { functionPrepends.push_back(set); } // now a set exists, just add a tee of its value, so we also set the phi merge var - mapping[old]->value = ret = builder.makeTeeLocal( + set->value = ret = builder.makeTeeLocal( new_, - mapping[old]->value + set->value ); } return ret; diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 75af68da81e..34f9e43a792 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -274,9 +274,7 @@ (block $in2 (set_local $5 (tee_local $6 - (tee_local $6 - (i32.const 10) - ) + (i32.const 10) ) ) (br_table $out $in2 From c7019326d5aeb72631349a249dbeba01bedbabdb Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Thu, 25 May 2017 12:00:58 -0700 Subject: [PATCH 20/44] more --- test/passes/ssa.txt | 30 ++++++++++++++++++++++++++++++ test/passes/ssa.wast | 10 ++++++++++ 2 files changed, 40 insertions(+) diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 34f9e43a792..010fa4971a5 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -229,9 +229,15 @@ (i32.const 1) ) ) + (drop + (get_local $1) + ) (br_if $out (i32.const 2) ) + (drop + (get_local $1) + ) (if (i32.const 3) (block $block @@ -240,18 +246,30 @@ (i32.const 1) ) ) + (drop + (get_local $2) + ) (br $out) ) ) + (drop + (get_local $1) + ) (set_local $3 (tee_local $6 (i32.const 4) ) ) + (drop + (get_local $3) + ) (if (i32.const 5) (br $out) ) + (drop + (get_local $3) + ) (if (i32.const 6) (nop) @@ -267,20 +285,32 @@ (i32.const 8) ) ) + (drop + (get_local $4) + ) (br_table $in $out (i32.const 9) ) ) + (drop + (get_local $4) + ) (block $in2 (set_local $5 (tee_local $6 (i32.const 10) ) ) + (drop + (get_local $5) + ) (br_table $out $in2 (i32.const 11) ) ) + (drop + (get_local $5) + ) ) (drop (get_local $6) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index cdc5305fa1f..6ee1fcd9e1a 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -87,17 +87,23 @@ (func $block2 (param $x i32) (block $out (set_local $x (i32.const 1)) + (drop (get_local $x)) (br_if $out (i32.const 2)) + (drop (get_local $x)) (if (i32.const 3) (block (set_local $x (i32.const 1)) + (drop (get_local $x)) (br $out) ) ) + (drop (get_local $x)) (set_local $x (i32.const 4)) + (drop (get_local $x)) (if (i32.const 5) (br $out) ) + (drop (get_local $x)) (if (i32.const 6) (nop) ) @@ -108,12 +114,16 @@ ;; finally, switching (block $in (set_local $x (i32.const 8)) + (drop (get_local $x)) (br_table $in $out (i32.const 9)) ) + (drop (get_local $x)) (block $in2 (set_local $x (i32.const 10)) + (drop (get_local $x)) (br_table $out $in2 (i32.const 11)) ) + (drop (get_local $x)) ) (drop (get_local $x)) ) From f83b830658f8f2dde88b976c153807b6ebcaff30 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Thu, 25 May 2017 12:09:05 -0700 Subject: [PATCH 21/44] loop --- test/passes/ssa.txt | 28 ++++++++++++++++++++++++++++ test/passes/ssa.wast | 9 +++++++++ 2 files changed, 37 insertions(+) diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 010fa4971a5..18ff7d7e9c4 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -316,4 +316,32 @@ (get_local $6) ) ) + (func $loop (type $0) (param $x i32) + (local $1 i32) + (local $2 i32) + (set_local $x + (tee_local $2 + (get_local $x) + ) + ) + (drop + (get_local $x) + ) + (loop $moar + (drop + (get_local $2) + ) + (set_local $1 + (tee_local $2 + (i32.const 1) + ) + ) + (br_if $moar + (i32.const 2) + ) + ) + (drop + (get_local $1) + ) + ) ) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index 6ee1fcd9e1a..183ad01bdde 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -127,5 +127,14 @@ ) (drop (get_local $x)) ) + (func $loop (param $x i32) + (drop (get_local $x)) + (loop $moar + (drop (get_local $x)) + (set_local $x (i32.const 1)) + (br_if $moar (i32.const 2)) + ) + (drop (get_local $x)) + ) ) From 682ebd67d006a9d41cbcd520765f81cb52bdeff9 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Thu, 25 May 2017 12:31:08 -0700 Subject: [PATCH 22/44] loop fix --- src/passes/SSAify.cpp | 8 +++++--- test/passes/ssa.txt | 38 ++++++++++++++++++++++++++++++++++++++ test/passes/ssa.wast | 12 ++++++++++++ 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 0c358fe059f..14d7ca5e47b 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -164,7 +164,8 @@ struct SSAify : public WalkerPass> { // local usage - void updateGetLocalIndex(GetLocal* curr, Mapping& mapping) { + // updates a get_local with the new, proper index. returns a possible replacment for the entire node + Expression* updateGetLocalIndex(GetLocal* curr, Mapping& mapping) { auto* set = mapping[curr->index]; if (set) { curr->index = set->index; @@ -174,9 +175,10 @@ struct SSAify : public WalkerPass> { // nothing to do, keep getting that param } else { // just replace with zero - replaceCurrent(LiteralUtils::makeZero(curr->type, *getModule())); + return LiteralUtils::makeZero(curr->type, *getModule()); } } + return curr; } void visitGetLocal(GetLocal* curr) { @@ -186,7 +188,7 @@ struct SSAify : public WalkerPass> { loopGets.push_back(curr); } originalGetIndexes[curr] = curr->index; - updateGetLocalIndex(curr, currMapping); + replaceCurrent(updateGetLocalIndex(curr, currMapping)); } void visitSetLocal(SetLocal* curr) { assert(currMapping.size() == numLocals); diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 18ff7d7e9c4..49b39fa3322 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -344,4 +344,42 @@ (get_local $1) ) ) + (func $loop2 (type $0) (param $x i32) + (local $1 i32) + (local $2 i32) + (local $3 i32) + (set_local $x + (tee_local $3 + (get_local $x) + ) + ) + (drop + (get_local $x) + ) + (loop $moar + (drop + (get_local $3) + ) + (set_local $1 + (tee_local $3 + (i32.const 1) + ) + ) + (br_if $moar + (i32.const 2) + ) + (drop + (get_local $1) + ) + (set_local $2 + (i32.const 3) + ) + (drop + (get_local $2) + ) + ) + (drop + (get_local $2) + ) + ) ) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index 183ad01bdde..1fa1f081455 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -136,5 +136,17 @@ ) (drop (get_local $x)) ) + (func $loop2 (param $x i32) + (drop (get_local $x)) + (loop $moar + (drop (get_local $x)) + (set_local $x (i32.const 1)) + (br_if $moar (i32.const 2)) + (drop (get_local $x)) ;; add use in loop before it ends, we should update this to the phi + (set_local $x (i32.const 3)) + (drop (get_local $x)) ;; another use, but should *not* be phi'd + ) + (drop (get_local $x)) + ) ) From 0382f195a808867b56ba141c4b9d6d84cabc3bf4 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Thu, 25 May 2017 13:46:43 -0700 Subject: [PATCH 23/44] more --- src/passes/SSAify.cpp | 37 +++++++++++++++++++++---------------- test/passes/ssa.txt | 26 ++++++++++++++++++-------- test/passes/ssa.wast | 3 +++ 3 files changed, 42 insertions(+), 24 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 14d7ca5e47b..5b77a7873ac 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -128,9 +128,20 @@ struct SSAify : public WalkerPass> { auto& merged = self->merge(infos); // every local we created a phi for requires us to update get_local operations in // the loop + // we need to replace precisely in this case: we enter the loop with + // old mapping of oldMapping[oldIndex] => newIndex , so wherever newIndex is + // used, we need the newer new index auto& gets = self->loopGetStack.back(); for (auto* get : gets) { - self->updateGetLocalIndex(get, merged); + auto original = self->originalGetIndexes[get]; + auto newSet = merged[original]; + if (!newSet) continue; // nothing here, nothing was merged + auto oldSet = before[original]; + // if no old set and we use the original index, or there is an old set and + // the get uses the index, then we should update to the newer phi index + if ((!oldSet && get->index == original) || (oldSet && get->index == oldSet->index)) { + get->index = newSet->index; + } } } self->mappingStack.pop_back(); @@ -164,9 +175,14 @@ struct SSAify : public WalkerPass> { // local usage - // updates a get_local with the new, proper index. returns a possible replacment for the entire node - Expression* updateGetLocalIndex(GetLocal* curr, Mapping& mapping) { - auto* set = mapping[curr->index]; + void visitGetLocal(GetLocal* curr) { + assert(currMapping.size() == numLocals); + assert(curr->index < numLocals); + for (auto& loopGets : loopGetStack) { + loopGets.push_back(curr); + } + originalGetIndexes[curr] = curr->index; + auto* set = currMapping[curr->index]; if (set) { curr->index = set->index; } else { @@ -175,20 +191,9 @@ struct SSAify : public WalkerPass> { // nothing to do, keep getting that param } else { // just replace with zero - return LiteralUtils::makeZero(curr->type, *getModule()); + replaceCurrent(LiteralUtils::makeZero(curr->type, *getModule())); } } - return curr; - } - - void visitGetLocal(GetLocal* curr) { - assert(currMapping.size() == numLocals); - assert(curr->index < numLocals); - for (auto& loopGets : loopGetStack) { - loopGets.push_back(curr); - } - originalGetIndexes[curr] = curr->index; - replaceCurrent(updateGetLocalIndex(curr, currMapping)); } void visitSetLocal(SetLocal* curr) { assert(currMapping.size() == numLocals); diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 49b39fa3322..5c466e07961 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -348,8 +348,9 @@ (local $1 i32) (local $2 i32) (local $3 i32) + (local $4 i32) (set_local $x - (tee_local $3 + (tee_local $4 (get_local $x) ) ) @@ -358,28 +359,37 @@ ) (loop $moar (drop - (get_local $3) + (get_local $4) ) (set_local $1 - (tee_local $3 - (i32.const 1) + (i32.const 1) + ) + (drop + (get_local $1) + ) + (set_local $2 + (tee_local $4 + (i32.const 123) ) ) + (drop + (get_local $2) + ) (br_if $moar (i32.const 2) ) (drop - (get_local $1) + (get_local $2) ) - (set_local $2 + (set_local $3 (i32.const 3) ) (drop - (get_local $2) + (get_local $3) ) ) (drop - (get_local $2) + (get_local $3) ) ) ) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index 1fa1f081455..b4d96eff4ea 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -141,6 +141,9 @@ (loop $moar (drop (get_local $x)) (set_local $x (i32.const 1)) + (drop (get_local $x)) + (set_local $x (i32.const 123)) + (drop (get_local $x)) (br_if $moar (i32.const 2)) (drop (get_local $x)) ;; add use in loop before it ends, we should update this to the phi (set_local $x (i32.const 3)) From 5b3ee850c2600977c796912a026ae9c47467b971 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Thu, 25 May 2017 14:16:43 -0700 Subject: [PATCH 24/44] more --- src/passes/SSAify.cpp | 11 +++++++++- src/wasm-traversal.h | 4 ++++ test/passes/ssa.txt | 50 +++++++++++++++++++++++++++++++++++++++++++ test/passes/ssa.wast | 16 ++++++++++++++ 4 files changed, 80 insertions(+), 1 deletion(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 5b77a7873ac..68888aabc54 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -58,6 +58,7 @@ struct SSAify : public WalkerPass> { std::vector> loopGetStack; // stack of loops, all the gets in each, so we can update them for phis std::map originalGetIndexes; std::vector functionPrepends; // things we add to the function prologue + std::map canZeros; // gets of a var without a set can be zero'd out void doWalkFunction(Function* func) { numLocals = func->getNumLocals(); @@ -68,6 +69,7 @@ struct SSAify : public WalkerPass> { for (auto& set : currMapping) set = nullptr; nextIndex = numLocals; WalkerPass>::walk(func->body); + // add prepends if (functionPrepends.size() > 0) { Builder builder(*getModule()); auto* block = builder.blockify(func->body); @@ -77,6 +79,12 @@ struct SSAify : public WalkerPass> { ExpressionManipulator::spliceIntoBlock(block, 0, pre); } } + // zero out stuff we can + for (auto iter : canZeros) { + auto* curr = iter.first; + auto* currp = iter.second; + *currp = LiteralUtils::makeZero(curr->type, *getModule()); + } } // control flow @@ -141,6 +149,7 @@ struct SSAify : public WalkerPass> { // the get uses the index, then we should update to the newer phi index if ((!oldSet && get->index == original) || (oldSet && get->index == oldSet->index)) { get->index = newSet->index; + self->canZeros.erase(get); } } } @@ -191,7 +200,7 @@ struct SSAify : public WalkerPass> { // nothing to do, keep getting that param } else { // just replace with zero - replaceCurrent(LiteralUtils::makeZero(curr->type, *getModule())); + canZeros[curr] = getCurrentPointer(); } } } diff --git a/src/wasm-traversal.h b/src/wasm-traversal.h index f49407cad88..aa8e008ad53 100644 --- a/src/wasm-traversal.h +++ b/src/wasm-traversal.h @@ -164,6 +164,10 @@ struct Walker : public VisitorType { return *replacep; } + Expression** getCurrentPointer() { + return replacep; + } + // Get the current module Module* getModule() { return currModule; diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 5c466e07961..cce089d6fa0 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -1,5 +1,6 @@ (module (type $0 (func (param i32))) + (type $1 (func)) (memory $0 0) (func $basics (type $0) (param $x i32) (local $y i32) @@ -392,4 +393,53 @@ (get_local $3) ) ) + (func $loop2-zeroinit (type $1) + (local $x i32) + (local $1 i32) + (local $2 i32) + (local $3 i32) + (local $4 i32) + (set_local $x + (tee_local $4 + (i32.const 0) + ) + ) + (drop + (i32.const 0) + ) + (loop $moar + (drop + (get_local $4) + ) + (set_local $1 + (i32.const 1) + ) + (drop + (get_local $1) + ) + (set_local $2 + (tee_local $4 + (i32.const 123) + ) + ) + (drop + (get_local $2) + ) + (br_if $moar + (i32.const 2) + ) + (drop + (get_local $2) + ) + (set_local $3 + (i32.const 3) + ) + (drop + (get_local $3) + ) + ) + (drop + (get_local $3) + ) + ) ) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index b4d96eff4ea..7f48f98429f 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -151,5 +151,21 @@ ) (drop (get_local $x)) ) + (func $loop2-zeroinit + (local $x i32) + (drop (get_local $x)) + (loop $moar + (drop (get_local $x)) + (set_local $x (i32.const 1)) + (drop (get_local $x)) + (set_local $x (i32.const 123)) + (drop (get_local $x)) + (br_if $moar (i32.const 2)) + (drop (get_local $x)) ;; add use in loop before it ends, we should update this to the phi + (set_local $x (i32.const 3)) + (drop (get_local $x)) ;; another use, but should *not* be phi'd + ) + (drop (get_local $x)) + ) ) From da9773e98ac29f478f250caa6223e06e8ca31a87 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Thu, 25 May 2017 14:46:46 -0700 Subject: [PATCH 25/44] fast --- src/passes/SSAify.cpp | 10 +- test/passes/ssa.txt | 348 ++++++++++++++++++++++-------------------- 2 files changed, 185 insertions(+), 173 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 68888aabc54..f05230d7bb1 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -28,7 +28,6 @@ #include "pass.h" #include "wasm-builder.h" #include "support/permutations.h" -#include "ast/manipulation.h" #include "ast/literal-utils.h" namespace wasm { @@ -72,12 +71,13 @@ struct SSAify : public WalkerPass> { // add prepends if (functionPrepends.size() > 0) { Builder builder(*getModule()); - auto* block = builder.blockify(func->body); - func->body = block; - // TODO: this is O(toplevel block size^2) + auto* block = builder.makeBlock(); for (auto* pre : functionPrepends) { - ExpressionManipulator::spliceIntoBlock(block, 0, pre); + block->list.push_back(pre); } + block->list.push_back(func->body); + block->finalize(func->body->type); + func->body = block; } // zero out stuff we can for (auto iter : canZeros) { diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index cce089d6fa0..b8a32337b53 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -65,11 +65,6 @@ (local $15 i32) (local $16 i32) (local $17 i32) - (set_local $p - (tee_local $6 - (get_local $p) - ) - ) (set_local $x (tee_local $4 (tee_local $8 @@ -77,97 +72,104 @@ ) ) ) - (drop - (if i32 - (i32.const 1) - (i32.const 0) - (i32.const 0) + (set_local $p + (tee_local $6 + (get_local $p) ) ) - (if - (i32.const 1) - (set_local $3 - (tee_local $4 + (block + (drop + (if i32 (i32.const 1) + (i32.const 0) + (i32.const 0) ) ) - ) - (drop - (get_local $4) - ) - (if - (i32.const 1) - (set_local $5 - (tee_local $6 - (i32.const 1) + (if + (i32.const 1) + (set_local $3 + (tee_local $4 + (i32.const 1) + ) ) ) - ) - (drop - (get_local $6) - ) - (if - (i32.const 1) - (set_local $7 - (tee_local $8 - (tee_local $10 - (i32.const 2) + (drop + (get_local $4) + ) + (if + (i32.const 1) + (set_local $5 + (tee_local $6 + (i32.const 1) ) ) ) - (nop) - ) - (drop - (get_local $8) - ) - (if - (i32.const 1) - (nop) - (set_local $9 - (tee_local $10 - (i32.const 3) - ) + (drop + (get_local $6) ) - ) - (drop - (get_local $10) - ) - (if - (i32.const 1) - (set_local $11 - (tee_local $13 - (i32.const 4) + (if + (i32.const 1) + (set_local $7 + (tee_local $8 + (tee_local $10 + (i32.const 2) + ) + ) ) + (nop) ) - (set_local $12 - (tee_local $13 - (i32.const 5) - ) + (drop + (get_local $8) ) - ) - (drop - (get_local $13) - ) - (if - (i32.const 1) - (set_local $14 - (tee_local $17 - (i32.const 6) + (if + (i32.const 1) + (nop) + (set_local $9 + (tee_local $10 + (i32.const 3) + ) ) ) - (block $block - (set_local $15 - (i32.const 7) + (drop + (get_local $10) + ) + (if + (i32.const 1) + (set_local $11 + (tee_local $13 + (i32.const 4) + ) + ) + (set_local $12 + (tee_local $13 + (i32.const 5) + ) ) - (set_local $16 + ) + (drop + (get_local $13) + ) + (if + (i32.const 1) + (set_local $14 (tee_local $17 - (i32.const 8) + (i32.const 6) + ) + ) + (block $block + (set_local $15 + (i32.const 7) + ) + (set_local $16 + (tee_local $17 + (i32.const 8) + ) ) ) ) - ) - (drop - (get_local $17) + (drop + (get_local $17) + ) ) ) (func $if2 (type $0) (param $x i32) @@ -178,21 +180,23 @@ (get_local $x) ) ) - (if - (i32.const 1) - (block $block - (set_local $1 - (tee_local $2 - (i32.const 1) + (block + (if + (i32.const 1) + (block $block + (set_local $1 + (tee_local $2 + (i32.const 1) + ) + ) + (drop + (get_local $1) ) - ) - (drop - (get_local $1) ) ) - ) - (drop - (get_local $2) + (drop + (get_local $2) + ) ) ) (func $block (type $0) (param $x i32) @@ -203,18 +207,20 @@ (get_local $x) ) ) - (block $out - (br_if $out - (i32.const 2) - ) - (set_local $1 - (tee_local $2 - (i32.const 1) + (block + (block $out + (br_if $out + (i32.const 2) + ) + (set_local $1 + (tee_local $2 + (i32.const 1) + ) ) ) - ) - (drop - (get_local $2) + (drop + (get_local $2) + ) ) ) (func $block2 (type $0) (param $x i32) @@ -325,25 +331,27 @@ (get_local $x) ) ) - (drop - (get_local $x) - ) - (loop $moar + (block (drop - (get_local $2) + (get_local $x) ) - (set_local $1 - (tee_local $2 - (i32.const 1) + (loop $moar + (drop + (get_local $2) + ) + (set_local $1 + (tee_local $2 + (i32.const 1) + ) + ) + (br_if $moar + (i32.const 2) ) ) - (br_if $moar - (i32.const 2) + (drop + (get_local $1) ) ) - (drop - (get_local $1) - ) ) (func $loop2 (type $0) (param $x i32) (local $1 i32) @@ -355,43 +363,45 @@ (get_local $x) ) ) - (drop - (get_local $x) - ) - (loop $moar - (drop - (get_local $4) - ) - (set_local $1 - (i32.const 1) - ) + (block (drop - (get_local $1) + (get_local $x) ) - (set_local $2 - (tee_local $4 - (i32.const 123) + (loop $moar + (drop + (get_local $4) + ) + (set_local $1 + (i32.const 1) + ) + (drop + (get_local $1) + ) + (set_local $2 + (tee_local $4 + (i32.const 123) + ) + ) + (drop + (get_local $2) + ) + (br_if $moar + (i32.const 2) + ) + (drop + (get_local $2) + ) + (set_local $3 + (i32.const 3) + ) + (drop + (get_local $3) ) - ) - (drop - (get_local $2) - ) - (br_if $moar - (i32.const 2) - ) - (drop - (get_local $2) - ) - (set_local $3 - (i32.const 3) ) (drop (get_local $3) ) ) - (drop - (get_local $3) - ) ) (func $loop2-zeroinit (type $1) (local $x i32) @@ -404,42 +414,44 @@ (i32.const 0) ) ) - (drop - (i32.const 0) - ) - (loop $moar - (drop - (get_local $4) - ) - (set_local $1 - (i32.const 1) - ) + (block (drop - (get_local $1) + (i32.const 0) ) - (set_local $2 - (tee_local $4 - (i32.const 123) + (loop $moar + (drop + (get_local $4) + ) + (set_local $1 + (i32.const 1) + ) + (drop + (get_local $1) + ) + (set_local $2 + (tee_local $4 + (i32.const 123) + ) + ) + (drop + (get_local $2) + ) + (br_if $moar + (i32.const 2) + ) + (drop + (get_local $2) + ) + (set_local $3 + (i32.const 3) + ) + (drop + (get_local $3) ) - ) - (drop - (get_local $2) - ) - (br_if $moar - (i32.const 2) - ) - (drop - (get_local $2) - ) - (set_local $3 - (i32.const 3) ) (drop (get_local $3) ) ) - (drop - (get_local $3) - ) ) ) From c92c7a702c5b06115d5900d191cb3215b379380f Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Thu, 25 May 2017 19:15:58 -0700 Subject: [PATCH 26/44] optimize --- src/passes/SSAify.cpp | 33 ++++++++------ test/passes/ssa.txt | 104 +++++++++++++++++------------------------- 2 files changed, 60 insertions(+), 77 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index f05230d7bb1..123b04261c8 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -295,22 +295,27 @@ struct SSAify : public WalkerPass> { // seen it already continue; } - if (!set) { - // this is a param or the zero init value. add a set first thing in - // the function - set = builder.makeSetLocal( - old, - getFunction()->isParam(old) ? builder.makeGetLocal(old, getFunction()->getLocalType(old)) - : LiteralUtils::makeZero(getFunction()->getLocalType(old), *getModule()) + if (set) { + // a set exists, just add a tee of its value, so we also set the phi merge var + set->value = ret = builder.makeTeeLocal( + new_, + set->value ); - mapping[old] = set; - functionPrepends.push_back(set); + } else { + // this is a param or the zero init value. + if (getFunction()->isParam(old)) { + // we add a set with the proper + // param value at the beginning of the function + auto* set = ret = builder.makeSetLocal( + new_, + getFunction()->isParam(old) ? builder.makeGetLocal(old, getFunction()->getLocalType(old)) + : LiteralUtils::makeZero(getFunction()->getLocalType(old), *getModule()) + ); + functionPrepends.push_back(set); + } else { + // this is a zero init, so we don't need to do anything actually + } } - // now a set exists, just add a tee of its value, so we also set the phi merge var - set->value = ret = builder.makeTeeLocal( - new_, - set->value - ); } return ret; } diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index b8a32337b53..41e13e73543 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -65,17 +65,8 @@ (local $15 i32) (local $16 i32) (local $17 i32) - (set_local $x - (tee_local $4 - (tee_local $8 - (i32.const 0) - ) - ) - ) - (set_local $p - (tee_local $6 - (get_local $p) - ) + (set_local $6 + (get_local $p) ) (block (drop @@ -89,7 +80,9 @@ (i32.const 1) (set_local $3 (tee_local $4 - (i32.const 1) + (tee_local $8 + (i32.const 1) + ) ) ) ) @@ -175,10 +168,8 @@ (func $if2 (type $0) (param $x i32) (local $1 i32) (local $2 i32) - (set_local $x - (tee_local $2 - (get_local $x) - ) + (set_local $2 + (get_local $x) ) (block (if @@ -202,10 +193,8 @@ (func $block (type $0) (param $x i32) (local $1 i32) (local $2 i32) - (set_local $x - (tee_local $2 - (get_local $x) - ) + (set_local $2 + (get_local $x) ) (block (block $out @@ -326,10 +315,8 @@ (func $loop (type $0) (param $x i32) (local $1 i32) (local $2 i32) - (set_local $x - (tee_local $2 - (get_local $x) - ) + (set_local $2 + (get_local $x) ) (block (drop @@ -358,10 +345,8 @@ (local $2 i32) (local $3 i32) (local $4 i32) - (set_local $x - (tee_local $4 - (get_local $x) - ) + (set_local $4 + (get_local $x) ) (block (drop @@ -409,49 +394,42 @@ (local $2 i32) (local $3 i32) (local $4 i32) - (set_local $x - (tee_local $4 - (i32.const 0) - ) + (drop + (i32.const 0) ) - (block + (loop $moar (drop - (i32.const 0) + (get_local $4) ) - (loop $moar - (drop - (get_local $4) - ) - (set_local $1 - (i32.const 1) - ) - (drop - (get_local $1) - ) - (set_local $2 - (tee_local $4 - (i32.const 123) - ) - ) - (drop - (get_local $2) - ) - (br_if $moar - (i32.const 2) - ) - (drop - (get_local $2) - ) - (set_local $3 - (i32.const 3) - ) - (drop - (get_local $3) + (set_local $1 + (i32.const 1) + ) + (drop + (get_local $1) + ) + (set_local $2 + (tee_local $4 + (i32.const 123) ) ) + (drop + (get_local $2) + ) + (br_if $moar + (i32.const 2) + ) + (drop + (get_local $2) + ) + (set_local $3 + (i32.const 3) + ) (drop (get_local $3) ) ) + (drop + (get_local $3) + ) ) ) From 98a86022cab4ffb1c6c08b0d2a57ae14dd362869 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Fri, 26 May 2017 13:36:28 -0700 Subject: [PATCH 27/44] testcase --- test/passes/ssa.wast | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index 7f48f98429f..c4e22b71b3c 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -1,4 +1,5 @@ (module + (import "env" "_emscripten_autodebug_i32" (func $import$15 (param i32 i32))) (func $basics (param $x i32) (local $y i32) (local $z f32) @@ -167,5 +168,31 @@ ) (drop (get_local $x)) ) + (func $real-loop + (param $param i32) + (local $loopvar i32) + (local $inc i32) + (set_local $loopvar + (get_local $param) + ) + (loop $more + (block $stop + (if + (i32.const 1) + (br $stop) + ) + (set_local $inc + (i32.add + (get_local $loopvar) ;; this var should be written to from before the loop and the inc at the end + (i32.const 1) + ) + ) + (set_local $loopvar + (get_local $inc) + ) + (br $more) + ) + ) + ) ) From d91d3954b27a3a7fff8b504334a1bf265560cfc7 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Sat, 27 May 2017 14:53:58 -0700 Subject: [PATCH 28/44] rework wip --- src/passes/SSAify.cpp | 148 +++++++++++++++++++++++------------------- 1 file changed, 82 insertions(+), 66 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 123b04261c8..93b313fa203 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -18,8 +18,7 @@ // Transforms code into SSA form. That ensures each variable has a // single assignment. For phis, we do not add a new node to the AST, // so the result is multiple assignments but with the guarantee that -// they all travel directly to the same basic block, i.e., they are -// a way to represent a phi in our AST. +// they all lead to the same get_local // TODO: consider adding a "proper" phi node to the AST, that passes // can utilize // @@ -43,21 +42,25 @@ struct SSAify : public WalkerPass> { Pass* create() override { return new SSAify; } - // we map (old local index) => the set_local for that index. The new - // index for the local can be seen in that set local. - // this can be nullptr if there is no set_local, and instead the value - // is a parameter, or the zero init. - typedef std::vector Mapping; + // the set_locals relevant for an index or a get. we use + // as set as merges of control flow mean more than 1 may + // be relevant; we create a phi on demand when necessary for those + typedef std::set Sets; + + // we map (old local index) => the set_locals for that index. + // a nullptr set means there is a virtual set, from a param + // initial value or the zero init initial value. + typedef std::vector Mapping; Index numLocals; Mapping currMapping; Index nextIndex; std::vector mappingStack; // used in ifs, loops std::map> breakMappings; // break target => infos that reach it - std::vector> loopGetStack; // stack of loops, all the gets in each, so we can update them for phis - std::map originalGetIndexes; + std::vector> loopGetStack; // stack of loops, all the gets in each, so we can update them for back branches std::vector functionPrepends; // things we add to the function prologue - std::map canZeros; // gets of a var without a set can be zero'd out + std::map getSetses; // the sets for each get + std::map getLocations; void doWalkFunction(Function* func) { numLocals = func->getNumLocals(); @@ -65,9 +68,13 @@ struct SSAify : public WalkerPass> { // We begin with each param being assigned from the incoming value, and the zero-init for the locals, // so the initial state is the identity permutation currMapping.resize(numLocals); - for (auto& set : currMapping) set = nullptr; + for (auto& set : currMapping) { + set = { nullptr }; + } nextIndex = numLocals; WalkerPass>::walk(func->body); + // apply - we now know the sets for each get + computeGetsAndPhis(); // add prepends if (functionPrepends.size() > 0) { Builder builder(*getModule()); @@ -79,12 +86,6 @@ struct SSAify : public WalkerPass> { block->finalize(func->body->type); func->body = block; } - // zero out stuff we can - for (auto iter : canZeros) { - auto* curr = iter.first; - auto* currp = iter.second; - *currp = LiteralUtils::makeZero(curr->type, *getModule()); - } } // control flow @@ -135,21 +136,30 @@ struct SSAify : public WalkerPass> { auto before = infos.back(); auto& merged = self->merge(infos); // every local we created a phi for requires us to update get_local operations in - // the loop - // we need to replace precisely in this case: we enter the loop with - // old mapping of oldMapping[oldIndex] => newIndex , so wherever newIndex is - // used, we need the newer new index + // the loop - the branch back has means that gets in the loop have potentially + // more sets reaching them. + // we can detect this as follows: if a get of oldIndex has the same sets + // as the sets at the entrance to the loop, then it is affected by the loop + // header sets, and we can add to there sets that looped back auto& gets = self->loopGetStack.back(); for (auto* get : gets) { - auto original = self->originalGetIndexes[get]; - auto newSet = merged[original]; - if (!newSet) continue; // nothing here, nothing was merged - auto oldSet = before[original]; - // if no old set and we use the original index, or there is an old set and - // the get uses the index, then we should update to the newer phi index - if ((!oldSet && get->index == original) || (oldSet && get->index == oldSet->index)) { - get->index = newSet->index; - self->canZeros.erase(get); + auto& beforeSets = before[get->index]; + auto& getSets = getSetses[get]; + if (getSets.size() < beforeSets.size()) { + // the get trivially has fewer sets, so it overrode the loop entry sets + continue; + } + Sets intersection; + std::set_intersection(beforeSets.begin(), beforeSets.end(), + getSets.begin(), getSets.end(), + std::back_inserter(intersection)); + if (intersection.size() < beforeSets.size()) { + // the get has not the same sets as in the loop entry + continue; + } + // the get has the entry sets, so add any new ones + for (auto* set : merged) { + getSets.insert(set); } } } @@ -190,24 +200,15 @@ struct SSAify : public WalkerPass> { for (auto& loopGets : loopGetStack) { loopGets.push_back(curr); } - originalGetIndexes[curr] = curr->index; - auto* set = currMapping[curr->index]; - if (set) { - curr->index = set->index; - } else { - // no set, this is either a param or a zero init - if (curr->index < getFunction()->getNumParams()) { - // nothing to do, keep getting that param - } else { - // just replace with zero - canZeros[curr] = getCurrentPointer(); - } - } + // current sets are our sets + getSetses[curr] = currMapping[curr->index]; + getLocations[curr] = getCurrPointer(); } void visitSetLocal(SetLocal* curr) { assert(currMapping.size() == numLocals); assert(curr->index < numLocals); - currMapping[curr->index] = curr; + // current sets are just this set + currMapping[curr->index] = { curr }; // TODO optimize? curr->index = addLocal(getFunction()->getLocalType(curr->index)); } @@ -252,34 +253,49 @@ struct SSAify : public WalkerPass> { assert(mappings.size() > 0); auto& out = mappings[0]; if (mappings.size() == 1) { - return mappings[0]; + return out; } - for (Index i = 0; i < numLocals; i++) { - SetLocal* merged = &IMPOSSIBLE_SET; - for (auto& mapping : mappings) { - if (isUnreachable(mapping)) continue; - if (merged == &IMPOSSIBLE_SET) { - merged = mapping[i]; + // merge into the first + for (Index j = 1; j < mappings.size(); j++) { + auto& other = mappings[j]; + for (Index i = 0; i < numLocals; i++) { + auto& outSets = out[i]; + for (auto* set : other[i]) { + outSets.insert(set); + } + } + } + return out; + } + + // After we traversed it all, we can compute gets and phis + void computeGetsAndPhis() { + for (auto& iter : getSetses) { + auto* get = iter.first; + auto& sets = iter.second; + if (sets->size() == 0) { + continue; // unreachable, ignore + } + if (sets->size() == 1) { + // TODO: add tests for this case + // easy, just one set, use it's index + auto* set = sets.begin(); + if (set) { + get->index = set->index; } else { - if (mapping[i] != merged) { - // we need a phi for this local, e.g., imagine - // if (..) { x = 1 } else { x = 2 } ..use x here.. - // create a new local, and also write to it at the old write locations - // if (..) { x = y = 1 } else { x = y = 2 } ..use y here.. - // (not that y seems as "single-assignment" as x here, but the point is - // that x may be merged multiple times, so we need a different phi merge - // local for each) - auto phiLocal = addLocal(getFunction()->getLocalType(i)); -; merged = addWritesToLocal(mappings, i, phiLocal); - break; + // no set, assign param or zero + if (getFunction()->isParam(get->index)) { + // leave it, it's fine + } else { + // zero it out + (*getLocations[get]) = LiteralUtils::makeZero(get->type, *getModule()); } } + continue; } - if (merged != &IMPOSSIBLE_SET) { - out[i] = merged; - } + // more than 1 set, need a phi: a new local written to at each of the sets + // if there is already a local with that property, reuse it } - return out; } // adds phi-style writes to sets of a local From 13b15a585f127e850887bce63572b8cde7f1d2e3 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Sat, 27 May 2017 15:37:10 -0700 Subject: [PATCH 29/44] write up --- src/passes/SSAify.cpp | 82 +++++++++++++++++++++++++------------------ 1 file changed, 48 insertions(+), 34 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 93b313fa203..58e4df6980e 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -295,45 +295,59 @@ struct SSAify : public WalkerPass> { } // more than 1 set, need a phi: a new local written to at each of the sets // if there is already a local with that property, reuse it - } - } - - // adds phi-style writes to sets of a local - // returns one of those writes - SetLocal* addWritesToLocal(std::vector& mappings, Index old, Index new_) { - SetLocal* ret = nullptr; - Builder builder(*getModule()); - // the same set may appear in multiple mappings; we just need one for each - std::set seen; - for (auto& mapping : mappings) { - auto* set = mapping[old]; - if (!seen.insert(set).second) { - // seen it already - continue; + auto gatherIndexes = [](SetLocal* set) { + std::set ret; + while (set) { + ret->insert(set->index); + set = set->value->dynCast(); + } + return ret; } - if (set) { - // a set exists, just add a tee of its value, so we also set the phi merge var - set->value = ret = builder.makeTeeLocal( - new_, - set->value - ); + auto indexes = gatherIndexes(sets[0]); + for (Index i = 1; i < sets.size(); i++) { + auto* curr = sets[i]; + auto currIndexes = gatherIndexes(curr); + Sets intersection; + std::set_intersection(indexes.begin(), indexes.end(), + currIndexes.begin(), currIndexes.end(), + std::back_inserter(intersection)); + indexes.swap(intersection); + if (indexes.empty()) break; + } + if (!indexes.empty()) { + // we found an index, use it + get->index = indexes.begin()->index; } else { - // this is a param or the zero init value. - if (getFunction()->isParam(old)) { - // we add a set with the proper - // param value at the beginning of the function - auto* set = ret = builder.makeSetLocal( - new_, - getFunction()->isParam(old) ? builder.makeGetLocal(old, getFunction()->getLocalType(old)) - : LiteralUtils::makeZero(getFunction()->getLocalType(old), *getModule()) - ); - functionPrepends.push_back(set); - } else { - // this is a zero init, so we don't need to do anything actually + // we need to create a local for this phi'ing + auto new_ = addLocal(get->type); + auto old = get->index; + get->index = new_; + Builder builder(*getModule()); + // write to the local in each of our sets + for (auto* set : sets) { + if (set) { + // a set exists, just add a tee of its value + set->value = builder.makeTeeLocal( + new_, + set->value + ); + } else { + // this is a param or the zero init value. + if (getFunction()->isParam(old)) { + // we add a set with the proper + // param value at the beginning of the function + auto* set = builder.makeSetLocal( + new_, + builder.makeGetLocal(old, getFunction()->getLocalType(old)) + ); + functionPrepends.push_back(set); + } else { + // this is a zero init, so we don't need to do anything actually + } + } } } } - return ret; } Index addLocal(WasmType type) { From 06dfc191c9c49f047cfed90475485a36155eb16c Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Sun, 28 May 2017 16:24:44 -0700 Subject: [PATCH 30/44] builds --- src/passes/SSAify.cpp | 43 ++++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 58e4df6980e..e6a3a8f463a 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -144,12 +144,12 @@ struct SSAify : public WalkerPass> { auto& gets = self->loopGetStack.back(); for (auto* get : gets) { auto& beforeSets = before[get->index]; - auto& getSets = getSetses[get]; + auto& getSets = self->getSetses[get]; if (getSets.size() < beforeSets.size()) { // the get trivially has fewer sets, so it overrode the loop entry sets continue; } - Sets intersection; + std::vector intersection; std::set_intersection(beforeSets.begin(), beforeSets.end(), getSets.begin(), getSets.end(), std::back_inserter(intersection)); @@ -158,7 +158,7 @@ struct SSAify : public WalkerPass> { continue; } // the get has the entry sets, so add any new ones - for (auto* set : merged) { + for (auto* set : merged[get->index]) { getSets.insert(set); } } @@ -202,7 +202,7 @@ struct SSAify : public WalkerPass> { } // current sets are our sets getSetses[curr] = currMapping[curr->index]; - getLocations[curr] = getCurrPointer(); + getLocations[curr] = getCurrentPointer(); } void visitSetLocal(SetLocal* curr) { assert(currMapping.size() == numLocals); @@ -239,11 +239,12 @@ struct SSAify : public WalkerPass> { void setUnreachable(Mapping& mapping) { mapping.resize(numLocals); // may have been emptied by a move - mapping[0] = &IMPOSSIBLE_SET; + mapping[0].clear(); } bool isUnreachable(Mapping& mapping) { - return mapping[0] == &IMPOSSIBLE_SET; + // we must have some set for each index, if only the zero init, so empty means we emptied it for unreachable code + return mapping[0].empty(); } // merges a bunch of infos into one. @@ -273,13 +274,13 @@ struct SSAify : public WalkerPass> { for (auto& iter : getSetses) { auto* get = iter.first; auto& sets = iter.second; - if (sets->size() == 0) { + if (sets.size() == 0) { continue; // unreachable, ignore } - if (sets->size() == 1) { + if (sets.size() == 1) { // TODO: add tests for this case // easy, just one set, use it's index - auto* set = sets.begin(); + auto* set = *sets.begin(); if (set) { get->index = set->index; } else { @@ -298,25 +299,29 @@ struct SSAify : public WalkerPass> { auto gatherIndexes = [](SetLocal* set) { std::set ret; while (set) { - ret->insert(set->index); + ret.insert(set->index); set = set->value->dynCast(); } return ret; - } - auto indexes = gatherIndexes(sets[0]); - for (Index i = 1; i < sets.size(); i++) { - auto* curr = sets[i]; - auto currIndexes = gatherIndexes(curr); - Sets intersection; + }; + auto indexes = gatherIndexes(*sets.begin()); + for (auto* set : sets) { + if (set == *sets.begin()) continue; + auto currIndexes = gatherIndexes(set); + std::vector intersection; std::set_intersection(indexes.begin(), indexes.end(), currIndexes.begin(), currIndexes.end(), std::back_inserter(intersection)); - indexes.swap(intersection); - if (indexes.empty()) break; + if (intersection.empty()) break; + // TODO: or keep sorted vectors? + indexes.clear(); + for (Index i : intersection) { + indexes.insert(i); + } } if (!indexes.empty()) { // we found an index, use it - get->index = indexes.begin()->index; + get->index = *indexes.begin(); } else { // we need to create a local for this phi'ing auto new_ = addLocal(get->type); From 25a2717292118cf47ffda85e4ba59e7c5cbad102 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Sun, 28 May 2017 20:12:36 -0700 Subject: [PATCH 31/44] fix --- src/passes/SSAify.cpp | 21 ++++++----- test/passes/ssa.txt | 81 +++++++++++++++++++++++++++++++------------ test/passes/ssa.wast | 1 - 3 files changed, 68 insertions(+), 35 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index e6a3a8f463a..a4a8f829360 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -128,23 +128,22 @@ struct SSAify : public WalkerPass> { self->mappingStack.push_back(self->currMapping); self->loopGetStack.push_back({}); } - static void doVisitLoop(SSAify* self, Expression** currp) { - auto* curr = (*currp)->cast(); - if (curr->name.is() && self->breakMappings.find(curr->name) != self->breakMappings.end()) { - auto& infos = self->breakMappings[curr->name]; - infos.emplace_back(std::move(self->mappingStack.back())); + void visitLoop(Loop* curr) { + if (curr->name.is() && breakMappings.find(curr->name) != breakMappings.end()) { + auto& infos = breakMappings[curr->name]; + infos.emplace_back(std::move(mappingStack.back())); auto before = infos.back(); - auto& merged = self->merge(infos); + auto& merged = merge(infos); // every local we created a phi for requires us to update get_local operations in // the loop - the branch back has means that gets in the loop have potentially // more sets reaching them. // we can detect this as follows: if a get of oldIndex has the same sets // as the sets at the entrance to the loop, then it is affected by the loop // header sets, and we can add to there sets that looped back - auto& gets = self->loopGetStack.back(); + auto& gets = loopGetStack.back(); for (auto* get : gets) { auto& beforeSets = before[get->index]; - auto& getSets = self->getSetses[get]; + auto& getSets = getSetses[get]; if (getSets.size() < beforeSets.size()) { // the get trivially has fewer sets, so it overrode the loop entry sets continue; @@ -163,8 +162,8 @@ struct SSAify : public WalkerPass> { } } } - self->mappingStack.pop_back(); - self->loopGetStack.pop_back(); + mappingStack.pop_back(); + loopGetStack.pop_back(); } void visitBreak(Break* curr) { if (curr->condition) { @@ -312,9 +311,9 @@ struct SSAify : public WalkerPass> { std::set_intersection(indexes.begin(), indexes.end(), currIndexes.begin(), currIndexes.end(), std::back_inserter(intersection)); + indexes.clear(); if (intersection.empty()) break; // TODO: or keep sorted vectors? - indexes.clear(); for (Index i : intersection) { indexes.insert(i); } diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 41e13e73543..69a9f94622b 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -65,7 +65,7 @@ (local $15 i32) (local $16 i32) (local $17 i32) - (set_local $6 + (set_local $13 (get_local $p) ) (block @@ -79,32 +79,34 @@ (if (i32.const 1) (set_local $3 - (tee_local $4 - (tee_local $8 - (i32.const 1) + (tee_local $15 + (tee_local $14 + (tee_local $12 + (i32.const 1) + ) ) ) ) ) (drop - (get_local $4) + (get_local $12) ) (if (i32.const 1) - (set_local $5 - (tee_local $6 + (set_local $4 + (tee_local $13 (i32.const 1) ) ) ) (drop - (get_local $6) + (get_local $13) ) (if (i32.const 1) - (set_local $7 - (tee_local $8 - (tee_local $10 + (set_local $5 + (tee_local $15 + (tee_local $14 (i32.const 2) ) ) @@ -112,48 +114,48 @@ (nop) ) (drop - (get_local $8) + (get_local $14) ) (if (i32.const 1) (nop) - (set_local $9 - (tee_local $10 + (set_local $6 + (tee_local $15 (i32.const 3) ) ) ) (drop - (get_local $10) + (get_local $15) ) (if (i32.const 1) - (set_local $11 - (tee_local $13 + (set_local $7 + (tee_local $16 (i32.const 4) ) ) - (set_local $12 - (tee_local $13 + (set_local $8 + (tee_local $16 (i32.const 5) ) ) ) (drop - (get_local $13) + (get_local $16) ) (if (i32.const 1) - (set_local $14 + (set_local $9 (tee_local $17 (i32.const 6) ) ) (block $block - (set_local $15 + (set_local $10 (i32.const 7) ) - (set_local $16 + (set_local $11 (tee_local $17 (i32.const 8) ) @@ -432,4 +434,37 @@ (get_local $3) ) ) + (func $real-loop (type $0) (param $param i32) + (local $loopvar i32) + (local $inc i32) + (local $3 i32) + (local $4 i32) + (local $5 i32) + (local $6 i32) + (set_local $3 + (tee_local $6 + (get_local $param) + ) + ) + (loop $more + (block $stop + (if + (i32.const 1) + (br $stop) + ) + (set_local $4 + (i32.add + (get_local $6) + (i32.const 1) + ) + ) + (set_local $5 + (tee_local $6 + (get_local $4) + ) + ) + (br $more) + ) + ) + ) ) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index c4e22b71b3c..27581f3aac7 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -1,5 +1,4 @@ (module - (import "env" "_emscripten_autodebug_i32" (func $import$15 (param i32 i32))) (func $basics (param $x i32) (local $y i32) (local $z f32) From 7099cac2c7bf950672d38525b8807b369cc83c4f Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 7 Jun 2017 14:29:22 -0700 Subject: [PATCH 32/44] default output names in wasm-as/dis are the input name with modified suffix --- src/tools/tool-utils.h | 37 +++++++++++++++++++++++++++++++++++++ src/tools/wasm-as.cpp | 7 +++++++ 2 files changed, 44 insertions(+) create mode 100644 src/tools/tool-utils.h diff --git a/src/tools/tool-utils.h b/src/tools/tool-utils.h new file mode 100644 index 00000000000..a897e01f0ca --- /dev/null +++ b/src/tools/tool-utils.h @@ -0,0 +1,37 @@ +/* + * Copyright 2017 WebAssembly Community Group participants + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// +// Shared utilities for commandline tools +// + +#include + +namespace wasm { + +// Removes a specific suffix if it is present, otherwise no-op +inline std::string removeSpecificSuffix(std::string str, std::string suffix) { + if (str.size() <= suffix.size()) { + return str; + } + if (str.substr(str.size() - suffix.size()).compare(suffix) == 0) { + return str.substr(0, str.size() - suffix.size()); + } + return str; +} + +} // namespace wasm + diff --git a/src/tools/wasm-as.cpp b/src/tools/wasm-as.cpp index e8003a5ca16..d4b495562ae 100644 --- a/src/tools/wasm-as.cpp +++ b/src/tools/wasm-as.cpp @@ -24,6 +24,8 @@ #include "wasm-binary.h" #include "wasm-s-parser.h" +#include "tool-utils.h" + using namespace cashew; using namespace wasm; @@ -68,6 +70,11 @@ int main(int argc, const char *argv[]) { }); options.parse(argc, argv); + // default output is infile with changed suffix + if (options.extra.find("output") == options.extra.end()) { + options.extra["output"] = removeSpecificSuffix(options.extra["infile"], ".wast") + ".wasm"; + } + auto input(read_file(options.extra["infile"], Flags::Text, options.debug ? Flags::Debug : Flags::Release)); Module wasm; From 97e5645a588914c4631cd566eaf7f3b8a18c9cdb Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 7 Jun 2017 16:59:13 -0700 Subject: [PATCH 33/44] add InstrumentLocals pass --- src/passes/CMakeLists.txt | 1 + src/passes/InstrumentLocals.cpp | 140 +++++++++++++++++++++++++++++ src/passes/pass.cpp | 1 + src/passes/passes.h | 1 + test/passes/instrument-locals.txt | 118 ++++++++++++++++++++++++ test/passes/instrument-locals.wast | 29 ++++++ 6 files changed, 290 insertions(+) create mode 100644 src/passes/InstrumentLocals.cpp create mode 100644 test/passes/instrument-locals.txt create mode 100644 test/passes/instrument-locals.wast diff --git a/src/passes/CMakeLists.txt b/src/passes/CMakeLists.txt index b774ebd2d98..f1411f1903c 100644 --- a/src/passes/CMakeLists.txt +++ b/src/passes/CMakeLists.txt @@ -10,6 +10,7 @@ SET(passes_SOURCES LegalizeJSInterface.cpp LocalCSE.cpp LogExecution.cpp + InstrumentLocals.cpp InstrumentMemory.cpp MemoryPacking.cpp MergeBlocks.cpp diff --git a/src/passes/InstrumentLocals.cpp b/src/passes/InstrumentLocals.cpp new file mode 100644 index 00000000000..22b8ebf7030 --- /dev/null +++ b/src/passes/InstrumentLocals.cpp @@ -0,0 +1,140 @@ +/* + * Copyright 2017 WebAssembly Community Group participants + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// +// Instruments the build with code to intercept all local reads and writes. +// +// gets: +// +// Before: +// (get_local $x) +// +// After: +// (call $get_TYPE +// (i32.const n) // call id +// (i32.const n) // local id +// (get_local $x) +// ) +// +// sets: +// +// Before: +// (set_local $x (i32.const 1)) +// +// After: +// (set_local $x +// (call $set_TYPE +// (i32.const n) // call id +// (i32.const n) // local id +// (i32.const 1) // value +// ) +// ) + +#include +#include +#include +#include "shared-constants.h" +#include "asmjs/shared-constants.h" +#include "asm_v_wasm.h" + +namespace wasm { + +Name get_i32("get_i32"); +Name get_i64("get_i64"); +Name get_f32("get_f32"); +Name get_f64("get_f64"); + +Name set_i32("set_i32"); +Name set_i64("set_i64"); +Name set_f32("set_f32"); +Name set_f64("set_f64"); + +struct InstrumentLocals : public WalkerPass> { + void visitGetLocal(GetLocal* curr) { + Builder builder(*getModule()); + Name import; + switch (curr->type) { + case i32: import = get_i32; break; + case i64: return; // TODO + case f32: import = get_f32; break; + case f64: import = get_f64; break; + default: WASM_UNREACHABLE(); + } + replaceCurrent( + builder.makeCallImport( + import, + { + builder.makeConst(Literal(int32_t(id++))), + builder.makeConst(Literal(int32_t(curr->index))), + curr + }, + curr->type + ) + ); + } + + void visitSetLocal(SetLocal* curr) { + Builder builder(*getModule()); + Name import; + switch (curr->value->type) { + case i32: import = set_i32; break; + case i64: return; // TODO + case f32: import = set_f32; break; + case f64: import = set_f64; break; + case unreachable: return; // nothing to do here + default: WASM_UNREACHABLE(); + } + curr->value = builder.makeCallImport( + import, + { + builder.makeConst(Literal(int32_t(id++))), + builder.makeConst(Literal(int32_t(curr->index))), + curr->value + }, + curr->value->type + ); + } + + void visitModule(Module* curr) { + addImport(curr, get_i32, "iiii"); + addImport(curr, get_i64, "jiij"); + addImport(curr, get_f32, "fiif"); + addImport(curr, get_f64, "diid"); + addImport(curr, set_i32, "iiii"); + addImport(curr, set_i64, "jiij"); + addImport(curr, set_f32, "fiif"); + addImport(curr, set_f64, "diid"); + } + +private: + Index id = 0; + + void addImport(Module* wasm, Name name, std::string sig) { + auto import = new Import; + import->name = name; + import->module = INSTRUMENT; + import->base = name; + import->functionType = ensureFunctionType(sig, wasm)->name; + import->kind = ExternalKind::Function; + wasm->addImport(import); + } +}; + +Pass* createInstrumentLocalsPass() { + return new InstrumentLocals(); +} + +} // namespace wasm diff --git a/src/passes/pass.cpp b/src/passes/pass.cpp index acc5f17bf51..f494378cb43 100644 --- a/src/passes/pass.cpp +++ b/src/passes/pass.cpp @@ -75,6 +75,7 @@ void PassRegistry::registerPasses() { registerPass("legalize-js-interface", "legalizes i64 types on the import/export boundary", createLegalizeJSInterfacePass); registerPass("local-cse", "common subexpression elimination inside basic blocks", createLocalCSEPass); registerPass("log-execution", "instrument the build with logging of where execution goes", createLogExecutionPass); + registerPass("instrument-locals", "instrument the build with code to intercept all loads and stores", createInstrumentLocalsPass); registerPass("instrument-memory", "instrument the build with code to intercept all loads and stores", createInstrumentMemoryPass); registerPass("memory-packing", "packs memory into separate segments, skipping zeros", createMemoryPackingPass); registerPass("merge-blocks", "merges blocks to their parents", createMergeBlocksPass); diff --git a/src/passes/passes.h b/src/passes/passes.h index 806ce0c2b18..e1ba286ad44 100644 --- a/src/passes/passes.h +++ b/src/passes/passes.h @@ -34,6 +34,7 @@ Pass *createInliningPass(); Pass *createLegalizeJSInterfacePass(); Pass *createLocalCSEPass(); Pass *createLogExecutionPass(); +Pass *createInstrumentLocalsPass(); Pass *createInstrumentMemoryPass(); Pass *createMemoryPackingPass(); Pass *createMergeBlocksPass(); diff --git a/test/passes/instrument-locals.txt b/test/passes/instrument-locals.txt new file mode 100644 index 00000000000..5cc8a98fefc --- /dev/null +++ b/test/passes/instrument-locals.txt @@ -0,0 +1,118 @@ +(module + (type $0 (func)) + (type $FUNCSIG$iiii (func (param i32 i32 i32) (result i32))) + (type $FUNCSIG$jiij (func (param i32 i32 i64) (result i64))) + (type $FUNCSIG$fiif (func (param i32 i32 f32) (result f32))) + (type $FUNCSIG$diid (func (param i32 i32 f64) (result f64))) + (import "instrument" "get_i32" (func $get_i32 (param i32 i32 i32) (result i32))) + (import "instrument" "get_i64" (func $get_i64 (param i32 i32 i64) (result i64))) + (import "instrument" "get_f32" (func $get_f32 (param i32 i32 f32) (result f32))) + (import "instrument" "get_f64" (func $get_f64 (param i32 i32 f64) (result f64))) + (import "instrument" "set_i32" (func $set_i32 (param i32 i32 i32) (result i32))) + (import "instrument" "set_i64" (func $set_i64 (param i32 i32 i64) (result i64))) + (import "instrument" "set_f32" (func $set_f32 (param i32 i32 f32) (result f32))) + (import "instrument" "set_f64" (func $set_f64 (param i32 i32 f64) (result f64))) + (memory $0 0) + (func $A (type $0) + (local $x i32) + (local $y i64) + (local $z f32) + (local $w f64) + (drop + (call $get_i32 + (i32.const 0) + (i32.const 0) + (get_local $x) + ) + ) + (drop + (get_local $y) + ) + (drop + (call $get_f32 + (i32.const 1) + (i32.const 2) + (get_local $z) + ) + ) + (drop + (call $get_f64 + (i32.const 2) + (i32.const 3) + (get_local $w) + ) + ) + (drop + (call $get_i32 + (i32.const 3) + (i32.const 0) + (get_local $x) + ) + ) + (drop + (get_local $y) + ) + (drop + (call $get_f32 + (i32.const 4) + (i32.const 2) + (get_local $z) + ) + ) + (drop + (call $get_f64 + (i32.const 5) + (i32.const 3) + (get_local $w) + ) + ) + (set_local $x + (call $set_i32 + (i32.const 6) + (i32.const 0) + (i32.const 1) + ) + ) + (set_local $y + (i64.const 2) + ) + (set_local $z + (call $set_f32 + (i32.const 7) + (i32.const 2) + (f32.const 3.2100000381469727) + ) + ) + (set_local $w + (call $set_f64 + (i32.const 8) + (i32.const 3) + (f64.const 4.321) + ) + ) + (set_local $x + (call $set_i32 + (i32.const 9) + (i32.const 0) + (i32.const 11) + ) + ) + (set_local $y + (i64.const 22) + ) + (set_local $z + (call $set_f32 + (i32.const 10) + (i32.const 2) + (f32.const 33.209999084472656) + ) + ) + (set_local $w + (call $set_f64 + (i32.const 11) + (i32.const 3) + (f64.const 44.321) + ) + ) + ) +) diff --git a/test/passes/instrument-locals.wast b/test/passes/instrument-locals.wast new file mode 100644 index 00000000000..b7e34c6d6dc --- /dev/null +++ b/test/passes/instrument-locals.wast @@ -0,0 +1,29 @@ +(module + (func $A + (local $x i32) + (local $y i64) + (local $z f32) + (local $w f64) + + (drop (get_local $x)) + (drop (get_local $y)) + (drop (get_local $z)) + (drop (get_local $w)) + + (drop (get_local $x)) + (drop (get_local $y)) + (drop (get_local $z)) + (drop (get_local $w)) + + (set_local $x (i32.const 1)) + (set_local $y (i64.const 2)) + (set_local $z (f32.const 3.21)) + (set_local $w (f64.const 4.321)) + + (set_local $x (i32.const 11)) + (set_local $y (i64.const 22)) + (set_local $z (f32.const 33.21)) + (set_local $w (f64.const 44.321)) + ) +) + From 8f245f88d4a9553e8c3f2a512e3779664935a22b Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Thu, 8 Jun 2017 11:54:52 -0700 Subject: [PATCH 34/44] refactor --- src/passes/SSAify.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index a4a8f829360..3dee1ea53bd 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -140,13 +140,11 @@ struct SSAify : public WalkerPass> { // we can detect this as follows: if a get of oldIndex has the same sets // as the sets at the entrance to the loop, then it is affected by the loop // header sets, and we can add to there sets that looped back - auto& gets = loopGetStack.back(); - for (auto* get : gets) { - auto& beforeSets = before[get->index]; - auto& getSets = getSetses[get]; + auto linkLoopTop = [&](Index i, Sets& getSets) { + auto& beforeSets = before[i]; if (getSets.size() < beforeSets.size()) { // the get trivially has fewer sets, so it overrode the loop entry sets - continue; + return; } std::vector intersection; std::set_intersection(beforeSets.begin(), beforeSets.end(), @@ -154,12 +152,16 @@ struct SSAify : public WalkerPass> { std::back_inserter(intersection)); if (intersection.size() < beforeSets.size()) { // the get has not the same sets as in the loop entry - continue; + return; } // the get has the entry sets, so add any new ones - for (auto* set : merged[get->index]) { + for (auto* set : merged[i]) { getSets.insert(set); } + }; + auto& gets = loopGetStack.back(); + for (auto* get : gets) { + linkLoopTop(get->index, getSetses[get]); } } mappingStack.pop_back(); From 5a7347cdb590bc74f29eafac4f8974400bc40d65 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Thu, 8 Jun 2017 11:57:01 -0700 Subject: [PATCH 35/44] add testcase showing a problem --- test/passes/ssa.txt | 42 ++++++++++++++++++++++++++++++++++++++++++ test/passes/ssa.wast | 23 +++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 69a9f94622b..304b6e60c55 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -467,4 +467,46 @@ ) ) ) + (func $loop-loop-param (type $0) (param $param i32) + (local $1 i32) + (local $2 i32) + (local $3 i32) + (local $4 i32) + (set_local $3 + (get_local $param) + ) + (set_local $4 + (get_local $param) + ) + (block + (loop $loop1 + (block $out1 + (if + (get_local $3) + (br $out1) + ) + (set_local $1 + (tee_local $3 + (i32.const 1) + ) + ) + (br $loop1) + ) + ) + (loop $loop2 + (block $out2 + (if + (get_local $4) + (br $out2) + ) + (set_local $2 + (tee_local $4 + (i32.const 2) + ) + ) + (br $loop2) + ) + ) + ) + ) ) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index 27581f3aac7..307ba615977 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -193,5 +193,28 @@ ) ) ) + (func $loop-loop-param + (param $param i32) + (loop $loop1 + (block $out1 + (if + (get_local $param) + (br $out1) + ) + (set_local $param (i32.const 1)) + (br $loop1) + ) + ) + (loop $loop2 + (block $out2 + (if + (get_local $param) + (br $out2) + ) + (set_local $param (i32.const 2)) + (br $loop2) + ) + ) + ) ) From 1b4f92ab5b94f90ec6ef957b4c9d3f68b879666e Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Thu, 8 Jun 2017 12:04:25 -0700 Subject: [PATCH 36/44] merge loop join into loop fallthrough --- src/passes/SSAify.cpp | 5 +++++ test/passes/ssa.txt | 6 ++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 3dee1ea53bd..f7c7318a03f 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -163,6 +163,11 @@ struct SSAify : public WalkerPass> { for (auto* get : gets) { linkLoopTop(get->index, getSetses[get]); } + // and the same for the loop fallthrough: any local that still has the + // entry sets should also have the loop-back sets as well + for (Index i = 0; i < numLocals; i++) { + linkLoopTop(i, currMapping[i]); + } } mappingStack.pop_back(); loopGetStack.pop_back(); diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 304b6e60c55..9c2f3cdbff1 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -486,8 +486,10 @@ (br $out1) ) (set_local $1 - (tee_local $3 - (i32.const 1) + (tee_local $4 + (tee_local $3 + (i32.const 1) + ) ) ) (br $loop1) From 23dad7d7d49d8001628fe33ffe1d5ce114cfbd49 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Thu, 8 Jun 2017 12:56:17 -0700 Subject: [PATCH 37/44] add testcase --- test/passes/ssa.txt | 33 +++++++++++++++++++++++++++++++++ test/passes/ssa.wast | 23 +++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 9c2f3cdbff1..911addbc7b2 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -511,4 +511,37 @@ ) ) ) + (func $loop-loop-param-nomerge (type $0) (param $param i32) + (local $1 i32) + (local $2 i32) + (local $3 i32) + (loop $loop1 + (block $out1 + (set_local $1 + (tee_local $3 + (i32.const 1) + ) + ) + (if + (get_local $1) + (br $out1) + ) + (br $loop1) + ) + ) + (loop $loop2 + (block $out2 + (if + (get_local $3) + (br $out2) + ) + (set_local $2 + (tee_local $3 + (i32.const 2) + ) + ) + (br $loop2) + ) + ) + ) ) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index 307ba615977..53dd44812c0 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -216,5 +216,28 @@ ) ) ) + (func $loop-loop-param-nomerge + (param $param i32) + (loop $loop1 + (block $out1 + (set_local $param (i32.const 1)) + (if + (get_local $param) + (br $out1) + ) + (br $loop1) + ) + ) + (loop $loop2 + (block $out2 + (if + (get_local $param) + (br $out2) + ) + (set_local $param (i32.const 2)) + (br $loop2) + ) + ) + ) ) From e6dbb10e47b26ec79c6d60e10a01bce8dc8177ff Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Thu, 8 Jun 2017 18:57:02 -0700 Subject: [PATCH 38/44] add failing testcase --- test/passes/ssa.wast | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index 53dd44812c0..b154a397224 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -239,5 +239,49 @@ ) ) ) + (func $loop-nesting + (param $x i32) + (block $out + (loop $loop1 + (if + (get_local $x) + (br $out) + ) + (loop $loop2 + (if + (get_local $x) + (br $out) + ) + (set_local $x (i32.const 1)) + (br $loop2) + ) + (set_local $x (i32.const 2)) + (br $loop1) + ) + ) + (drop (get_local $x)) ;; can receive from either set, or input param + ) + (func $loop-nesting-2 + (param $x i32) + (block $out + (loop $loop1 + (if + (get_local $x) + (br $out) + ) + (loop $loop2 + (if + (get_local $x) + (br $out) + ) + (set_local $x (i32.const 1)) + (br_if $loop2 (i32.const 3)) ;; add fallthrough + ) + (set_local $x (i32.const 2)) + (br $loop1) + ) + ) + (drop (get_local $x)) ;; can receive from either set, or input param + ) ) From 9dbbca3b4261a0ce6f0272ea51adbd0b189b0ede Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Fri, 9 Jun 2017 10:39:22 -0700 Subject: [PATCH 39/44] add another bad testcase --- test/passes/ssa.wast | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast index b154a397224..fe78aecb52b 100644 --- a/test/passes/ssa.wast +++ b/test/passes/ssa.wast @@ -192,6 +192,34 @@ (br $more) ) ) + (drop (get_local $loopvar)) + ) + (func $real-loop-outblock + (param $param i32) + (local $loopvar i32) + (local $inc i32) + (set_local $loopvar + (get_local $param) + ) + (block $stop + (loop $more + (if + (i32.const 1) + (br $stop) + ) + (set_local $inc + (i32.add + (get_local $loopvar) ;; this var should be written to from before the loop and the inc at the end + (i32.const 1) + ) + ) + (set_local $loopvar + (get_local $inc) + ) + (br $more) + ) + ) + (drop (get_local $loopvar)) ) (func $loop-loop-param (param $param i32) From 6e47df8e9c7655ab08f3222d403c4d32766406b3 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Fri, 9 Jun 2017 10:48:13 -0700 Subject: [PATCH 40/44] fix ssa break loop updating --- src/passes/SSAify.cpp | 14 ++++ test/passes/ssa.txt | 147 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 161 insertions(+) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index f7c7318a03f..2692b9ddb95 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -95,6 +95,7 @@ struct SSAify : public WalkerPass> { auto& infos = breakMappings[curr->name]; infos.emplace_back(std::move(currMapping)); currMapping = std::move(merge(infos)); + breakMappings.erase(curr->name); } } @@ -168,6 +169,19 @@ struct SSAify : public WalkerPass> { for (Index i = 0; i < numLocals; i++) { linkLoopTop(i, currMapping[i]); } + // finally, breaks still in flight must be updated too + for (auto& iter : breakMappings) { + auto name = iter.first; + if (name == curr->name) continue; // skip our own (which is still in use) + auto& mappings = iter.second; + for (auto& mapping : mappings) { + for (Index i = 0; i < numLocals; i++) { + linkLoopTop(i, mapping[i]); + } + } + } + // now that we are done with using the mappings, erase our own + breakMappings.erase(curr->name); } mappingStack.pop_back(); loopGetStack.pop_back(); diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt index 911addbc7b2..3d410057c6a 100644 --- a/test/passes/ssa.txt +++ b/test/passes/ssa.txt @@ -466,6 +466,45 @@ (br $more) ) ) + (drop + (get_local $6) + ) + ) + (func $real-loop-outblock (type $0) (param $param i32) + (local $loopvar i32) + (local $inc i32) + (local $3 i32) + (local $4 i32) + (local $5 i32) + (local $6 i32) + (set_local $3 + (tee_local $6 + (get_local $param) + ) + ) + (block $stop + (loop $more + (if + (i32.const 1) + (br $stop) + ) + (set_local $4 + (i32.add + (get_local $6) + (i32.const 1) + ) + ) + (set_local $5 + (tee_local $6 + (get_local $4) + ) + ) + (br $more) + ) + ) + (drop + (get_local $6) + ) ) (func $loop-loop-param (type $0) (param $param i32) (local $1 i32) @@ -544,4 +583,112 @@ ) ) ) + (func $loop-nesting (type $0) (param $x i32) + (local $1 i32) + (local $2 i32) + (local $3 i32) + (local $4 i32) + (local $5 i32) + (set_local $3 + (get_local $x) + ) + (set_local $4 + (get_local $x) + ) + (set_local $5 + (get_local $x) + ) + (block + (block $out + (loop $loop1 + (if + (get_local $3) + (br $out) + ) + (loop $loop2 + (if + (get_local $4) + (br $out) + ) + (set_local $1 + (tee_local $5 + (tee_local $4 + (i32.const 1) + ) + ) + ) + (br $loop2) + ) + (set_local $2 + (tee_local $5 + (tee_local $4 + (tee_local $3 + (i32.const 2) + ) + ) + ) + ) + (br $loop1) + ) + ) + (drop + (get_local $5) + ) + ) + ) + (func $loop-nesting-2 (type $0) (param $x i32) + (local $1 i32) + (local $2 i32) + (local $3 i32) + (local $4 i32) + (local $5 i32) + (set_local $3 + (get_local $x) + ) + (set_local $4 + (get_local $x) + ) + (set_local $5 + (get_local $x) + ) + (block + (block $out + (loop $loop1 + (if + (get_local $3) + (br $out) + ) + (loop $loop2 + (if + (get_local $4) + (br $out) + ) + (set_local $1 + (tee_local $5 + (tee_local $4 + (i32.const 1) + ) + ) + ) + (br_if $loop2 + (i32.const 3) + ) + ) + (set_local $2 + (tee_local $5 + (tee_local $4 + (tee_local $3 + (i32.const 2) + ) + ) + ) + ) + (br $loop1) + ) + ) + (drop + (get_local $5) + ) + ) + ) ) From d24ef3f48633413edf3d54ad428abb3e5380b564 Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Fri, 9 Jun 2017 11:18:44 -0700 Subject: [PATCH 41/44] add function name validation to asm2wasm --- src/asm2wasm.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/asm2wasm.h b/src/asm2wasm.h index b8439bdbf8d..f6d0443ec37 100644 --- a/src/asm2wasm.h +++ b/src/asm2wasm.h @@ -1162,6 +1162,9 @@ void Asm2WasmBuilder::processAsm(Ref ast) { } else if (curr[0] == DEFUN) { // function auto* func = processFunction(curr); + if (wasm.getFunctionOrNull(func->name)) { + Fatal() << "duplicate function: " << func->name; + } if (runOptimizationPasses) { optimizingBuilder->addFunction(func); } else { From 6678b59e9f77b02d1b10f1104702621c2762df9a Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Fri, 9 Jun 2017 17:06:33 -0700 Subject: [PATCH 42/44] do not optimize tees in pick-load-signs, it is invalid to do so --- src/passes/PickLoadSigns.cpp | 4 ++++ test/passes/pick-load-signs.txt | 16 ++++++++++++++++ test/passes/pick-load-signs.wast | 16 ++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/src/passes/PickLoadSigns.cpp b/src/passes/PickLoadSigns.cpp index 6ead0cc4e98..d7947960ce2 100644 --- a/src/passes/PickLoadSigns.cpp +++ b/src/passes/PickLoadSigns.cpp @@ -94,6 +94,10 @@ struct PickLoadSigns : public WalkerPass> { } void visitSetLocal(SetLocal* curr) { + if (curr->isTee()) { + // we can't modify a tee, the value is used elsewhere + return; + } if (auto* load = curr->value->dynCast()) { loads[load] = curr->index; } diff --git a/test/passes/pick-load-signs.txt b/test/passes/pick-load-signs.txt index 707bb50e928..e8507fa29ab 100644 --- a/test/passes/pick-load-signs.txt +++ b/test/passes/pick-load-signs.txt @@ -260,4 +260,20 @@ (i32.const 1024) ) ) + (func $tees (type $0) + (local $y i32) + (drop + (tee_local $y + (i32.load8_s + (i32.const 1024) + ) + ) + ) + (drop + (i32.and + (get_local $y) + (i32.const 255) + ) + ) + ) ) diff --git a/test/passes/pick-load-signs.wast b/test/passes/pick-load-signs.wast index 49105e497a9..33f814926bb 100644 --- a/test/passes/pick-load-signs.wast +++ b/test/passes/pick-load-signs.wast @@ -257,4 +257,20 @@ (i32.const 1024) ) ) + (func $tees + (local $y i32) + (drop ;; a "use", so we can't alter the value + (tee_local $y + (i32.load8_s + (i32.const 1024) + ) + ) + ) + (drop + (i32.and + (get_local $y) + (i32.const 255) + ) + ) + ) ) From 00c6fd077dedc0d8f9c1241a2e2f9095f51da19f Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Sat, 10 Jun 2017 11:17:52 -0700 Subject: [PATCH 43/44] add include --- src/passes/SSAify.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 2692b9ddb95..25fc7fc8b6b 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -23,6 +23,8 @@ // can utilize // +#include + #include "wasm.h" #include "pass.h" #include "wasm-builder.h" From 67bc5c3b742131260d179f795a3264159041bffe Mon Sep 17 00:00:00 2001 From: "Alon Zakai (kripken)" Date: Sat, 10 Jun 2017 11:54:05 -0700 Subject: [PATCH 44/44] comment --- src/passes/SSAify.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp index 25fc7fc8b6b..9c9aeb573ab 100644 --- a/src/passes/SSAify.cpp +++ b/src/passes/SSAify.cpp @@ -16,9 +16,14 @@ // // Transforms code into SSA form. That ensures each variable has a -// single assignment. For phis, we do not add a new node to the AST, -// so the result is multiple assignments but with the guarantee that -// they all lead to the same get_local +// single assignment. +// +// Note that "SSA form" usually means SSA + phis. This pass does not +// create phis, we still emit something in our AST, which does not +// have a phi instruction. What we emit when control flow joins +// require more than one input to a value is multiple assignments +// to the same local, with the SSA guarantee that one and only one +// of those assignments will arrive at the uses of that "merge local". // TODO: consider adding a "proper" phi node to the AST, that passes // can utilize //