refactor[venom]: update more passes to use InstUpdater

vyper/venom/analysis/cfg.py

@@ -75,7 +75,9 @@ def invalidate(self):
             bb.cfg_out = OrderedSet()
             bb.out_vars = OrderedSet()
 
+        # just to be on the safe side, but this is probably not needed.
+        self.analyses_cache.invalidate_analysis(DFGAnalysis)
+
         self.analyses_cache.invalidate_analysis(DominatorTreeAnalysis)
         self.analyses_cache.invalidate_analysis(LivenessAnalysis)
-        self.analyses_cache.invalidate_analysis(DFGAnalysis)
         self._dfs = None

vyper/venom/analysis/dfg.py

@@ -31,8 +31,12 @@ def get_producing_instruction(self, op: IRVariable) -> Optional[IRInstruction]:
         return self._dfg_outputs.get(op)
 
     def set_producing_instruction(self, op: IRVariable, inst: IRInstruction):
+        # should this check if inst.output is already in dfg_outputs?
         self._dfg_outputs[op] = inst
 
+    def remove_producing_instruction(self, op: IRVariable):
+        del self._dfg_outputs[op]
+
     def add_use(self, op: IRVariable, inst: IRInstruction):
         uses = self._dfg_inputs.setdefault(op, OrderedSet())
         uses.add(inst)

vyper/venom/passes/algebraic_optimization.py

@@ -284,13 +284,15 @@ def _handle_inst_peephole(self, inst: IRInstruction):
             # (saves codesize, not gas)
             if lit_eq(operands[0], -1):
                 var = self.updater.add_before(inst, "not", [operands[1]])
+                assert var is not None  # help mypy
                 self.updater.update(inst, "iszero", [var])
                 return
 
             if prefer_iszero:
                 # (eq x y) has the same truthyness as (iszero (xor x y))
                 tmp = self.updater.add_before(inst, "xor", [operands[0], operands[1]])
 
+                assert tmp is not None  # help mypy
                 self.updater.update(inst, "iszero", [tmp])
                 return
 

vyper/venom/passes/branch_optimization.py

@@ -1,6 +1,5 @@
 from vyper.venom.analysis import CFGAnalysis, DFGAnalysis, LivenessAnalysis
-from vyper.venom.basicblock import IRInstruction
-from vyper.venom.passes.base_pass import IRPass
+from vyper.venom.passes.base_pass import InstUpdater, IRPass
 
 
 class BranchOptimizationPass(IRPass):
@@ -26,17 +25,18 @@ def _optimize_branches(self) -> None:
             prev_inst = self.dfg.get_producing_instruction(cond)
             if cost_a >= cost_b and prev_inst.opcode == "iszero":
                 new_cond = prev_inst.operands[0]
-                term_inst.operands = [new_cond, term_inst.operands[2], term_inst.operands[1]]
+                new_operands = [new_cond, term_inst.operands[2], term_inst.operands[1]]
+                self.updater.update(term_inst, term_inst.opcode, new_operands)
             elif cost_a > cost_b:
-                new_cond = fn.get_next_variable()
-                inst = IRInstruction("iszero", [term_inst.operands[0]], output=new_cond)
-                bb.insert_instruction(inst, index=-1)
-                term_inst.operands = [new_cond, term_inst.operands[2], term_inst.operands[1]]
+                new_cond = self.updater.add_before(term_inst, "iszero", [term_inst.operands[0]])
+                new_operands = [new_cond, term_inst.operands[2], term_inst.operands[1]]
+                self.updater.update(term_inst, term_inst.opcode, new_operands)
 
     def run_pass(self):
         self.liveness = self.analyses_cache.request_analysis(LivenessAnalysis)
         self.cfg = self.analyses_cache.request_analysis(CFGAnalysis)
         self.dfg = self.analyses_cache.request_analysis(DFGAnalysis)
+        self.updater = InstUpdater(self.dfg)
 
         self._optimize_branches()
 

vyper/venom/passes/load_elimination.py

@@ -36,7 +36,6 @@ def run_pass(self):
             self._process_bb(bb, None, "calldataload", None)
 
         self.analyses_cache.invalidate_analysis(LivenessAnalysis)
-        self.analyses_cache.invalidate_analysis(DFGAnalysis)
 
     def equivalent(self, op1, op2):
         return op1 == op2
@@ -72,8 +71,7 @@ def _handle_load(self, inst):
         self._lattice[ptr] = inst.output
 
         if existing_value is not None:
-            inst.opcode = "store"
-            inst.operands = [existing_value]
+            self.updater.store(inst, existing_value)
 
     def _handle_store(self, inst, store_opcode):
         # mstore [val, ptr]

