From d8873c794b970a82ad7cdd5be72bcb20a38aa93c Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 8 Jul 2025 12:24:44 +0100 Subject: [PATCH 01/16] refactor[venom]: strengthen an assumption in venom lowering this commit strengthens an assumption in venom_to_assembly. due to SSA form, the arguments to phi must be in the predecessor blocks. this means that the def does not reach any use from both branches, except in the case that the use is a phi. --- vyper/venom/venom_to_assembly.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index c03b62431b..e6405561b5 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/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": From dbad4c65bd0433e2d7a9fa58b9359a56a9455691 Mon Sep 17 00:00:00 2001 From: Hodan Date: Wed, 13 Aug 2025 14:06:26 +0200 Subject: [PATCH 02/16] phi single use and fix of the simplify cfg pass --- vyper/venom/__init__.py | 1 + vyper/venom/check_venom.py | 7 ++++++ vyper/venom/passes/simplify_cfg.py | 5 +++++ vyper/venom/passes/single_use_expansion.py | 26 +++++++++++++++++++++- 4 files changed, 38 insertions(+), 1 deletion(-) diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index 7ccd7b75e8..72e0117ce7 100644 --- a/vyper/venom/__init__.py +++ b/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 diff --git a/vyper/venom/check_venom.py b/vyper/venom/check_venom.py index 0dc198d2b3..66aa6af71f 100644 --- a/vyper/venom/check_venom.py +++ b/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) diff --git a/vyper/venom/passes/simplify_cfg.py b/vyper/venom/passes/simplify_cfg.py index bf6306a91a..41566efd23 100644 --- a/vyper/venom/passes/simplify_cfg.py +++ b/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) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index e50deed079..fb258b48ac 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/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.passes.base_pass import IRPass +from vyper.venom.passes.machinery.inst_updater import InstUpdater class SingleUseExpansion(IRPass): @@ -21,6 +22,7 @@ class SingleUseExpansion(IRPass): def run_pass(self): self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) + self.updater = InstUpdater(self.dfg) for bb in self.function.get_basic_blocks(): self._process_bb(bb) @@ -31,7 +33,12 @@ 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._process_phi(inst) i += 1 continue @@ -58,3 +65,20 @@ def _process_bb(self, bb): i += 1 i += 1 + + def _process_phi(self, inst: IRInstruction): + assert inst.opcode == "phi" + + new_vars: list[IRVariable] = [] + for label, var in inst.phi_operands: + assert isinstance(var, IRVariable) + 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 + new_vars.append(new_var) + + for index, var in enumerate(new_vars): + i = 2 * index + 1 + inst.operands[i] = var From fa89756c8a05e707d35bd9cd132c27e23b9f8c16 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 13 Aug 2025 16:28:09 +0300 Subject: [PATCH 03/16] Apply suggestions from code review --- vyper/venom/passes/single_use_expansion.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index fb258b48ac..c55719aa4f 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -79,6 +79,6 @@ def _process_phi(self, inst: IRInstruction): assert new_var is not None new_vars.append(new_var) - for index, var in enumerate(new_vars): + for index, new_var in enumerate(new_vars): i = 2 * index + 1 - inst.operands[i] = var + inst.operands[i] = new_var From ab7993431ec0fcae8e4629bc18cd33e70718c87f Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 14 Aug 2025 13:13:04 +0300 Subject: [PATCH 04/16] use replace_operands --- vyper/venom/passes/single_use_expansion.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index c55719aa4f..2408dd23d2 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -69,7 +69,7 @@ def _process_bb(self, bb): def _process_phi(self, inst: IRInstruction): assert inst.opcode == "phi" - new_vars: list[IRVariable] = [] + replacements: dict[IRVariable, IRVariable] = {} for label, var in inst.phi_operands: assert isinstance(var, IRVariable) bb = self.function.get_basic_block(label.name) @@ -77,8 +77,6 @@ def _process_phi(self, inst: IRInstruction): assert term.is_bb_terminator new_var = self.updater.add_before(term, "assign", [var]) assert new_var is not None - new_vars.append(new_var) + replacements[var] = new_var - for index, new_var in enumerate(new_vars): - i = 2 * index + 1 - inst.operands[i] = new_var + inst.replace_operands(replacements) From c08111d86c618d7d2b83f7c287230488e862de41 Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 15 Aug 2025 10:19:46 +0200 Subject: [PATCH 05/16] used updater --- vyper/venom/passes/single_use_expansion.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index 2408dd23d2..a09d19c5a0 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -58,9 +58,10 @@ 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) + #var = self.function.get_next_variable() + #to_insert = IRInstruction("assign", [op], var) + #bb.insert_instruction(to_insert, index=i) + var = self.updater.add_before(inst, "assign", [op]) inst.operands[j] = var i += 1 From 10b5be49d2f57bdbff932050df1425fe65881429 Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 15 Aug 2025 10:33:12 +0200 Subject: [PATCH 06/16] check for only one use --- vyper/venom/passes/single_use_expansion.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index a09d19c5a0..468aad2ab7 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -73,6 +73,9 @@ def _process_phi(self, inst: IRInstruction): replacements: dict[IRVariable, IRVariable] = {} for label, var in inst.phi_operands: assert isinstance(var, IRVariable) + uses = self.dfg.get_uses(var) + if len(uses) == 1: + continue bb = self.function.get_basic_block(label.name) term = bb.instructions[-1] assert term.is_bb_terminator From e2a8214d3092952103170319facbe7def98bfe3c Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 15 Aug 2025 10:42:49 +0200 Subject: [PATCH 07/16] do phis after everything --- vyper/venom/passes/single_use_expansion.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index 468aad2ab7..9ef582c20c 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -23,9 +23,13 @@ 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) @@ -38,7 +42,7 @@ def _process_bb(self, bb): continue if inst.opcode == "phi": - self._process_phi(inst) + self.phis.append(inst) i += 1 continue From f211bf5025f217fe6616729568967864dcff159d Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 15 Aug 2025 11:17:02 +0200 Subject: [PATCH 08/16] only look at non assign uses --- vyper/venom/passes/single_use_expansion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index 9ef582c20c..1f75d4041d 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -77,7 +77,7 @@ def _process_phi(self, inst: IRInstruction): replacements: dict[IRVariable, IRVariable] = {} for label, var in inst.phi_operands: assert isinstance(var, IRVariable) - uses = self.dfg.get_uses(var) + uses = [inst for inst in self.dfg.get_uses(var) if inst.opcode != "assign"] if len(uses) == 1: continue bb = self.function.get_basic_block(label.name) From 4ada353d7fd752ae632bd4be9817cddf09336d69 Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 15 Aug 2025 11:46:31 +0200 Subject: [PATCH 09/16] small clean up --- vyper/venom/passes/single_use_expansion.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index 1f75d4041d..52dade7118 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -62,9 +62,6 @@ 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) var = self.updater.add_before(inst, "assign", [op]) inst.operands[j] = var i += 1 From 78a710e2acada593f156b8ea364fe17f902e1b6c Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 15 Aug 2025 13:57:21 +0200 Subject: [PATCH 10/16] better cond --- vyper/venom/passes/single_use_expansion.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index 52dade7118..2d9dfe5d1d 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -74,8 +74,12 @@ def _process_phi(self, inst: IRInstruction): replacements: dict[IRVariable, IRVariable] = {} for label, var in inst.phi_operands: assert isinstance(var, IRVariable) - uses = [inst for inst in self.dfg.get_uses(var) if inst.opcode != "assign"] - if len(uses) == 1: + uses = [ + use + for use in self.dfg.get_uses(var) + if not use.opcode == "assign" + ] + if len(uses) <= 1: continue bb = self.function.get_basic_block(label.name) term = bb.instructions[-1] From 98ff102e6ae35b7c29fa42307cd374572cd909da Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 15 Aug 2025 14:05:28 +0200 Subject: [PATCH 11/16] better cond for filter --- vyper/venom/passes/single_use_expansion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index 2d9dfe5d1d..ae613518c3 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -77,7 +77,7 @@ def _process_phi(self, inst: IRInstruction): uses = [ use for use in self.dfg.get_uses(var) - if not use.opcode == "assign" + if not (use.opcode == "assign" or (use.opcode == "phi" and inst.parent != use.parent)) ] if len(uses) <= 1: continue From c2451449f85e20cf3b716b09b9decbca140b0de3 Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 15 Aug 2025 14:34:46 +0200 Subject: [PATCH 12/16] finally properly used updater --- vyper/venom/passes/single_use_expansion.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index ae613518c3..7fdc4bba5c 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -1,5 +1,5 @@ from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis -from vyper.venom.basicblock import IRInstruction, IRLiteral, IRVariable +from vyper.venom.basicblock import IRInstruction, IRLiteral, IRVariable, IROperand from vyper.venom.passes.base_pass import IRPass from vyper.venom.passes.machinery.inst_updater import InstUpdater @@ -63,7 +63,10 @@ def _process_bb(self, bb): continue var = self.updater.add_before(inst, "assign", [op]) - inst.operands[j] = var + assert var is not None + ops = inst.operands.copy() + ops[j] = var + self.updater.update(inst, inst.opcode, ops) i += 1 i += 1 @@ -71,13 +74,13 @@ def _process_bb(self, bb): def _process_phi(self, inst: IRInstruction): assert inst.opcode == "phi" - replacements: dict[IRVariable, IRVariable] = {} + replacements: dict[IROperand, IROperand] = {} for label, var in inst.phi_operands: assert isinstance(var, IRVariable) uses = [ use for use in self.dfg.get_uses(var) - if not (use.opcode == "assign" or (use.opcode == "phi" and inst.parent != use.parent)) + if not (use.opcode == "assign" or (use.opcode == "phi" and use.parent != inst.parent)) ] if len(uses) <= 1: continue @@ -88,4 +91,4 @@ def _process_phi(self, inst: IRInstruction): assert new_var is not None replacements[var] = new_var - inst.replace_operands(replacements) + self.updater.update_operands(inst, replacements) From f4f594372b7d860e72957dd319749db343976d09 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 25 Aug 2025 15:03:43 +0200 Subject: [PATCH 13/16] comment --- vyper/venom/passes/single_use_expansion.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index 7fdc4bba5c..1bd4d7dce7 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -77,6 +77,19 @@ def _process_phi(self, inst: IRInstruction): 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 + # %var = %origin + # ... + # jmp @bb2 + # bb2: + # %phi = phi @bb1, %origin, @someother, %somevar + # ... uses = [ use for use in self.dfg.get_uses(var) From 0cdf6c486d83fc893ac12750ea20952644b33fdc Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 25 Aug 2025 15:05:41 +0200 Subject: [PATCH 14/16] lint --- vyper/venom/passes/single_use_expansion.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index 1bd4d7dce7..938c654df4 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -1,5 +1,5 @@ from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis -from vyper.venom.basicblock import IRInstruction, IRLiteral, IRVariable, IROperand +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 @@ -66,7 +66,7 @@ def _process_bb(self, bb): assert var is not None ops = inst.operands.copy() ops[j] = var - self.updater.update(inst, inst.opcode, ops) + self.updater.update(inst, inst.opcode, ops) i += 1 i += 1 @@ -92,8 +92,10 @@ def _process_phi(self, inst: IRInstruction): # ... uses = [ use - for use in self.dfg.get_uses(var) - if not (use.opcode == "assign" or (use.opcode == "phi" and use.parent != inst.parent)) + for use in self.dfg.get_uses(var) + if not ( + use.opcode == "assign" or (use.opcode == "phi" and use.parent != inst.parent) + ) ] if len(uses) <= 1: continue @@ -104,4 +106,4 @@ def _process_phi(self, inst: IRInstruction): assert new_var is not None replacements[var] = new_var - self.updater.update_operands(inst, replacements) + self.updater.update_operands(inst, replacements) From 0c24dee8518072bc11cd5b5caffa3b3825163514 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 1 Sep 2025 09:55:17 +0200 Subject: [PATCH 15/16] split the uses query into two --- vyper/venom/passes/single_use_expansion.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index 938c654df4..6a094261c2 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -90,13 +90,14 @@ def _process_phi(self, inst: IRInstruction): # bb2: # %phi = phi @bb1, %origin, @someother, %somevar # ... - uses = [ - use - for use in self.dfg.get_uses(var) - if not ( - use.opcode == "assign" or (use.opcode == "phi" and use.parent != inst.parent) - ) - ] + + # 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) From 42ceed143f0a18b46749b9ff21c62d873cc1bd9e Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 1 Sep 2025 10:08:12 +0200 Subject: [PATCH 16/16] more comments --- vyper/venom/passes/single_use_expansion.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/vyper/venom/passes/single_use_expansion.py b/vyper/venom/passes/single_use_expansion.py index 6a094261c2..d07bb74cc2 100644 --- a/vyper/venom/passes/single_use_expansion.py +++ b/vyper/venom/passes/single_use_expansion.py @@ -84,10 +84,15 @@ def _process_phi(self, inst: IRInstruction): # 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 # ...