refactor[venom]: strengthen an assumption in venom lowering

vyper/venom/__init__.py

@@ -9,6 +9,7 @@
 from vyper.exceptions import CompilerPanic
 from vyper.venom.analysis import MemSSA
 from vyper.venom.analysis.analysis import IRAnalysesCache
+from vyper.venom.check_venom import check_venom_fn
 from vyper.venom.context import IRContext
 from vyper.venom.function import IRFunction
 from vyper.venom.ir_node_to_venom import ir_node_to_venom

vyper/venom/check_venom.py

@@ -77,6 +77,13 @@ def find_semantic_errors(context: IRContext) -> list[VenomError]:
     return errors
 
 
+def check_venom_fn(fn: IRFunction):
+    errors = find_semantic_errors_fn(fn)
+
+    if errors:
+        raise ExceptionGroup("venom semantic errors", errors)
+
+
 def check_venom_ctx(context: IRContext):
     errors = find_semantic_errors(context)
 

vyper/venom/passes/simplify_cfg.py

@@ -36,6 +36,11 @@ def _merge_jump(self, a: IRBasicBlock, b: IRBasicBlock):
         jump_inst = a.instructions[-1]
         assert b.label in jump_inst.operands, f"{b.label} {jump_inst.operands}"
         jump_inst.operands[jump_inst.operands.index(b.label)] = next_bb.label
+        for inst in next_bb.instructions:
+            # assume phi instructions are at beginning of bb
+            if inst.opcode != "phi":
+                break
+            inst.operands[inst.operands.index(b.label)] = a.label
 
         self._schedule_label_replacement(b.label, next_bb.label)
 

vyper/venom/passes/single_use_expansion.py

@@ -1,6 +1,7 @@
 from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis
-from vyper.venom.basicblock import IRInstruction, IRLiteral, IRVariable
+from vyper.venom.basicblock import IRInstruction, IRLiteral, IROperand, IRVariable
 from vyper.venom.passes.base_pass import IRPass
+from vyper.venom.passes.machinery.inst_updater import InstUpdater
 
 
 class SingleUseExpansion(IRPass):
@@ -21,17 +22,27 @@ class SingleUseExpansion(IRPass):
 
     def run_pass(self):
         self.dfg = self.analyses_cache.request_analysis(DFGAnalysis)
+        self.updater = InstUpdater(self.dfg)
+        self.phis: list[IRInstruction] = []
         for bb in self.function.get_basic_blocks():
             self._process_bb(bb)
 
+        for inst in self.phis:
+            self._process_phi(inst)
+
         self.analyses_cache.invalidate_analysis(DFGAnalysis)
         self.analyses_cache.invalidate_analysis(LivenessAnalysis)
 
     def _process_bb(self, bb):
         i = 0
         while i < len(bb.instructions):
             inst = bb.instructions[i]
-            if inst.opcode in ("assign", "offset", "phi", "param"):
+            if inst.opcode in ("assign", "offset", "param"):
+                i += 1
+                continue
+
+            if inst.opcode == "phi":
+                self.phis.append(inst)
                 i += 1
                 continue
 
@@ -51,10 +62,54 @@ def _process_bb(self, bb):
                     # skip them for now.
                     continue
 
-                var = self.function.get_next_variable()
-                to_insert = IRInstruction("assign", [op], var)
-                bb.insert_instruction(to_insert, index=i)
-                inst.operands[j] = var
+                var = self.updater.add_before(inst, "assign", [op])
+                assert var is not None
+                ops = inst.operands.copy()
+                ops[j] = var
+                self.updater.update(inst, inst.opcode, ops)
                 i += 1
 
             i += 1
+
+    def _process_phi(self, inst: IRInstruction):
+        assert inst.opcode == "phi"
+
+        replacements: dict[IROperand, IROperand] = {}
+        for label, var in inst.phi_operands:
+            assert isinstance(var, IRVariable)
+            # you only care about the cases which would be not correct
+            # as an output of this pass
+            # example
+            #
+            #   bb1:
+            #       ...
+            #       ; it does not matter that the %origin is here for the phi instruction
+            #       ; since if this is the only place where the origin is used
+            #       ; other then the phi node then the phi node does not have to add
+            #       ; additional store for it as and input to phi
+            #       %var = %origin
+            #       ...
+            #       jmp @bb2
+            #   bb2:
+            #       ; the %origin does not have to be extracted to new varible
+            #       ; since the only place where it is used is assign instruction
+            #       %phi = phi @bb1, %origin, @someother, %somevar
+            #       ...
+
+            # if the only other use would be in assigns then the variable
+            # does not have to be moved out to the new assign
+            uses = [use for use in self.dfg.get_uses(var) if use.opcode != "assign"]
+
+            # if the only other use would be in phi node in the some other block then
+            # the same rule applies
+            uses = [use for use in uses if use.opcode != "phi" or use.parent == inst.parent]
+            if len(uses) <= 1:
+                continue
+            bb = self.function.get_basic_block(label.name)
+            term = bb.instructions[-1]
+            assert term.is_bb_terminator
+            new_var = self.updater.add_before(term, "assign", [var])
+            assert new_var is not None
+            replacements[var] = new_var
+
+        self.updater.update_operands(inst, replacements)

vyper/venom/venom_to_assembly.py

@@ -436,13 +436,9 @@ def _generate_evm_for_instruction(
             # example, for `%56 = %label1 %13 %label2 %14`, we will
             # find an instance of %13 *or* %14 in the stack and replace it with %56.
             to_be_replaced = stack.peek(depth)
-            if to_be_replaced in next_liveness:
-                # this branch seems unreachable (maybe due to make_ssa)
-                # %13/%14 is still live(!), so we make a copy of it
-                self.dup(assembly, stack, depth)
-                stack.poke(0, ret)
-            else:
-                stack.poke(depth, ret)
+            # precondition from SSA
+            assert to_be_replaced not in next_liveness
+            stack.poke(depth, ret)
             return apply_line_numbers(inst, assembly)
 
         if opcode == "offset":