vyper/venom/passes/machinery/inst_updater.py

@@ -1,3 +1,5 @@
+from typing import Optional
+
 from vyper.venom.analysis import DFGAnalysis
 from vyper.venom.basicblock import NO_OUTPUT_INSTRUCTIONS, IRInstruction, IROperand, IRVariable
 
@@ -16,7 +18,13 @@
         new_operands = [replace_dict[op] if op in replace_dict else op for op in old_operands]
         self.update(inst, inst.opcode, new_operands)
 
-    def update(self, inst: IRInstruction, opcode: str, new_operands: list[IROperand]):
+    def update(
+        self,
+        inst: IRInstruction,
+        opcode: str,
+        new_operands: list[IROperand],
+        new_output: Optional[IRVariable] = None,
+    ):
         assert opcode != "phi"
         # sanity
         assert all(isinstance(op, IROperand) for op in new_operands)
@@ -33,9 +41,19 @@
             if isinstance(op, IRVariable):
                 self.dfg.add_use(op, inst)
 
-        if opcode in NO_OUTPUT_INSTRUCTIONS and inst.output is not None:
-            assert len(uses := self.dfg.get_uses(inst.output)) == 0, (inst, uses)
-            inst.output = None
+        if opcode in NO_OUTPUT_INSTRUCTIONS:
+            if inst.output is not None:
+                assert new_output is None
+                assert len(uses := self.dfg.get_uses(inst.output)) == 0, (inst, uses)
+                self.dfg.remove_producing_instruction(inst.output)
+                inst.output = None
+        else:
+            # new_output is None is sentinel meaning "no change"
+            if new_output is not None and new_output != inst.output:
+                if inst.output is not None:
+                    self.dfg.remove_producing_instruction(inst.output)
+                self.dfg.set_producing_instruction(new_output, inst)
+                inst.output = new_output
 
         inst.opcode = opcode
         inst.operands = new_operands
@@ -48,22 +66,28 @@
         self.nop(inst)  # for dfg updates and checks
         inst.parent.remove_instruction(inst)
 
-    def store(self, inst: IRInstruction, op: IROperand):
-        self.update(inst, "store", [op])
+    def store(self, inst: IRInstruction, op: IROperand, new_output: Optional[IRVariable] = None):
+        self.update(inst, "store", [op], new_output=new_output)
 
-    def add_before(self, inst: IRInstruction, opcode: str, args: list[IROperand]) -> IRVariable:
+    def add_before(
+        self, inst: IRInstruction, opcode: str, args: list[IROperand]
+    ) -> Optional[IRVariable]:
         """
         Insert another instruction before the given instruction
         """
         assert opcode != "phi"
         index = inst.parent.instructions.index(inst)
-        var = inst.parent.parent.get_next_variable()
+
+        var = None
+        if opcode not in NO_OUTPUT_INSTRUCTIONS:
+            var = inst.parent.parent.get_next_variable()
+
         operands = list(args)
-        # TODO: add support for NO_OUTPUT_INSTRUCTIONS
         new_inst = IRInstruction(opcode, operands, output=var)
         inst.parent.insert_instruction(new_inst, index)
         for op in new_inst.operands:
             if isinstance(op, IRVariable):
                 self.dfg.add_use(op, new_inst)
-        self.dfg.set_producing_instruction(var, new_inst)
+        if var is not None:
+            self.dfg.set_producing_instruction(var, new_inst)
         return var

vyper/venom/passes/make_ssa.py

@@ -1,5 +1,5 @@
 from vyper.utils import OrderedSet
-from vyper.venom.analysis import CFGAnalysis, DominatorTreeAnalysis, LivenessAnalysis
+from vyper.venom.analysis import CFGAnalysis, DFGAnalysis, DominatorTreeAnalysis, LivenessAnalysis
 from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IROperand, IRVariable
 from vyper.venom.passes.base_pass import IRPass
 
@@ -29,6 +29,7 @@ def run_pass(self):
         self._remove_degenerate_phis(fn.entry)
 
         self.analyses_cache.invalidate_analysis(LivenessAnalysis)
