From a8da89653981bb37565981afc69104f918fea64a Mon Sep 17 00:00:00 2001
From: Alon Zakai <alonzakai@gmail.com>
Date: Wed, 11 Oct 2017 10:13:32 -0700
Subject: [PATCH] fix ssaify bug where we failed to update the location of
 values as we moved them around, causing us to zero out the wrong thing in
 another place and ensuing hilarity (#1212)

---
 src/ast/LocalGraph.cpp | 13 +++++++++++++
 src/passes/SSAify.cpp  | 18 ++++++++++++------
 test/passes/ssa.txt    | 36 ++++++++++++++++++++++++++++++++++++
 test/passes/ssa.wast   | 28 ++++++++++++++++++++++++++++
 4 files changed, 89 insertions(+), 6 deletions(-)

diff --git a/src/ast/LocalGraph.cpp b/src/ast/LocalGraph.cpp
index c997eff1b16..0d36fec84eb 100644
--- a/src/ast/LocalGraph.cpp
+++ b/src/ast/LocalGraph.cpp
@@ -17,6 +17,7 @@
 #include <iterator>
 
 #include <wasm-builder.h>
+#include <wasm-printing.h>
 #include <ast/find_all.h>
 #include <ast/local-graph.h>
 
@@ -24,6 +25,18 @@ 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() {
diff --git a/src/passes/SSAify.cpp b/src/passes/SSAify.cpp
index a71a75308be..aafea8a973a 100644
--- a/src/passes/SSAify.cpp
+++ b/src/passes/SSAify.cpp
@@ -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();
@@ -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;
@@ -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;
@@ -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)) {
diff --git a/test/passes/ssa.txt b/test/passes/ssa.txt
index d3f9140afec..916fed54adf 100644
--- a/test/passes/ssa.txt
+++ b/test/passes/ssa.txt
@@ -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)
@@ -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)
+  )
+ )
 )
diff --git a/test/passes/ssa.wast b/test/passes/ssa.wast
index fe78aecb52b..e51189b5eb0 100644
--- a/test/passes/ssa.wast
+++ b/test/passes/ssa.wast
@@ -1,4 +1,5 @@
 (module
+  (global $global$0 (mut i32) (i32.const 1))
   (func $basics (param $x i32)
     (local $y i32)
     (local $z f32)
@@ -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
+   )
+  )
 )