[SSA] Fix bug in to_ssa.py

Fixes Issue sampsyo#108.

The optimization in prune_phis() is not sound. phi nodes that are
"partially undefined" cannot be removed. It is only illegal to read
from the result if the second to last executed label corresponds to
an undefined argument. `to_ssa.py` generated incorrect code for the
two tests added, before the fix.

In most cases, these phi nodes aren't read from, and will be removed
by DCE. However, in the case where the phi nodes are read, a correct
optimization would be:
1. Detect partially undefined phi nodes whose value is always used.
   Let `undefined_labels` be the set of labels whose argument is
   undefined.
2. Leverage the undefined behavior to say that `undefined_labels`
   are in fact not predecessors to the basic block.
   `preds = preds - undefined_labels`.
Though, I'm not sure how useful that optimization would be in
practice.

examples/test/to_ssa/if-const.bril

@@ -0,0 +1,14 @@
+@main() {
+    cond: bool = const true;
+    br cond .true .false;
+.true:
+    a: int = const 0;
+    jmp .zexit;
+.false:
+    b: int = const 1;
+    jmp .zexit;
+# zexit to trigger a bug in to_ssa.py that depends on
+# the order that basic blocks get renamed.
+.zexit:
+    print a;
+}

examples/test/to_ssa/if-const.out

@@ -0,0 +1,16 @@
+@main {
+.b1:
+  cond.0: bool = const true;
+  br cond.0 .true .false;
+.true:
+  a.0: int = const 0;
+  jmp .zexit;
+.false:
+  b.0: int = const 1;
+  jmp .zexit;
+.zexit:
+  b.1: int = phi b.0 b.0 .false .true;
+  a.1: int = phi __undefined a.0 .false .true;
+  print a.1;
+  ret;
+}

examples/test/to_ssa/if-ssa.bril

@@ -0,0 +1,16 @@
+@main(cond: bool) {
+.entry:
+    a.1: int = const 47;
+    br cond .left .right;
+.left:
+    a.2: int = add a.1 a.1;
+    jmp .zexit;
+.right:
+    a.3: int = mul a.1 a.1;
+    jmp .zexit;
+# zexit to trigger a bug in to_ssa.py that depends on
+# the order that basic blocks get renamed.
+.zexit:
+    a.4: int = phi .left a.2 .right a.3;
+    print a.4;
+}

examples/test/to_ssa/if-ssa.out

@@ -0,0 +1,17 @@
+@main(cond: bool) {
+.entry:
+  a.1.0: int = const 47;
+  br cond .left .right;
+.left:
+  a.2.0: int = add a.1.0 a.1.0;
+  jmp .zexit;
+.right:
+  a.3.0: int = mul a.1.0 a.1.0;
+  jmp .zexit;
+.zexit:
+  a.3.1: int = phi __undefined a.3.0 .left .right;
+  a.2.1: int = phi a.2.0 a.2.0 .left .right;
+  a.4.0: int = phi a.2.1 a.3.1 .left .right;
+  print a.4.0;
+  ret;
+}

examples/test/to_ssa/loop.out

@@ -3,7 +3,9 @@
   i.0: int = const 1;
   jmp .loop;
 .loop:
+  max.0: int = phi __undefined max.1 .entry .body;
   i.1: int = phi i.0 i.2 .entry .body;
+  cond.0: bool = phi __undefined cond.1 .entry .body;
   max.1: int = const 10;
   cond.1: bool = lt i.1 max.1;
   br cond.1 .body .exit;

examples/test/to_ssa/selfloop.out

@@ -6,6 +6,7 @@
   jmp .loop;
 .loop:
   x.1: int = phi x.0 x.2 .entry .br;
+  done.0: bool = phi __undefined done.1 .entry .br;
   x.2: int = sub x.1 one.0;
   done.1: bool = eq x.2 zero.0;
   jmp .br;

examples/test/to_ssa/while.out

@@ -2,6 +2,9 @@
 .entry1:
   jmp .while.cond;
 .while.cond:
+  zero.0: int = phi __undefined zero.1 .entry1 .while.body;
+  one.0: int = phi __undefined one.1 .entry1 .while.body;
+  is_term.0: bool = phi __undefined is_term.1 .entry1 .while.body;
   a.0: int = phi a a.1 .entry1 .while.body;
   zero.1: int = const 0;
   is_term.1: bool = eq a.0 zero.1;

examples/to_ssa.py

@@ -74,6 +74,9 @@ def _rename(block):
             for p in phis[s]:
                 if stack[p]:
                     phi_args[s][p].append((block, stack[p][0]))
+                else:
+                    # The variable is not defined on this path
+                    phi_args[s][p].append((block, "__undefined"))
 
         # Recursive calls.
         for b in sorted(domtree[block]):
@@ -88,43 +91,6 @@ def _rename(block):
     return phi_args, phi_dests
 
 
-def prune_phis(pred, phi_args, phi_dests):
-    """Prune possibly-undefined phi-nodes.
-
-    Ordinary phi insertion will create phi-nodes that are "partially
-    undefined" because they represent a convergence of paths where the
-    variable is defined along some but not all paths. These phi-nodes
-    are useless because it is illegal to read from the result. And they
-    can confuse the out-of-SSA pass because it creates nonsensical
-    copies. This algorithm iteratively eliminates such phi-nodes,
-    propagating through to eliminate consumer phi-nodes until
-    convergence.
-    """
-    # We build up a set of new names (phi destinations) to prune, and we
-    # iterate until this set stops growing.
-    old_prune_len = -1
-    prune = set()
-    while len(prune) != old_prune_len:
-        old_prune_len = len(prune)
-
-        # Look at each phi.
-        for block, args in phi_args.items():
-            dests = phi_dests[block]
-            for v, v_args in args.items():
-                # How many non-pruned arguments does this phi have?
-                live_args = [a for _, a in v_args if a not in prune]
-                if len(live_args) < len(pred[block]):
-                    # Prune phis with insufficient arguments.
-                    prune.add(dests[v])
-
-    # Actually delete all phis with pruned destinations.
-    for block, args in phi_args.items():
-        dests = phi_dests[block]
-        to_del = {v for v, d in dests.items() if d in prune}
-        for v in to_del:
-            del args[v]
-            del dests[v]
-
 
 def insert_phis(blocks, phi_args, phi_dests, types):
     for block, instrs in blocks.items():
@@ -166,7 +132,6 @@ def func_to_ssa(func):
     phis = get_phis(blocks, df, defs)
     phi_args, phi_dests = ssa_rename(blocks, phis, succ, dom_tree(dom),
                                      arg_names)
-    prune_phis(pred, phi_args, phi_dests)
     insert_phis(blocks, phi_args, phi_dests, types)
 
     func['instrs'] = reassemble(blocks)