From 881959c496a9aed759ed7cb2a8b8224d84ada6d3 Mon Sep 17 00:00:00 2001 From: banteg <4562643+banteg@users.noreply.github.com> Date: Fri, 12 Jun 2026 14:38:03 +0400 Subject: [PATCH 1/3] fix[venom]: keep phis over distinct outputs of one invoke --- .../compiler/venom/test_phi_elimination.py | 40 ++++++++++++++++ vyper/venom/passes/phi_elimination.py | 47 +++++++++++-------- 2 files changed, 67 insertions(+), 20 deletions(-) diff --git a/tests/unit/compiler/venom/test_phi_elimination.py b/tests/unit/compiler/venom/test_phi_elimination.py index 66d436e7fd..0f5428a8e3 100644 --- a/tests/unit/compiler/venom/test_phi_elimination.py +++ b/tests/unit/compiler/venom/test_phi_elimination.py @@ -1,6 +1,9 @@ import pytest from tests.venom_utils import PrePostChecker +from vyper.venom.analysis import IRAnalysesCache +from vyper.venom.basicblock import IRLabel +from vyper.venom.parser import parse_venom from vyper.venom.passes import PhiEliminationPass _check_pre_post = PrePostChecker([PhiEliminationPass], default_hevm=False) @@ -319,3 +322,40 @@ def test_phi_elim_two_phi_merges(): """ _check_pre_post(pre, post, hevm=True) + + +def test_phi_elim_multi_output_invoke(): + # a phi over two different outputs of the same invoke instruction + # must not be collapsed: the origins trace to the same producing + # instruction, but to different variables (GH 5039) + pre = """ + function main { + main: + %cond = source + %r1, %r2 = invoke @f + jnz %cond, @then, @else + then: + jmp @join + else: + jmp @join + join: + %z = phi @then, %r1, @else, %r2 + sink %z + } + + function f { + f: + %retpc = param + ret %retpc + } + """ + + ctx = parse_venom(pre) + fn = ctx.get_function(IRLabel("main")) + ac = IRAnalysesCache(fn) + PhiEliminationPass(ac, fn).run_pass() + + insts = [inst for bb in fn.get_basic_blocks() for inst in bb.instructions] + phis = [inst for inst in insts if inst.opcode == "phi"] + assert len(phis) == 1, "phi over distinct invoke outputs must be kept" + assert phis[0].output.name == "%z" diff --git a/vyper/venom/passes/phi_elimination.py b/vyper/venom/passes/phi_elimination.py index 16f6cf42f6..45cd0c2538 100644 --- a/vyper/venom/passes/phi_elimination.py +++ b/vyper/venom/passes/phi_elimination.py @@ -1,10 +1,14 @@ from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis -from vyper.venom.basicblock import IRInstruction, IRVariable +from vyper.venom.basicblock import IRInstruction, IROperand, IRVariable from vyper.venom.passes.base_pass import InstUpdater, IRPass class PhiEliminationPass(IRPass): - phi_to_origins: dict[IRInstruction, set[IRInstruction]] + # phi -> set of (root instruction, variable produced by it). origins + # are keyed by the produced variable and not just the instruction, + # since different outputs of one multi-output instruction (e.g. + # invoke) are distinct origins. + phi_to_origins: dict[IRInstruction, set[tuple[IRInstruction, IROperand]]] def run_pass(self): self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) @@ -27,10 +31,10 @@ def _process_phi(self, inst: IRInstruction): # len > 1: multiple origins, phi is doing real work, keep it. if len(srcs) == 1: - src = next(iter(srcs)) + ((src, var),) = srcs if src == inst: return - self.updater.mk_assign(inst, src.output) + self.updater.mk_assign(inst, var) def _calculate_phi_origins(self): self.phi_to_origins = dict() @@ -44,13 +48,14 @@ def _calculate_phi_origins(self): def _get_phi_origins(self, inst: IRInstruction): assert inst.opcode == "phi" # sanity visited: set[IRInstruction] = set() - self.phi_to_origins[inst] = self._get_phi_origins_r(inst, visited) + self.phi_to_origins[inst] = self._get_phi_origins_r(inst, inst.output, visited) - # traverse chains of phis and stores to get the "root" instructions - # for phis. + # traverse chains of phis and stores to get the "root" definitions + # for phis. `var` is the ssa variable through which `inst` was + # reached; it identifies which output of `inst` is the origin. def _get_phi_origins_r( - self, inst: IRInstruction, visited: set[IRInstruction] - ) -> set[IRInstruction]: + self, inst: IRInstruction, var: IROperand, visited: set[IRInstruction] + ) -> set[tuple[IRInstruction, IROperand]]: if inst.opcode == "phi": if inst in self.phi_to_origins: return self.phi_to_origins[inst] @@ -64,12 +69,12 @@ def _get_phi_origins_r( visited.add(inst) - res: set[IRInstruction] = set() + res: set[tuple[IRInstruction, IROperand]] = set() - for _, var in inst.phi_operands: - next_inst = self.dfg.get_producing_instruction(var) - assert next_inst is not None, (inst, var) - res |= self._get_phi_origins_r(next_inst, visited) + for _, op_var in inst.phi_operands: + next_inst = self.dfg.get_producing_instruction(op_var) + assert next_inst is not None, (inst, op_var) + res |= self._get_phi_origins_r(next_inst, op_var, visited) if len(res) > 1: # multi-origin phi: treat the phi itself as a "barrier" origin. @@ -84,15 +89,17 @@ def _get_phi_origins_r( # %c = phi %a, %b ; barrier (two origins) # %d = %c # %f = phi %d, %c ; both paths lead to %c, so %f = %c - return set([inst]) + return set([(inst, inst.output)]) return res if inst.opcode == "assign" and isinstance(inst.operands[0], IRVariable): # traverse assignment chain - var = inst.operands[0] - next_inst = self.dfg.get_producing_instruction(var) + src_var = inst.operands[0] + next_inst = self.dfg.get_producing_instruction(src_var) assert next_inst is not None - return self._get_phi_origins_r(next_inst, visited) + return self._get_phi_origins_r(next_inst, src_var, visited) - # root of the phi/assignment chain - return set([inst]) + # root of the phi/assignment chain. note that for multi-output + # instructions (e.g. invoke), different outputs are distinct + # origins, so the origin is identified by (inst, var). + return set([(inst, var)]) From 4781eb1929c207c83d1763cfc8b1d79b16ef07cf Mon Sep 17 00:00:00 2001 From: banteg <4562643+banteg@users.noreply.github.com> Date: Wed, 17 Jun 2026 14:26:24 +0400 Subject: [PATCH 2/3] fix[venom]: eliminate phis over same invoke output --- .../compiler/venom/test_phi_elimination.py | 43 ++++++++++++++++++- vyper/venom/passes/phi_elimination.py | 8 ---- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/tests/unit/compiler/venom/test_phi_elimination.py b/tests/unit/compiler/venom/test_phi_elimination.py index 0f5428a8e3..ba254635ab 100644 --- a/tests/unit/compiler/venom/test_phi_elimination.py +++ b/tests/unit/compiler/venom/test_phi_elimination.py @@ -324,7 +324,7 @@ def test_phi_elim_two_phi_merges(): _check_pre_post(pre, post, hevm=True) -def test_phi_elim_multi_output_invoke(): +def test_phi_elim_multi_output_invoke_distinct_outputs_kept(): # a phi over two different outputs of the same invoke instruction # must not be collapsed: the origins trace to the same producing # instruction, but to different variables (GH 5039) @@ -359,3 +359,44 @@ def test_phi_elim_multi_output_invoke(): phis = [inst for inst in insts if inst.opcode == "phi"] assert len(phis) == 1, "phi over distinct invoke outputs must be kept" assert phis[0].output.name == "%z" + + +def test_phi_elim_multi_output_invoke_same_output_eliminated(): + # Same producer instruction and same output variable: this phi is trivial + # even though the root instruction has multiple outputs. + source = """ + function main { + main: + %cond = source + %r1, %r2 = invoke @f + %r2_copy = %r2 + jnz %cond, @then, @else + then: + jmp @join + else: + jmp @join + join: + %z = phi @then, %r2, @else, %r2_copy + sink %z + } + + function f { + f: + %retpc = param + ret %retpc + } + """ + + ctx = parse_venom(source) + fn = ctx.get_function(IRLabel("main")) + ac = IRAnalysesCache(fn) + PhiEliminationPass(ac, fn).run_pass() + + insts = [inst for bb in fn.get_basic_blocks() for inst in bb.instructions] + phis = [inst for inst in insts if inst.opcode == "phi"] + assert len(phis) == 0, "phi over the same invoke output should be eliminated" + + z_assigns = [inst for inst in insts if [out.name for out in inst.get_outputs()] == ["%z"]] + assert len(z_assigns) == 1 + assert z_assigns[0].opcode == "assign" + assert z_assigns[0].operands[0].name == "%r2" diff --git a/vyper/venom/passes/phi_elimination.py b/vyper/venom/passes/phi_elimination.py index 11e6414c5b..45cd0c2538 100644 --- a/vyper/venom/passes/phi_elimination.py +++ b/vyper/venom/passes/phi_elimination.py @@ -34,14 +34,6 @@ def _process_phi(self, inst: IRInstruction): ((src, var),) = srcs if src == inst: return - # the origin may be a multi-output instruction (invoke with a - # hidden adopted-FMP output, bump); `src.output` asserts - # single-output, and collapsing the phi to outputs[0] would be - # wrong when the phi'd value is a later output. Latent for - # multi-output invokes; common once FmpLoweringPass threads - # bump/invoke outputs through loop phis. - if len(src.get_outputs()) != 1: - return self.updater.mk_assign(inst, var) def _calculate_phi_origins(self): From 56bd2b189da9caf77762d80f00f083dd58d49596 Mon Sep 17 00:00:00 2001 From: banteg <4562643+banteg@users.noreply.github.com> Date: Thu, 18 Jun 2026 02:21:35 +0400 Subject: [PATCH 3/3] refactor[venom]: track phi origins by operand --- vyper/venom/passes/phi_elimination.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/vyper/venom/passes/phi_elimination.py b/vyper/venom/passes/phi_elimination.py index 45cd0c2538..4e95dc9f3e 100644 --- a/vyper/venom/passes/phi_elimination.py +++ b/vyper/venom/passes/phi_elimination.py @@ -4,11 +4,9 @@ class PhiEliminationPass(IRPass): - # phi -> set of (root instruction, variable produced by it). origins - # are keyed by the produced variable and not just the instruction, - # since different outputs of one multi-output instruction (e.g. - # invoke) are distinct origins. - phi_to_origins: dict[IRInstruction, set[tuple[IRInstruction, IROperand]]] + # phi -> set of root operands. The operand identifies both the + # producing instruction and the produced output slot. + phi_to_origins: dict[IRInstruction, set[IROperand]] def run_pass(self): self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) @@ -31,8 +29,8 @@ def _process_phi(self, inst: IRInstruction): # len > 1: multiple origins, phi is doing real work, keep it. if len(srcs) == 1: - ((src, var),) = srcs - if src == inst: + (var,) = srcs + if var == inst.output: return self.updater.mk_assign(inst, var) @@ -55,7 +53,7 @@ def _get_phi_origins(self, inst: IRInstruction): # reached; it identifies which output of `inst` is the origin. def _get_phi_origins_r( self, inst: IRInstruction, var: IROperand, visited: set[IRInstruction] - ) -> set[tuple[IRInstruction, IROperand]]: + ) -> set[IROperand]: if inst.opcode == "phi": if inst in self.phi_to_origins: return self.phi_to_origins[inst] @@ -69,7 +67,7 @@ def _get_phi_origins_r( visited.add(inst) - res: set[tuple[IRInstruction, IROperand]] = set() + res: set[IROperand] = set() for _, op_var in inst.phi_operands: next_inst = self.dfg.get_producing_instruction(op_var) @@ -89,7 +87,7 @@ def _get_phi_origins_r( # %c = phi %a, %b ; barrier (two origins) # %d = %c # %f = phi %d, %c ; both paths lead to %c, so %f = %c - return set([(inst, inst.output)]) + return set([inst.output]) return res if inst.opcode == "assign" and isinstance(inst.operands[0], IRVariable): @@ -101,5 +99,5 @@ def _get_phi_origins_r( # root of the phi/assignment chain. note that for multi-output # instructions (e.g. invoke), different outputs are distinct - # origins, so the origin is identified by (inst, var). - return set([(inst, var)]) + # origins because their output operands are distinct. + return set([var])