Skip to content

Commit

Permalink
fix ssaify bug where we failed to update the location of values as we…
Browse files Browse the repository at this point in the history
… moved them around, causing us to zero out the wrong thing in another place and ensuing hilarity (#1212)
  • Loading branch information
kripken committed Oct 11, 2017
1 parent f0d858b commit a8da896
Show file tree
Hide file tree
Showing 4 changed files with 89 additions and 6 deletions.
13 changes: 13 additions & 0 deletions src/ast/LocalGraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,26 @@
#include <iterator>

#include <wasm-builder.h>
#include <wasm-printing.h>
#include <ast/find_all.h>
#include <ast/local-graph.h>

namespace wasm {

LocalGraph::LocalGraph(Function* func, Module* module) {
walkFunctionInModule(func, module);

#ifdef LOCAL_GRAPH_DEBUG
std::cout << "LocalGraph::dump\n";
for (auto& pair : getSetses) {
auto* get = pair.first;
auto& sets = pair.second;
std::cout << "GET\n" << get << " is influenced by\n";
for (auto* set : sets) {
std::cout << set << '\n';
}
}
#endif
}

void LocalGraph::computeInfluences() {
Expand Down
18 changes: 12 additions & 6 deletions src/passes/SSAify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ struct SSAify : public Pass {
LocalGraph graph(func, module);
// create new local indexes, one for each set
createNewIndexes(graph);
// we now know the sets for each get
// we now know the sets for each get, and can compute get indexes and handle phis
computeGetsAndPhis(graph);
// add prepends to function
addPrepends();
Expand All @@ -75,7 +75,6 @@ struct SSAify : public Pass {
}
}

// After we traversed it all, we can compute gets and phis
void computeGetsAndPhis(LocalGraph& graph) {
for (auto& iter : graph.getSetses) {
auto* get = iter.first;
Expand All @@ -84,8 +83,7 @@ struct SSAify : public Pass {
continue; // unreachable, ignore
}
if (sets.size() == 1) {
// TODO: add tests for this case
// easy, just one set, use it's index
// easy, just one set, use its index
auto* set = *sets.begin();
if (set) {
get->index = set->index;
Expand Down Expand Up @@ -138,10 +136,18 @@ struct SSAify : public Pass {
for (auto* set : sets) {
if (set) {
// a set exists, just add a tee of its value
set->value = builder.makeTeeLocal(
auto* value = set->value;
auto* tee = builder.makeTeeLocal(
new_,
set->value
value
);
set->value = tee;
// the value may have been something we tracked the location
// of. if so, update that, since we moved it into the tee
if (graph.locations.count(value) > 0) {
assert(graph.locations[value] == &set->value);
graph.locations[value] = &tee->value;
}
} else {
// this is a param or the zero init value.
if (func->isParam(old)) {
Expand Down
36 changes: 36 additions & 0 deletions test/passes/ssa.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
(module
(type $0 (func (param i32)))
(type $1 (func))
(type $2 (func (result i32)))
(global $global$0 (mut i32) (i32.const 1))
(memory $0 0)
(func $basics (type $0) (param $x i32)
(local $y i32)
Expand Down Expand Up @@ -691,4 +693,38 @@
)
)
)
(func $func_6 (type $2) (result i32)
(local $result i32)
(local $zero i32)
(local $2 i32)
(local $3 i32)
(local $4 i32)
(loop $label$1
(if
(i32.eqz
(get_global $global$0)
)
(return
(get_local $4)
)
)
(set_global $global$0
(i32.const 0)
)
(set_local $2
(tee_local $4
(i32.const 1)
)
)
(br_if $label$1
(i32.const 0)
)
(set_local $3
(tee_local $4
(i32.const 0)
)
)
(br $label$1)
)
)
)
28 changes: 28 additions & 0 deletions test/passes/ssa.wast
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
(module
(global $global$0 (mut i32) (i32.const 1))
(func $basics (param $x i32)
(local $y i32)
(local $z f32)
Expand Down Expand Up @@ -311,5 +312,32 @@
)
(drop (get_local $x)) ;; can receive from either set, or input param
)
(func $func_6 (result i32)
(local $result i32)
(local $zero i32)
(loop $label$1
(if
(i32.eqz
(get_global $global$0)
)
(return
(get_local $result) ;; we eventually reach here
)
)
(set_global $global$0
(i32.const 0) ;; tell next iteration to return
)
(set_local $result
(i32.const 1) ;; set the return value to 1, temporarily
)
(br_if $label$1
(i32.const 0) ;; don't do anything here
)
(set_local $result
(get_local $zero) ;; set it to zero instead
)
(br $label$1) ;; back to the top, where we will return the zero
)
)
)

0 comments on commit a8da896

Please sign in to comment.