+        self.analyses_cache.invalidate_analysis(DFGAnalysis)
 
     def _add_phi_nodes(self):
         """

vyper/venom/passes/mem2var.py

@@ -2,7 +2,7 @@
 from vyper.venom.analysis import CFGAnalysis, DFGAnalysis, LivenessAnalysis
 from vyper.venom.basicblock import IRInstruction, IRVariable
 from vyper.venom.function import IRFunction
-from vyper.venom.passes.base_pass import IRPass
+from vyper.venom.passes.base_pass import InstUpdater, IRPass
 
 
 class Mem2Var(IRPass):
@@ -16,15 +16,15 @@
     def run_pass(self):
         self.analyses_cache.request_analysis(CFGAnalysis)
         dfg = self.analyses_cache.request_analysis(DFGAnalysis)
+        self.updater = InstUpdater(dfg)
 
         self.var_name_count = 0
-        for var, inst in dfg.outputs.items():
+        for var, inst in dfg.outputs.copy().items():
             if inst.opcode == "alloca":
                 self._process_alloca_var(dfg, inst, var)
             elif inst.opcode == "palloca":
                 self._process_palloca_var(dfg, inst, var)
 
-        self.analyses_cache.invalidate_analysis(DFGAnalysis)
         self.analyses_cache.invalidate_analysis(LivenessAnalysis)
 
     def _mk_varname(self, varname: str, alloca_id: int):
@@ -46,20 +46,13 @@
         alloca_id = alloca_inst.operands[2]
         var_name = self._mk_varname(var.value, alloca_id.value)
         var = IRVariable(var_name)
-        for inst in uses:
+        for inst in uses.copy():
             if inst.opcode == "mstore":
-                inst.opcode = "store"
-                inst.output = var
-                inst.operands = [inst.operands[0]]
+                self.updater.store(inst, inst.operands[0], new_output=var)
             elif inst.opcode == "mload":
-                inst.opcode = "store"
-                inst.operands = [var]
+                self.updater.store(inst, var)
             elif inst.opcode == "return":
-                bb = inst.parent
-                idx = len(bb.instructions) - 1
-                assert inst == bb.instructions[idx]  # sanity
-                new_inst = IRInstruction("mstore", [var, inst.operands[1]])
-                bb.insert_instruction(new_inst, idx)
+                self.updater.add_before(inst, "mstore", [var, inst.operands[1]])
 
     def _process_palloca_var(self, dfg: DFGAnalysis, palloca_inst: IRInstruction, var: IRVariable):
         """
@@ -75,15 +68,12 @@
         var = IRVariable(var_name)
 
         # some value given to us by the calling convention
-        palloca_inst.opcode = "mload"
-        palloca_inst.operands = [ofst]
-        palloca_inst.output = var
+        # TODO: maybe better to not touch the palloca instruction,
+        # and instead "add_after" the palloca instruction.
+        self.updater.update(palloca_inst, "mload", [ofst], new_output=var)
 
-        for inst in uses:
+        for inst in uses.copy():
             if inst.opcode == "mstore":
-                inst.opcode = "store"
-                inst.output = var
-                inst.operands = [inst.operands[0]]
+                self.updater.store(inst, inst.operands[0], new_output=var)
             elif inst.opcode == "mload":
-                inst.opcode = "store"
-                inst.operands = [var]
+                self.updater.store(inst, var)

vyper/venom/passes/memmerging.py

@@ -137,6 +137,7 @@ def _optimize_copy(self, bb: IRBasicBlock, copy_opcode: str, load_opcode: str):
                 # we are converting an mcopy into an mload+mstore (mload+mstore
                 # is 1 byte smaller than mcopy).
                 val = self.updater.add_before(inst, load_opcode, [IRLiteral(copy.src)])
+                assert val is not None  # help mypy
                 self.updater.update(inst, "mstore", [val, IRLiteral(copy.dst)])
 
             for inst in copy.insts[:-1]:
@@ -292,6 +293,7 @@ def _optimize_memzero(self, bb: IRBasicBlock):
                 self.updater.update(inst, "mstore", new_ops)
             else:
                 calldatasize = self.updater.add_before(inst, "calldatasize", [])
+                assert calldatasize is not None  # help mypy
                 new_ops = [IRLiteral(copy.length), calldatasize, IRLiteral(copy.dst)]
                 self.updater.update(inst, "calldatacopy", new_ops)
 

vyper/venom/passes/simplify_cfg.py

@@ -1,6 +1,6 @@
 from vyper.exceptions import CompilerPanic
 from vyper.utils import OrderedSet
-from vyper.venom.analysis import CFGAnalysis
+from vyper.venom.analysis import CFGAnalysis, DFGAnalysis
 from vyper.venom.basicblock import IRBasicBlock, IRLabel
 from vyper.venom.passes.base_pass import IRPass
 
@@ -138,3 +138,4 @@ def run_pass(self):
             raise CompilerPanic("Too many iterations collapsing chained blocks")
 
         self.analyses_cache.invalidate_analysis(CFGAnalysis)
+        self.analyses_cache.invalidate_analysis(DFGAnalysis)