fix[venom]: fix MakeSSA with existing phis

tests/functional/venom/parser/test_parsing.py

@@ -1,4 +1,4 @@
-from tests.venom_utils import assert_ctx_eq
+from tests.venom_utils import assert_bb_eq, assert_ctx_eq
 from vyper.venom.basicblock import IRBasicBlock, IRLabel, IRLiteral, IRVariable
 from vyper.venom.context import DataItem, DataSection, IRContext
 from vyper.venom.function import IRFunction
@@ -231,3 +231,122 @@
     ]
 
     assert_ctx_eq(parsed_ctx, expected_ctx)
+
+
+def test_phis():
+    # @external
+    # def _loop() -> uint256:
+    #    res: uint256 = 9
+    #    for i: uint256 in range(res, bound=10):
+    #        res = res + i
+    # return res
+    source = """
+    function __main_entry {
+      __main_entry:  ; IN=[] OUT=[fallback, 1_then] => {}
+        %27 = 0
+        %1 = calldataload %27
+        %28 = %1
+        %29 = 224
+        %2 = shr %29, %28
+        %31 = %2
+        %30 = 1729138561
+        %4 = xor %30, %31
+        %32 = %4
+        jnz %32, @fallback, @1_then
+        ; (__main_entry)
+
+
+      1_then:  ; IN=[__main_entry] OUT=[4_condition] => {%11, %var8_0}
+        %6 = callvalue
+        %33 = %6
+        %7 = iszero %33
+        %34 = %7
+        assert %34
+        %var8_0 = 9
+        %11 = 0
+        nop
+        jmp @4_condition
+        ; (__main_entry)
+
+
+      4_condition:  ; IN=[1_then, 5_body] OUT=[5_body, 7_exit] => {%11:3, %var8_0:2}
+        %var8_0:2 = phi @1_then, %var8_0, @5_body, %var8_0:3
+        %11:3 = phi @1_then, %11, @5_body, %11:4
+        %35 = %11:3
+        %36 = 9
+        %15 = xor %36, %35
+        %37 = %15
+        jnz %37, @5_body, @7_exit
+        ; (__main_entry)
+
+
+      5_body:  ; IN=[4_condition] OUT=[4_condition] => {%11:4, %var8_0:3}
+        %38 = %11:3
+        %39 = %var8_0:2
+        %22 = add %39, %38
+        %41 = %22
+        %40 = %var8_0:2
+        %24 = gt %40, %41
+        %42 = %24
+        %25 = iszero %42
+        %43 = %25
+        assert %43
+        %var8_0:3 = %22
+        %44 = %11:3
+        %45 = 1
+        %11:4 = add %45, %44
+        jmp @4_condition
+        ; (__main_entry)
+
+
+      7_exit:  ; IN=[4_condition] OUT=[] => {}
+        %46 = %var8_0:2
+        %47 = 64
+        mstore %47, %46
+        %48 = 32
+        %49 = 64
+        return %49, %48
+        ; (__main_entry)
+
+
+      fallback:  ; IN=[__main_entry] OUT=[] => {}
+        %50 = 0
+        %51 = 0
+        revert %51, %50
+        stop
+        ; (__main_entry)
+    }  ; close function __main_entry
+    """
+    ctx = parse_venom(source)
+
+    expected_ctx = IRContext()
+    expected_ctx.add_function(entry_fn := IRFunction(IRLabel("__main_entry")))
+
+    expect_bb = IRBasicBlock(IRLabel("4_condition"), entry_fn)
+    entry_fn.append_basic_block(expect_bb)
+
+    expect_bb.append_instruction(
+        "phi",
+        IRLabel("1_then"),
+        IRVariable("%var8_0"),
+        IRLabel("5_body"),
+        IRVariable("%var8_0:3"),
+        ret=IRVariable("var8_0:2"),
+    )
+    expect_bb.append_instruction(
+        "phi",
+        IRLabel("1_then"),
+        IRVariable("%11"),
+        IRLabel("5_body"),
+        IRVariable("%11:4"),
+        ret=IRVariable("11:3"),
+    )
+    expect_bb.append_instruction("store", IRVariable("11:3"), ret=IRVariable("%35"))
+    expect_bb.append_instruction("store", IRLiteral(9), ret=IRVariable("%36"))
+    expect_bb.append_instruction("xor", IRVariable("%35"), IRVariable("%36"), ret=IRVariable("%15"))
+    expect_bb.append_instruction("store", IRVariable("%15"), ret=IRVariable("%37"))
+    expect_bb.append_instruction("jnz", IRVariable("%37"), IRLabel("5_body"), IRLabel("7_exit"))
+    # other basic blocks omitted for brevity
+
+    parsed_fn = next(iter(ctx.functions.values()))
+    assert_bb_eq(parsed_fn.get_basic_block(expect_bb.label.name), expect_bb)

