From 9ceb3e37599fe6aff252a7df4a623aadef96169f Mon Sep 17 00:00:00 2001 From: Hodan Date: Thu, 18 Jul 2024 17:03:42 +0200 Subject: [PATCH 01/13] fix[venom]: test fix for issue https://github.com/vyperlang/vyper/issues/4070 --- vyper/venom/passes/simplify_cfg.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/vyper/venom/passes/simplify_cfg.py b/vyper/venom/passes/simplify_cfg.py index 08582fee96..84d7483e4b 100644 --- a/vyper/venom/passes/simplify_cfg.py +++ b/vyper/venom/passes/simplify_cfg.py @@ -21,14 +21,15 @@ def _merge_blocks(self, a: IRBasicBlock, b: IRBasicBlock): # Update CFG a.cfg_out = b.cfg_out if len(b.cfg_out) > 0: - next_bb = b.cfg_out.first() - next_bb.remove_cfg_in(b) - next_bb.add_cfg_in(a) - - for inst in next_bb.instructions: - if inst.opcode != "phi": - break - inst.operands[inst.operands.index(b.label)] = a.label + #next_bb = b.cfg_out.first() + for next_bb in b.cfg_out: + next_bb.remove_cfg_in(b) + next_bb.add_cfg_in(a) + + for inst in next_bb.instructions: + if inst.opcode != "phi": + break + inst.operands[inst.operands.index(b.label)] = a.label self.function.remove_basic_block(b) From c18ca96ac1643aec07db97b3e2f95288451abba3 Mon Sep 17 00:00:00 2001 From: Hodan Date: Thu, 25 Jul 2024 11:32:14 +0200 Subject: [PATCH 02/13] fix[venom]: ternary issue --- vyper/venom/passes/simplify_cfg.py | 1 - 1 file changed, 1 deletion(-) diff --git a/vyper/venom/passes/simplify_cfg.py b/vyper/venom/passes/simplify_cfg.py index 84d7483e4b..938ad5201a 100644 --- a/vyper/venom/passes/simplify_cfg.py +++ b/vyper/venom/passes/simplify_cfg.py @@ -21,7 +21,6 @@ def _merge_blocks(self, a: IRBasicBlock, b: IRBasicBlock): # Update CFG a.cfg_out = b.cfg_out if len(b.cfg_out) > 0: - #next_bb = b.cfg_out.first() for next_bb in b.cfg_out: next_bb.remove_cfg_in(b) next_bb.add_cfg_in(a) From dff03be92fbb22da6cf0043957093054b13de995 Mon Sep 17 00:00:00 2001 From: Hodan Date: Thu, 25 Jul 2024 13:05:52 +0200 Subject: [PATCH 03/13] fix[venom]: ternary issue --- vyper/venom/passes/simplify_cfg.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vyper/venom/passes/simplify_cfg.py b/vyper/venom/passes/simplify_cfg.py index 938ad5201a..bcb8b706bd 100644 --- a/vyper/venom/passes/simplify_cfg.py +++ b/vyper/venom/passes/simplify_cfg.py @@ -11,11 +11,10 @@ class SimplifyCFGPass(IRPass): def _merge_blocks(self, a: IRBasicBlock, b: IRBasicBlock): a.instructions.pop() for inst in b.instructions: - assert inst.opcode != "phi", "Not implemented yet" + inst.parent = a if inst.opcode == "phi": a.instructions.insert(0, inst) else: - inst.parent = a a.instructions.append(inst) # Update CFG From 98652d08a08a480426f578d10b2d195fcec0a748 Mon Sep 17 00:00:00 2001 From: Hodan Date: Thu, 25 Jul 2024 16:30:45 +0200 Subject: [PATCH 04/13] fix[venom]: removing problematic phi nodes --- vyper/venom/__init__.py | 2 ++ .../venom/passes/remove_invalid_phi_nodes.py | 36 +++++++++++++++++++ vyper/venom/passes/simplify_cfg.py | 6 ++-- 3 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 vyper/venom/passes/remove_invalid_phi_nodes.py diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index afd79fc44f..990b761b77 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -19,6 +19,7 @@ from vyper.venom.passes.sccp import SCCP from vyper.venom.passes.simplify_cfg import SimplifyCFGPass from vyper.venom.passes.store_elimination import StoreElimination +from vyper.venom.passes.remove_invalid_phi_nodes import RemoveInvalidPhiPass from vyper.venom.venom_to_assembly import VenomCompiler DEFAULT_OPT_LEVEL = OptimizationLevel.default() @@ -50,6 +51,7 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: Mem2Var(ac, fn).run_pass() MakeSSA(ac, fn).run_pass() SCCP(ac, fn).run_pass() + RemoveInvalidPhiPass(ac, fn).run_pass() StoreElimination(ac, fn).run_pass() SimplifyCFGPass(ac, fn).run_pass() AlgebraicOptimizationPass(ac, fn).run_pass() diff --git a/vyper/venom/passes/remove_invalid_phi_nodes.py b/vyper/venom/passes/remove_invalid_phi_nodes.py new file mode 100644 index 0000000000..54dfb1f5d8 --- /dev/null +++ b/vyper/venom/passes/remove_invalid_phi_nodes.py @@ -0,0 +1,36 @@ +from vyper.venom.basicblock import IRBasicBlock, IRLabel, IRInstruction, IRVariable +from vyper.venom.passes.base_pass import IRPass +from vyper.venom.analysis.dfg import DFGAnalysis +from vyper.venom.analysis.cfg import CFGAnalysis + +class RemoveInvalidPhiPass(IRPass): + dfg : DFGAnalysis + + def run_pass(self): + self.analyses_cache.request_analysis(CFGAnalysis) + self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) + + for bb in self.function.get_basic_blocks(): + for inst in bb.instructions.copy(): + if inst.opcode == "phi": + self._handle_phi(inst) + + def _handle_phi(self, phi_inst : IRInstruction) -> bool: + if len(phi_inst.parent.cfg_in) != 1: + return False + + src_bb : IRBasicBlock = phi_inst.parent.cfg_in.first() + assert isinstance(src_bb, IRBasicBlock) + + from_src_bb = filter(lambda x : x[0] == src_bb.label, phi_inst.phi_operands) + operands = list(map(lambda x : x[1], from_src_bb)) + + assert len(operands) == 1 + assert isinstance(operands[0], IRVariable) + phi_inst.output.value = operands[0] + phi_inst.parent.remove_instruction(phi_inst) + + return True + + + diff --git a/vyper/venom/passes/simplify_cfg.py b/vyper/venom/passes/simplify_cfg.py index bcb8b706bd..e52c98f1a3 100644 --- a/vyper/venom/passes/simplify_cfg.py +++ b/vyper/venom/passes/simplify_cfg.py @@ -11,11 +11,9 @@ class SimplifyCFGPass(IRPass): def _merge_blocks(self, a: IRBasicBlock, b: IRBasicBlock): a.instructions.pop() for inst in b.instructions: + assert inst.opcode != "phi", f"Instruction should never be phi {inst}" inst.parent = a - if inst.opcode == "phi": - a.instructions.insert(0, inst) - else: - a.instructions.append(inst) + a.instructions.append(inst) # Update CFG a.cfg_out = b.cfg_out From db53eecc0a9e7ef6376bb66a54d891d1f3a5a5d1 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 12 Aug 2024 13:18:55 +0200 Subject: [PATCH 05/13] fix[venom]: moved phi fix to sccp pass --- vyper/venom/__init__.py | 2 -- .../venom/passes/remove_invalid_phi_nodes.py | 36 ------------------- vyper/venom/passes/sccp/sccp.py | 28 ++++++++++++++- 3 files changed, 27 insertions(+), 39 deletions(-) delete mode 100644 vyper/venom/passes/remove_invalid_phi_nodes.py diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index 990b761b77..afd79fc44f 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -19,7 +19,6 @@ from vyper.venom.passes.sccp import SCCP from vyper.venom.passes.simplify_cfg import SimplifyCFGPass from vyper.venom.passes.store_elimination import StoreElimination -from vyper.venom.passes.remove_invalid_phi_nodes import RemoveInvalidPhiPass from vyper.venom.venom_to_assembly import VenomCompiler DEFAULT_OPT_LEVEL = OptimizationLevel.default() @@ -51,7 +50,6 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: Mem2Var(ac, fn).run_pass() MakeSSA(ac, fn).run_pass() SCCP(ac, fn).run_pass() - RemoveInvalidPhiPass(ac, fn).run_pass() StoreElimination(ac, fn).run_pass() SimplifyCFGPass(ac, fn).run_pass() AlgebraicOptimizationPass(ac, fn).run_pass() diff --git a/vyper/venom/passes/remove_invalid_phi_nodes.py b/vyper/venom/passes/remove_invalid_phi_nodes.py deleted file mode 100644 index 54dfb1f5d8..0000000000 --- a/vyper/venom/passes/remove_invalid_phi_nodes.py +++ /dev/null @@ -1,36 +0,0 @@ -from vyper.venom.basicblock import IRBasicBlock, IRLabel, IRInstruction, IRVariable -from vyper.venom.passes.base_pass import IRPass -from vyper.venom.analysis.dfg import DFGAnalysis -from vyper.venom.analysis.cfg import CFGAnalysis - -class RemoveInvalidPhiPass(IRPass): - dfg : DFGAnalysis - - def run_pass(self): - self.analyses_cache.request_analysis(CFGAnalysis) - self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) - - for bb in self.function.get_basic_blocks(): - for inst in bb.instructions.copy(): - if inst.opcode == "phi": - self._handle_phi(inst) - - def _handle_phi(self, phi_inst : IRInstruction) -> bool: - if len(phi_inst.parent.cfg_in) != 1: - return False - - src_bb : IRBasicBlock = phi_inst.parent.cfg_in.first() - assert isinstance(src_bb, IRBasicBlock) - - from_src_bb = filter(lambda x : x[0] == src_bb.label, phi_inst.phi_operands) - operands = list(map(lambda x : x[1], from_src_bb)) - - assert len(operands) == 1 - assert isinstance(operands[0], IRVariable) - phi_inst.output.value = operands[0] - phi_inst.parent.remove_instruction(phi_inst) - - return True - - - diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index 164d8e241d..d51569cb73 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -74,7 +74,11 @@ def run_pass(self): # self._propagate_variables() - self.analyses_cache.invalidate_analysis(CFGAnalysis) + if self.cfg_dirty: + self.analyses_cache.force_analysis(CFGAnalysis) + self._fix_phi_nodes() + else: + self.analyses_cache.invalidate_analysis(CFGAnalysis) def _calculate_sccp(self, entry: IRBasicBlock): """ @@ -329,6 +333,28 @@ def _replace_constants(self, inst: IRInstruction): if isinstance(lat, IRLiteral): inst.operands[i] = lat + def _fix_phi_nodes(self): + for bb in self.function.get_basic_blocks(): + for inst in bb.instructions.copy(): + if inst.opcode == "phi": + self._fix_phi_node_inst(inst) + + def _fix_phi_node_inst(self, phi_inst: IRInstruction): + if len(phi_inst.parent.cfg_in) != 1: + return + + src_bb: IRBasicBlock = phi_inst.parent.cfg_in.first() + assert isinstance(src_bb, IRBasicBlock) + + from_src_bb = filter(lambda x: x[0] == src_bb.label, phi_inst.phi_operands) + operands = list(map(lambda x: x[1], from_src_bb)) + + assert len(operands) == 1 + assert isinstance(operands[0], IRVariable) + assert phi_inst.output is not None + phi_inst.output.value = operands[0] + phi_inst.parent.remove_instruction(phi_inst) + def _meet(x: LatticeItem, y: LatticeItem) -> LatticeItem: if x == LatticeEnum.TOP: From 37dc99ded4fa01a075c515dfdf0085b22ba9e324 Mon Sep 17 00:00:00 2001 From: plodeada Date: Mon, 2 Sep 2024 15:09:28 +0200 Subject: [PATCH 06/13] better phi fixing --- vyper/venom/passes/sccp/sccp.py | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index d51569cb73..e6da7c9cf8 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -58,12 +58,14 @@ class SCCP(IRPass): work_list: list[WorkListItem] cfg_dirty: bool cfg_in_exec: dict[IRBasicBlock, OrderedSet[IRBasicBlock]] + cfg_phi_fix_needed: OrderedSet[IRBasicBlock] def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction): super().__init__(analyses_cache, function) self.lattice = {} self.work_list: list[WorkListItem] = [] self.cfg_dirty = False + self.cfg_phi_fix_needed = OrderedSet() def run_pass(self): self.fn = self.function @@ -309,6 +311,9 @@ def _replace_constants(self, inst: IRInstruction): inst.opcode = "jmp" inst.operands = [target] self.cfg_dirty = True + for bb in inst.parent.cfg_out: + if bb.label == target: + self.cfg_phi_fix_needed.add(bb) elif inst.opcode in ("assert", "assert_unreachable"): lat = self._eval_from_lattice(inst.operands[0]) @@ -334,10 +339,22 @@ def _replace_constants(self, inst: IRInstruction): inst.operands[i] = lat def _fix_phi_nodes(self): - for bb in self.function.get_basic_blocks(): - for inst in bb.instructions.copy(): - if inst.opcode == "phi": - self._fix_phi_node_inst(inst) + visited: OrderedSet[IRBasicBlock] = OrderedSet() + for bb in self.cfg_phi_fix_needed: + self._fix_phi_bb_r(bb, visited) + + def _fix_phi_bb_r(self, bb: IRBasicBlock, visited: OrderedSet[IRBasicBlock]): + if bb in visited: + return + + for inst in bb.instructions: + if inst.opcode == "phi": + self._fix_phi_node_inst(inst) + + visited.add(bb) + + for bb in bb.cfg_out: + return self._fix_phi_bb_r(bb, visited) def _fix_phi_node_inst(self, phi_inst: IRInstruction): if len(phi_inst.parent.cfg_in) != 1: From 4bd7fb7010d9694aa56d5d8e8ef03bf43bda1950 Mon Sep 17 00:00:00 2001 From: plodeada Date: Mon, 2 Sep 2024 15:48:14 +0200 Subject: [PATCH 07/13] better phi fixing (lint) --- vyper/venom/passes/sccp/sccp.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index e6da7c9cf8..4284ff2b50 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -353,8 +353,8 @@ def _fix_phi_bb_r(self, bb: IRBasicBlock, visited: OrderedSet[IRBasicBlock]): visited.add(bb) - for bb in bb.cfg_out: - return self._fix_phi_bb_r(bb, visited) + for next_bb in bb.cfg_out: + self._fix_phi_bb_r(next_bb, visited) def _fix_phi_node_inst(self, phi_inst: IRInstruction): if len(phi_inst.parent.cfg_in) != 1: From 4f57fa1dc2a94a82e23cfa8d5ec9458e81ee877f Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 18 Sep 2024 10:30:06 -0400 Subject: [PATCH 08/13] simplify the cleanup --- vyper/venom/passes/sccp/sccp.py | 54 ++++++++++++--------------------- 1 file changed, 19 insertions(+), 35 deletions(-) diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index 4284ff2b50..6b9eadc96e 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -56,16 +56,18 @@ class SCCP(IRPass): uses: dict[IRVariable, OrderedSet[IRInstruction]] lattice: Lattice work_list: list[WorkListItem] - cfg_dirty: bool cfg_in_exec: dict[IRBasicBlock, OrderedSet[IRBasicBlock]] - cfg_phi_fix_needed: OrderedSet[IRBasicBlock] + + cfg_dirty: bool + # list of basic blocks whose cfg_in was modified + cfg_in_modified: OrderedSet[IRBasicBlock] def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction): super().__init__(analyses_cache, function) self.lattice = {} self.work_list: list[WorkListItem] = [] self.cfg_dirty = False - self.cfg_phi_fix_needed = OrderedSet() + self.cfg_in_modified = OrderedSet() def run_pass(self): self.fn = self.function @@ -79,8 +81,6 @@ def run_pass(self): if self.cfg_dirty: self.analyses_cache.force_analysis(CFGAnalysis) self._fix_phi_nodes() - else: - self.analyses_cache.invalidate_analysis(CFGAnalysis) def _calculate_sccp(self, entry: IRBasicBlock): """ @@ -310,10 +310,11 @@ def _replace_constants(self, inst: IRInstruction): target = inst.operands[1] inst.opcode = "jmp" inst.operands = [target] + self.cfg_dirty = True for bb in inst.parent.cfg_out: if bb.label == target: - self.cfg_phi_fix_needed.add(bb) + self.cfg_in_modified.add(bb) elif inst.opcode in ("assert", "assert_unreachable"): lat = self._eval_from_lattice(inst.operands[0]) @@ -339,38 +340,21 @@ def _replace_constants(self, inst: IRInstruction): inst.operands[i] = lat def _fix_phi_nodes(self): - visited: OrderedSet[IRBasicBlock] = OrderedSet() - for bb in self.cfg_phi_fix_needed: - self._fix_phi_bb_r(bb, visited) - - def _fix_phi_bb_r(self, bb: IRBasicBlock, visited: OrderedSet[IRBasicBlock]): - if bb in visited: - return + # fix basic blocks whose cfg in was changed + # maybe this should really be done in _visit_phi + to_delete = set() - for inst in bb.instructions: - if inst.opcode == "phi": - self._fix_phi_node_inst(inst) - - visited.add(bb) - - for next_bb in bb.cfg_out: - self._fix_phi_bb_r(next_bb, visited) - - def _fix_phi_node_inst(self, phi_inst: IRInstruction): - if len(phi_inst.parent.cfg_in) != 1: - return - - src_bb: IRBasicBlock = phi_inst.parent.cfg_in.first() - assert isinstance(src_bb, IRBasicBlock) + for bb in self.cfg_in_modified: + cfg_in_labels = OrderedSet(in_bb.label for in_bb in bb.cfg_in) + for inst in bb.instructions: + if inst.opcode != "phi": + break - from_src_bb = filter(lambda x: x[0] == src_bb.label, phi_inst.phi_operands) - operands = list(map(lambda x: x[1], from_src_bb)) + if any(label not in cfg_in_labels for label in inst.get_label_operands()): + to_delete.add(inst) - assert len(operands) == 1 - assert isinstance(operands[0], IRVariable) - assert phi_inst.output is not None - phi_inst.output.value = operands[0] - phi_inst.parent.remove_instruction(phi_inst) + if to_delete: + bb.instructions = [inst for inst in bb.instructions if inst not in to_delete] def _meet(x: LatticeItem, y: LatticeItem) -> LatticeItem: From a39bbe15e11d1c84cbbf80df3ffa7effc358a988 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 18 Sep 2024 11:44:21 -0400 Subject: [PATCH 09/13] transform phis into stores --- vyper/venom/passes/sccp/sccp.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index 6b9eadc96e..cfac6794f8 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -342,19 +342,30 @@ def _replace_constants(self, inst: IRInstruction): def _fix_phi_nodes(self): # fix basic blocks whose cfg in was changed # maybe this should really be done in _visit_phi - to_delete = set() + needs_sort = False - for bb in self.cfg_in_modified: + for bb in self.fn.get_basic_blocks(): cfg_in_labels = OrderedSet(in_bb.label for in_bb in bb.cfg_in) + for inst in bb.instructions: if inst.opcode != "phi": break + needs_sort |= self._fix_phi_inst(inst, cfg_in_labels) + + # move phi instructions to the top of the block + if needs_sort: + bb.instructions.sort(key=lambda inst: inst.opcode != "phi") + + def _fix_phi_inst(self, inst: IRInstruction, cfg_in_labels: OrderedSet): + operands = [op for label, op in inst.phi_operands if label in cfg_in_labels] - if any(label not in cfg_in_labels for label in inst.get_label_operands()): - to_delete.add(inst) + if len(operands) != 1: + return False - if to_delete: - bb.instructions = [inst for inst in bb.instructions if inst not in to_delete] + assert inst.output is not None + inst.opcode = "store" + inst.operands = operands + return True def _meet(x: LatticeItem, y: LatticeItem) -> LatticeItem: From 1128c606a47c36552a12210617b15dfa8335911d Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sun, 22 Sep 2024 19:52:51 -0400 Subject: [PATCH 10/13] simplify some code --- vyper/venom/passes/simplify_cfg.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/vyper/venom/passes/simplify_cfg.py b/vyper/venom/passes/simplify_cfg.py index e52c98f1a3..70e09716f2 100644 --- a/vyper/venom/passes/simplify_cfg.py +++ b/vyper/venom/passes/simplify_cfg.py @@ -9,23 +9,24 @@ class SimplifyCFGPass(IRPass): visited: OrderedSet def _merge_blocks(self, a: IRBasicBlock, b: IRBasicBlock): - a.instructions.pop() + a.instructions.pop() # pop terminating instructoin for inst in b.instructions: - assert inst.opcode != "phi", f"Instruction should never be phi {inst}" + assert inst.opcode != "phi", f"Instruction should never be phi {b}" inst.parent = a a.instructions.append(inst) # Update CFG a.cfg_out = b.cfg_out - if len(b.cfg_out) > 0: - for next_bb in b.cfg_out: - next_bb.remove_cfg_in(b) - next_bb.add_cfg_in(a) - - for inst in next_bb.instructions: - if inst.opcode != "phi": - break - inst.operands[inst.operands.index(b.label)] = a.label + + for next_bb in a.cfg_out: + next_bb.remove_cfg_in(b) + next_bb.add_cfg_in(a) + + 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.function.remove_basic_block(b) From c552757ff5cc9c000f869107a2857c2e23354d48 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 26 Sep 2024 18:41:00 -0400 Subject: [PATCH 11/13] fix a comment --- vyper/venom/passes/simplify_cfg.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/venom/passes/simplify_cfg.py b/vyper/venom/passes/simplify_cfg.py index 70e09716f2..1409f43947 100644 --- a/vyper/venom/passes/simplify_cfg.py +++ b/vyper/venom/passes/simplify_cfg.py @@ -9,7 +9,7 @@ class SimplifyCFGPass(IRPass): visited: OrderedSet def _merge_blocks(self, a: IRBasicBlock, b: IRBasicBlock): - a.instructions.pop() # pop terminating instructoin + a.instructions.pop() # pop terminating instruction for inst in b.instructions: assert inst.opcode != "phi", f"Instruction should never be phi {b}" inst.parent = a From ab38dea3dbdd766f75503a6641faff4827f93de8 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 26 Sep 2024 20:59:45 -0400 Subject: [PATCH 12/13] add test for cfg simplification --- .../unit/compiler/venom/test_simplify_cfg.py | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 tests/unit/compiler/venom/test_simplify_cfg.py diff --git a/tests/unit/compiler/venom/test_simplify_cfg.py b/tests/unit/compiler/venom/test_simplify_cfg.py new file mode 100644 index 0000000000..c4bdbb263b --- /dev/null +++ b/tests/unit/compiler/venom/test_simplify_cfg.py @@ -0,0 +1,49 @@ +from vyper.venom.analysis.analysis import IRAnalysesCache +from vyper.venom.basicblock import IRBasicBlock, IRLabel, IRLiteral +from vyper.venom.context import IRContext +from vyper.venom.passes.sccp import SCCP +from vyper.venom.passes.simplify_cfg import SimplifyCFGPass + + +def test_phi_reduction_after_block_pruning(): + ctx = IRContext() + fn = ctx.create_function("_global") + + bb = fn.get_basic_block() + + br1 = IRBasicBlock(IRLabel("then"), fn) + fn.append_basic_block(br1) + br2 = IRBasicBlock(IRLabel("else"), fn) + fn.append_basic_block(br2) + + join = IRBasicBlock(IRLabel("join"), fn) + fn.append_basic_block(join) + + true = IRLiteral(1) + bb.append_instruction("jnz", true, br1.label, br2.label) + + op1 = br1.append_instruction("store", 1) + op2 = br2.append_instruction("store", 2) + + br1.append_instruction("jmp", join.label) + br2.append_instruction("jmp", join.label) + + join.append_instruction("phi", br1.label, op1, br2.label, op2) + join.append_instruction("stop") + + ac = IRAnalysesCache(fn) + SCCP(ac, fn).run_pass() + SimplifyCFGPass(ac, fn).run_pass() + + bbs = list(fn.get_basic_blocks()) + + assert len(bbs) == 1 + final_bb = bbs[0] + + inst0, inst1, inst2 = final_bb.instructions + + assert inst0.opcode == "store" + assert inst0.operands == [IRLiteral(1)] + assert inst1.opcode == "store" + assert inst1.operands == [inst0.output] + assert inst2.opcode == "stop" From 4599ea1310863afe9cbd1fe810e4a945d2dc2ccc Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 26 Sep 2024 20:59:52 -0400 Subject: [PATCH 13/13] add test for phi reduction --- tests/unit/compiler/venom/test_sccp.py | 31 ++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/unit/compiler/venom/test_sccp.py b/tests/unit/compiler/venom/test_sccp.py index e65839136e..478acc1079 100644 --- a/tests/unit/compiler/venom/test_sccp.py +++ b/tests/unit/compiler/venom/test_sccp.py @@ -211,3 +211,34 @@ def test_cont_phi_const_case(): assert sccp.lattice[IRVariable("%5", version=1)].value == 106 assert sccp.lattice[IRVariable("%5", version=2)].value == 97 assert sccp.lattice[IRVariable("%5")].value == 2 + + +def test_phi_reduction_after_unreachable_block(): + ctx = IRContext() + fn = ctx.create_function("_global") + + bb = fn.get_basic_block() + + br1 = IRBasicBlock(IRLabel("then"), fn) + fn.append_basic_block(br1) + join = IRBasicBlock(IRLabel("join"), fn) + fn.append_basic_block(join) + + op = bb.append_instruction("store", 1) + true = IRLiteral(1) + bb.append_instruction("jnz", true, br1.label, join.label) + + op1 = br1.append_instruction("store", 2) + + br1.append_instruction("jmp", join.label) + + join.append_instruction("phi", bb.label, op, br1.label, op1) + join.append_instruction("stop") + + ac = IRAnalysesCache(fn) + SCCP(ac, fn).run_pass() + + assert join.instructions[0].opcode == "store", join.instructions[0] + assert join.instructions[0].operands == [op1] + + assert join.instructions[1].opcode == "stop"