tests/functional/venom/test_venom_repr.py

@@ -1,9 +1,12 @@
 import glob
 
+import textwrap
 import pytest
 
 from tests.venom_utils import assert_ctx_eq, parse_venom
 from vyper.compiler import compile_code
+from vyper.compiler.phases import generate_bytecode
+from vyper.venom import generate_assembly_experimental, run_passes_on
 from vyper.venom.context import IRContext
 
 """
@@ -16,15 +19,47 @@
 
 
 @pytest.mark.parametrize("vy_filename", get_example_vy_filenames())
-def test_round_trip(vy_filename, optimize, request):
+def test_round_trip_example(vy_filename, optimize):
+    """
+    Check all examples round trip
+    """
     path = f"examples/{vy_filename}"
     with open(path) as f:
         vyper_source = f.read()
 
-    out = compile_code(vyper_source, output_formats=["bb_runtime"])
+    _round_trip_helper(vyper_source, optimize)
+
+vyper_sources = ["""
+    @external
+    def _loop() -> uint256:
+        res: uint256 = 9
+        for i: uint256 in range(res, bound=10):
+            res = res + i
+        return res
+        """]
+
+@pytest.mark.parametrize("vyper_source", vyper_sources)
+def test_round_trip_source(vyper_source, optimize):
+    vyper_source = textwrap.dedent(vyper_source)
+    _round_trip_helper(vyper_source, optimize)
+
+
+def _round_trip_helper(vyper_source, optimize):
+    out = compile_code(vyper_source, output_formats=["bb_runtime", "bytecode_runtime"])
     bb_runtime = out["bb_runtime"]
     venom_code = IRContext.__repr__(bb_runtime)
 
     ctx = parse_venom(venom_code)
 
     assert_ctx_eq(bb_runtime, ctx)
+
+    # check it's valid to run venom passes+analyses
+    run_passes_on(ctx, optimize)
+
+    asm = generate_assembly_experimental(ctx)
+    bytecode = generate_bytecode(asm, compiler_metadata=None)
+    bytecode = f"0x{bytecode.hex()}"
+
+    # TODO investigate: bytecodes should be equal (even without
+    # `run_passes_on`) but not for some reason
+    # assert bytecode == out["bytecode_runtime"]

vyper/venom/passes/make_ssa.py

@@ -35,8 +35,8 @@ def _add_phi_nodes(self):
         Add phi nodes to the function.
         """
         self._compute_defs()
-        work = {var: 0 for var in self.dom.dfs_walk}
-        has_already = {var: 0 for var in self.dom.dfs_walk}
+        work = {bb: 0 for bb in self.dom.dfs_walk}
+        has_already = {bb: 0 for bb in self.dom.dfs_walk}
         i = 0
 
         # Iterate over all variables
@@ -106,8 +106,9 @@ def _rename_vars(self, basic_block: IRBasicBlock):
                 assert inst.output is not None, "Phi instruction without output"
                 for i, op in enumerate(inst.operands):
                     if op == basic_block.label:
+                        var = inst.operands[i + 1]
                         inst.operands[i + 1] = IRVariable(
-                            inst.output.name, version=self.var_name_stacks[inst.output.name][-1]
+                            var.name, version=self.var_name_stacks[var.name][-1]
                         )
 
         for bb in self.dom.dominated[basic_block]: