From 51cbd356d4b15823eb39767a8e141c464c051197 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 4 Dec 2024 06:58:22 -0500 Subject: [PATCH 01/39] feat[venom]: mark loads as non-volatile this commit marks load instructions (`mload`, `sload`, etc) as non-volatile, allowing them to be removed in the `remove_unused_variables` pass. --- vyper/venom/basicblock.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/vyper/venom/basicblock.py b/vyper/venom/basicblock.py index 968ce42bdf..a61eb54acb 100644 --- a/vyper/venom/basicblock.py +++ b/vyper/venom/basicblock.py @@ -16,14 +16,10 @@ "create", "create2", "invoke", - "sload", "sstore", - "iload", "istore", - "tload", "tstore", "mstore", - "mload", "calldatacopy", "mcopy", "extcodecopy", From 9befa121487952ba6050e06d980fbd6296cdc785 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 7 Dec 2024 13:16:47 -0500 Subject: [PATCH 02/39] handle msize --- vyper/venom/passes/remove_unused_variables.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index 3ce5bdf2d3..7f52ab3659 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -1,4 +1,5 @@ from vyper.utils import OrderedSet +from vyper.venom import effects from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis from vyper.venom.basicblock import IRInstruction from vyper.venom.passes.base_pass import IRPass @@ -11,10 +12,18 @@ class RemoveUnusedVariablesPass(IRPass): dfg: DFGAnalysis work_list: OrderedSet[IRInstruction] + reads_msize: bool def run_pass(self): self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) + self.reads_msize = False + for bb in self.function.get_basic_blocks(): + for inst in bb.instructions: + if inst.opcode == "msize": + self.reads_msize = True + break + work_list = OrderedSet() self.work_list = work_list @@ -33,6 +42,9 @@ def _process_instruction(self, inst): return if inst.is_volatile or inst.is_bb_terminator: return + if self.reads_msize and effects.MSIZE in inst.get_write_effects(): + return + uses = self.dfg.get_uses(inst.output) if len(uses) > 0: return From fa851937be5736592cd6cf70c697a1660743d161 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 7 Dec 2024 13:21:11 -0500 Subject: [PATCH 03/39] fix msize effects --- vyper/venom/effects.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vyper/venom/effects.py b/vyper/venom/effects.py index 97cffe2cb2..5833860789 100644 --- a/vyper/venom/effects.py +++ b/vyper/venom/effects.py @@ -83,11 +83,11 @@ def __iter__(self): writes = _writes.copy() for k, v in reads.items(): - if MEMORY in v: + if MEMORY in v or IMMUTABLES in b: if k not in writes: writes[k] = EMPTY writes[k] |= MSIZE for k, v in writes.items(): - if MEMORY in v: + if MEMORY in v or IMMUTABLES in b: writes[k] |= MSIZE From ea2ba1dfb57f9e1d15206298a280c7180c93c7f9 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 7 Dec 2024 17:11:58 -0500 Subject: [PATCH 04/39] fix typo --- vyper/venom/effects.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vyper/venom/effects.py b/vyper/venom/effects.py index 5833860789..e551f06f41 100644 --- a/vyper/venom/effects.py +++ b/vyper/venom/effects.py @@ -83,11 +83,11 @@ def __iter__(self): writes = _writes.copy() for k, v in reads.items(): - if MEMORY in v or IMMUTABLES in b: + if MEMORY in v or IMMUTABLES in v: if k not in writes: writes[k] = EMPTY writes[k] |= MSIZE for k, v in writes.items(): - if MEMORY in v or IMMUTABLES in b: + if MEMORY in v or IMMUTABLES in v: writes[k] |= MSIZE From e3575a016288c7bbefd29f99607d63611043ed68 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 7 Dec 2024 17:18:45 -0500 Subject: [PATCH 05/39] add a note --- vyper/venom/passes/remove_unused_variables.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index 7f52ab3659..4258631012 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -42,6 +42,8 @@ def _process_instruction(self, inst): return if inst.is_volatile or inst.is_bb_terminator: return + # TODO: improve this, we only need the fence if the msize is reachable + # from this basic block. if self.reads_msize and effects.MSIZE in inst.get_write_effects(): return From 12b96040e2406c8c66634b37cb085fcdfce5288c Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 7 Dec 2024 17:27:34 -0500 Subject: [PATCH 06/39] improve the analysis --- vyper/venom/analysis/__init__.py | 1 + vyper/venom/analysis/cfg.py | 8 +++- vyper/venom/analysis/reachable.py | 37 +++++++++++++++++++ vyper/venom/passes/remove_unused_variables.py | 12 ++++-- 4 files changed, 53 insertions(+), 5 deletions(-) create mode 100644 vyper/venom/analysis/reachable.py diff --git a/vyper/venom/analysis/__init__.py b/vyper/venom/analysis/__init__.py index 4870de3fb7..8714417548 100644 --- a/vyper/venom/analysis/__init__.py +++ b/vyper/venom/analysis/__init__.py @@ -4,3 +4,4 @@ from .dominators import DominatorTreeAnalysis from .equivalent_vars import VarEquivalenceAnalysis from .liveness import LivenessAnalysis +from .reachable import ReachableAnalysis diff --git a/vyper/venom/analysis/cfg.py b/vyper/venom/analysis/cfg.py index 700fd73f26..354f77de88 100644 --- a/vyper/venom/analysis/cfg.py +++ b/vyper/venom/analysis/cfg.py @@ -51,10 +51,16 @@ def dfs_walk(self) -> Iterator[IRBasicBlock]: return iter(self._dfs) def invalidate(self): - from vyper.venom.analysis import DFGAnalysis, DominatorTreeAnalysis, LivenessAnalysis + from vyper.venom.analysis import ( + DFGAnalysis, + DominatorTreeAnalysis, + LivenessAnalysis, + ReachableAnalysis, + ) self.analyses_cache.invalidate_analysis(DominatorTreeAnalysis) self.analyses_cache.invalidate_analysis(LivenessAnalysis) + self.analyses_cache.invalidate_analysis(ReachableAnalysis) self._dfs = None diff --git a/vyper/venom/analysis/reachable.py b/vyper/venom/analysis/reachable.py new file mode 100644 index 0000000000..b0035aff1f --- /dev/null +++ b/vyper/venom/analysis/reachable.py @@ -0,0 +1,37 @@ +from vyper.utils import OrderedSet +from vyper.venom.analysis import IRAnalysis +from vyper.venom.analysis.cfg import CFGAnalysis +from vyper.venom.basicblock import IRBasicBlock + + +class ReachableAnalysis(IRAnalysis): + """ + Compute control flow graph information for each basic block in the function. + """ + + reachable: dict[IRBasicBlock, OrderedSet[IRBasicBlock]] + + def analyze(self) -> None: + self.analyses_cache.request_analysis(CFGAnalysis) + self.reachable = {} + + def _compute_reachable_r(self, bb): + if bb in self.reachable: + return + + downstream = bb.cfg_out.copy() + for out_bb in bb.cfg_out: + self._compute_reachable_r(out_bb) + downstream.update(self.reachable[out_bb]) + self.reachable[bb] = downstream + + def invalidate(self): + from vyper.venom.analysis import DFGAnalysis, DominatorTreeAnalysis, LivenessAnalysis + + self.analyses_cache.invalidate_analysis(DominatorTreeAnalysis) + self.analyses_cache.invalidate_analysis(LivenessAnalysis) + + self._dfs = None + + # be conservative - assume cfg invalidation invalidates dfg + self.analyses_cache.invalidate_analysis(DFGAnalysis) diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index 4258631012..98a99ce5c4 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -1,6 +1,6 @@ from vyper.utils import OrderedSet from vyper.venom import effects -from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis +from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis, ReachableAnalysis from vyper.venom.basicblock import IRInstruction from vyper.venom.passes.base_pass import IRPass @@ -16,12 +16,13 @@ class RemoveUnusedVariablesPass(IRPass): def run_pass(self): self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) + self.reachable = self.analyses_cache.request_analysis(ReachableAnalysis) - self.reads_msize = False + self.reads_msize = set() for bb in self.function.get_basic_blocks(): for inst in bb.instructions: if inst.opcode == "msize": - self.reads_msize = True + self.reads_msize.add(bb) break work_list = OrderedSet() @@ -44,7 +45,10 @@ def _process_instruction(self, inst): return # TODO: improve this, we only need the fence if the msize is reachable # from this basic block. - if self.reads_msize and effects.MSIZE in inst.get_write_effects(): + bb = inst.parent + if effects.MSIZE in inst.get_write_effects() and any( + reachable_bb in self.reachable[bb] for reachable_bb in self.reads_msize + ): return uses = self.dfg.get_uses(inst.output) From 0baece8802323f77532410287f96d4449a1d2d36 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 7 Dec 2024 17:29:08 -0500 Subject: [PATCH 07/39] typo --- vyper/venom/passes/remove_unused_variables.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index 98a99ce5c4..a4d4453ea4 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -16,7 +16,7 @@ class RemoveUnusedVariablesPass(IRPass): def run_pass(self): self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) - self.reachable = self.analyses_cache.request_analysis(ReachableAnalysis) + self.reachable = self.analyses_cache.request_analysis(ReachableAnalysis).reachable self.reads_msize = set() for bb in self.function.get_basic_blocks(): From 00d4f6f667e131bf16a981bb037aeb5cf45f15a3 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 7 Dec 2024 17:32:30 -0500 Subject: [PATCH 08/39] fix: defaultdict --- vyper/venom/analysis/reachable.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/vyper/venom/analysis/reachable.py b/vyper/venom/analysis/reachable.py index b0035aff1f..76adf533e1 100644 --- a/vyper/venom/analysis/reachable.py +++ b/vyper/venom/analysis/reachable.py @@ -1,4 +1,5 @@ from vyper.utils import OrderedSet +from collections import defaultdict from vyper.venom.analysis import IRAnalysis from vyper.venom.analysis.cfg import CFGAnalysis from vyper.venom.basicblock import IRBasicBlock @@ -13,16 +14,18 @@ class ReachableAnalysis(IRAnalysis): def analyze(self) -> None: self.analyses_cache.request_analysis(CFGAnalysis) - self.reachable = {} + self.reachable = defaultdict(OrderedSet) def _compute_reachable_r(self, bb): if bb in self.reachable: return downstream = bb.cfg_out.copy() + for out_bb in bb.cfg_out: self._compute_reachable_r(out_bb) downstream.update(self.reachable[out_bb]) + self.reachable[bb] = downstream def invalidate(self): From d8f6a2c97ee6f0c6321238b27ce67f6da8fe72eb Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 7 Dec 2024 17:40:44 -0500 Subject: [PATCH 09/39] fix the analysis and the pass --- vyper/venom/analysis/reachable.py | 12 ++++++---- vyper/venom/passes/remove_unused_variables.py | 23 ++++++++++--------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/vyper/venom/analysis/reachable.py b/vyper/venom/analysis/reachable.py index 76adf533e1..9607e3aae9 100644 --- a/vyper/venom/analysis/reachable.py +++ b/vyper/venom/analysis/reachable.py @@ -1,5 +1,6 @@ -from vyper.utils import OrderedSet from collections import defaultdict + +from vyper.utils import OrderedSet from vyper.venom.analysis import IRAnalysis from vyper.venom.analysis.cfg import CFGAnalysis from vyper.venom.basicblock import IRBasicBlock @@ -16,17 +17,18 @@ def analyze(self) -> None: self.analyses_cache.request_analysis(CFGAnalysis) self.reachable = defaultdict(OrderedSet) + self._compute_reachable_r(self.function.entry) + def _compute_reachable_r(self, bb): if bb in self.reachable: return - downstream = bb.cfg_out.copy() + s = bb.cfg_out.copy() + self.reachable[bb] = s for out_bb in bb.cfg_out: self._compute_reachable_r(out_bb) - downstream.update(self.reachable[out_bb]) - - self.reachable[bb] = downstream + s.update(self.reachable[out_bb]) def invalidate(self): from vyper.venom.analysis import DFGAnalysis, DominatorTreeAnalysis, LivenessAnalysis diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index a4d4453ea4..36beac4c83 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -18,12 +18,13 @@ def run_pass(self): self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) self.reachable = self.analyses_cache.request_analysis(ReachableAnalysis).reachable - self.reads_msize = set() + self.reads_msize = {} + self.instruction_index = {} for bb in self.function.get_basic_blocks(): - for inst in bb.instructions: - if inst.opcode == "msize": - self.reads_msize.add(bb) - break + for idx, inst in enumerate(bb.instructions): + self.instruction_index[inst] = idx + if inst.opcode == "msize" and bb not in self.reads_msize: + self.reads_msize[bb] = idx work_list = OrderedSet() self.work_list = work_list @@ -43,13 +44,13 @@ def _process_instruction(self, inst): return if inst.is_volatile or inst.is_bb_terminator: return - # TODO: improve this, we only need the fence if the msize is reachable - # from this basic block. bb = inst.parent - if effects.MSIZE in inst.get_write_effects() and any( - reachable_bb in self.reachable[bb] for reachable_bb in self.reads_msize - ): - return + if effects.MSIZE in inst.get_write_effects(): + # msize after memory touch + if bb in self.reads_msize and self.instruction_index[inst] < self.reads_msize[bb]: + return + if any(reachable_bb in self.reachable[bb] for reachable_bb in self.reads_msize): + return uses = self.dfg.get_uses(inst.output) if len(uses) > 0: From 2b95c7122c8993cd31a07129600ac91b0627c208 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 9 Dec 2024 09:28:41 +0100 Subject: [PATCH 10/39] basic tests --- .../unit/compiler/venom/test_removeunused.py | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 tests/unit/compiler/venom/test_removeunused.py diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py new file mode 100644 index 0000000000..7b2f8b730e --- /dev/null +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -0,0 +1,45 @@ +from vyper.venom.context import IRContext +from vyper.venom.analysis.analysis import IRAnalysesCache +from vyper.venom.passes import RemoveUnusedVariablesPass + +def test_removeunused_msize_basic(): + ctx = IRContext() + fn = ctx.create_function("_global") + + bb = fn.get_basic_block() + bb.append_instruction("mload", 32) + msize = bb.append_instruction("msize") + bb.append_instruction("mload", 64) + bb.append_instruction("return", msize) + + ac = IRAnalysesCache(fn) + RemoveUnusedVariablesPass(ac, fn).run_pass() + + assert bb.instructions[0].opcode == "mload" + assert bb.instructions[0].operands[0].value == 32 + assert bb.instructions[1].opcode == "msize" + assert bb.instructions[2].opcode == "return" + + +def test_removeunused_msize_two_msizes(): + ctx = IRContext() + fn = ctx.create_function("_global") + + bb = fn.get_basic_block() + bb.append_instruction("mload", 32) + msize1 = bb.append_instruction("msize") + bb.append_instruction("mload", 64) + msize2 = bb.append_instruction("msize") + bb.append_instruction("return", msize1, msize2) + + ac = IRAnalysesCache(fn) + RemoveUnusedVariablesPass(ac, fn).run_pass() + + assert bb.instructions[0].opcode == "mload" + assert bb.instructions[0].operands[0] == 32 + assert bb.instructions[1].opcode == "msize" + assert bb.instructions[2].opcode == "mload" + assert bb.instructions[2].operands[0] == 64 + assert bb.instructions[3].opcode == "msize" + assert bb.instructions[4].opcode == "return" + From 22de2e32668dfb516d78fb4aa3e528b63c58c416 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 9 Dec 2024 09:55:53 +0100 Subject: [PATCH 11/39] loop test --- .../unit/compiler/venom/test_removeunused.py | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index 7b2f8b730e..a95a11c0ab 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -36,10 +36,27 @@ def test_removeunused_msize_two_msizes(): RemoveUnusedVariablesPass(ac, fn).run_pass() assert bb.instructions[0].opcode == "mload" - assert bb.instructions[0].operands[0] == 32 + assert bb.instructions[0].operands[0].value == 32 assert bb.instructions[1].opcode == "msize" assert bb.instructions[2].opcode == "mload" - assert bb.instructions[2].operands[0] == 64 + assert bb.instructions[2].operands[0].value == 64 assert bb.instructions[3].opcode == "msize" assert bb.instructions[4].opcode == "return" + +def test_removeunused_msize_loop(): + ctx = IRContext() + fn = ctx.create_function("_global") + + bb = fn.get_basic_block() + msize = bb.append_instruction("msize") + bb.append_instruction("mload", msize) + bb.append_instruction("jmp", bb.label) + + ac = IRAnalysesCache(fn) + RemoveUnusedVariablesPass(ac, fn).run_pass() + + assert bb.instructions[0].opcode == "msize" + assert bb.instructions[1].opcode == "mload" + assert bb.instructions[1].operands[0] == msize + assert bb.instructions[2].opcode == "jmp" From 14894a346e008f6fc3bb8276a7c281d80ff45239 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 9 Dec 2024 10:37:46 +0100 Subject: [PATCH 12/39] unused msize --- .../unit/compiler/venom/test_removeunused.py | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index a95a11c0ab..9350c141f6 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -1,7 +1,8 @@ -from vyper.venom.context import IRContext from vyper.venom.analysis.analysis import IRAnalysesCache +from vyper.venom.context import IRContext from vyper.venom.passes import RemoveUnusedVariablesPass + def test_removeunused_msize_basic(): ctx = IRContext() fn = ctx.create_function("_global") @@ -31,7 +32,7 @@ def test_removeunused_msize_two_msizes(): bb.append_instruction("mload", 64) msize2 = bb.append_instruction("msize") bb.append_instruction("return", msize1, msize2) - + ac = IRAnalysesCache(fn) RemoveUnusedVariablesPass(ac, fn).run_pass() @@ -60,3 +61,35 @@ def test_removeunused_msize_loop(): assert bb.instructions[1].opcode == "mload" assert bb.instructions[1].operands[0] == msize assert bb.instructions[2].opcode == "jmp" + + +# Should this work? +def test_removeunused_unused_msize_loop(): + ctx = IRContext() + fn = ctx.create_function("_global") + + bb = fn.get_basic_block() + bb.append_instruction("msize") + bb.append_instruction("mload", 10) + bb.append_instruction("jmp", bb.label) + + ac = IRAnalysesCache(fn) + RemoveUnusedVariablesPass(ac, fn).run_pass() + + assert bb.instructions[0].opcode == "jmp" + + +# Should this work? +def test_removeunused_unused_msize(): + ctx = IRContext() + fn = ctx.create_function("_global") + + bb = fn.get_basic_block() + bb.append_instruction("mload", 10) + bb.append_instruction("msize") + bb.append_instruction("stop") + + ac = IRAnalysesCache(fn) + RemoveUnusedVariablesPass(ac, fn).run_pass() + + assert bb.instructions[0].opcode == "stop", bb From f071b11cee8cbc54310af88b01214d6ee8ebc521 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 9 Dec 2024 14:35:42 +0100 Subject: [PATCH 13/39] test without the msize --- .../unit/compiler/venom/test_removeunused.py | 61 ++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index 9350c141f6..7c35714c3e 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -1,6 +1,7 @@ from vyper.venom.analysis.analysis import IRAnalysesCache +from vyper.venom.basicblock import IRBasicBlock, IRLabel from vyper.venom.context import IRContext -from vyper.venom.passes import RemoveUnusedVariablesPass +from vyper.venom.passes import MakeSSA, RemoveUnusedVariablesPass def test_removeunused_msize_basic(): @@ -93,3 +94,61 @@ def test_removeunused_unused_msize(): RemoveUnusedVariablesPass(ac, fn).run_pass() assert bb.instructions[0].opcode == "stop", bb + + +def test_removeunused_basic(): + ctx = IRContext() + fn = ctx.create_function("_global") + + bb = fn.get_basic_block() + var1 = bb.append_instruction("add", 10, 20) + bb.append_instruction("add", var1, 10) + bb.append_instruction("mstore", var1, 20) + bb.append_instruction("stop") + + ac = IRAnalysesCache(fn) + RemoveUnusedVariablesPass(ac, fn).run_pass() + + assert bb.instructions[0].opcode == "add" + assert bb.instructions[0].operands[0].value == 10 + assert bb.instructions[0].operands[1].value == 20 + assert bb.instructions[1].opcode == "mstore" + assert bb.instructions[1].operands[0] == var1 + assert bb.instructions[1].operands[1].value == 20 + assert bb.instructions[2].opcode == "stop" + + +def test_removeunused_loop(): + ctx = IRContext() + fn = ctx.create_function("_global") + + bb = fn.get_basic_block() + after_bb = IRBasicBlock(IRLabel("after"), fn) + fn.append_basic_block(after_bb) + + var1 = bb.append_instruction("store", 10) + bb.append_instruction("jmp", after_bb.label) + + var2 = fn.get_next_variable() + var_phi = after_bb.append_instruction("phi", bb.label, var1, after_bb.label, var2) + after_bb.append_instruction("add", var_phi, 1, ret=var2) + after_bb.append_instruction("add", var2, var_phi) + after_bb.append_instruction("mstore", var2, 10) + after_bb.append_instruction("jmp", after_bb.label) + + ac = IRAnalysesCache(fn) + RemoveUnusedVariablesPass(ac, fn).run_pass() + + assert bb.instructions[0].opcode == "store" + assert bb.instructions[0].operands[0].value == 10 + assert bb.instructions[1].opcode == "jmp" + + assert after_bb.instructions[0].opcode == "phi" + assert after_bb.instructions[1].opcode == "add" + assert after_bb.instructions[1].operands[0] == var_phi + assert after_bb.instructions[1].operands[1].value == 1 + assert after_bb.instructions[2].opcode == "mstore" + assert after_bb.instructions[2].operands[0] == var2 + assert after_bb.instructions[2].operands[1].value == 10 + assert after_bb.instructions[3].opcode == "jmp" + assert after_bb.instructions[3].operands[0] == after_bb.label From 6cef92b65a626acfd11ba5c500e0e89e28e1a2b1 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 9 Dec 2024 14:48:44 +0100 Subject: [PATCH 14/39] fix of the order msize --- vyper/venom/passes/remove_unused_variables.py | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index 36beac4c83..252f66999b 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -1,7 +1,7 @@ from vyper.utils import OrderedSet from vyper.venom import effects from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis, ReachableAnalysis -from vyper.venom.basicblock import IRInstruction +from vyper.venom.basicblock import IRBasicBlock, IRInstruction from vyper.venom.passes.base_pass import IRPass @@ -12,19 +12,20 @@ class RemoveUnusedVariablesPass(IRPass): dfg: DFGAnalysis work_list: OrderedSet[IRInstruction] - reads_msize: bool + last_msize_position: dict[IRBasicBlock, int] def run_pass(self): self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) self.reachable = self.analyses_cache.request_analysis(ReachableAnalysis).reachable - self.reads_msize = {} + self.last_msize_position = {} self.instruction_index = {} for bb in self.function.get_basic_blocks(): - for idx, inst in enumerate(bb.instructions): + for idx in range(len(bb.instructions) - 1, -1, -1): + inst = bb.instructions[idx] self.instruction_index[inst] = idx - if inst.opcode == "msize" and bb not in self.reads_msize: - self.reads_msize[bb] = idx + if inst.opcode == "msize" and bb not in self.last_msize_position: + self.last_msize_position[bb] = idx work_list = OrderedSet() self.work_list = work_list @@ -47,9 +48,12 @@ def _process_instruction(self, inst): bb = inst.parent if effects.MSIZE in inst.get_write_effects(): # msize after memory touch - if bb in self.reads_msize and self.instruction_index[inst] < self.reads_msize[bb]: + if ( + bb in self.last_msize_position + and self.instruction_index[inst] < self.last_msize_position[bb] + ): return - if any(reachable_bb in self.reachable[bb] for reachable_bb in self.reads_msize): + if any(reachable_bb in self.reachable[bb] for reachable_bb in self.last_msize_position): return uses = self.dfg.get_uses(inst.output) From 799964f12e2fd5b59efbec6920fbd5dcc7ccd8e5 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 9 Dec 2024 09:07:15 -0500 Subject: [PATCH 15/39] clean up for range --- vyper/venom/passes/remove_unused_variables.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index 252f66999b..d4af5fa99c 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -21,10 +21,10 @@ def run_pass(self): self.last_msize_position = {} self.instruction_index = {} for bb in self.function.get_basic_blocks(): - for idx in range(len(bb.instructions) - 1, -1, -1): + for idx, inst in enumerate(bb.instructions): inst = bb.instructions[idx] self.instruction_index[inst] = idx - if inst.opcode == "msize" and bb not in self.last_msize_position: + if inst.opcode == "msize": self.last_msize_position[bb] = idx work_list = OrderedSet() From 2433a4a67f0e50b9359b29a03a8d28e008106b33 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 9 Dec 2024 09:13:57 -0500 Subject: [PATCH 16/39] fix: remove msize from index --- .../unit/compiler/venom/test_removeunused.py | 2 +- vyper/venom/passes/remove_unused_variables.py | 32 +++++++++++++------ 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index 7c35714c3e..cfa2efd093 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -1,7 +1,7 @@ from vyper.venom.analysis.analysis import IRAnalysesCache from vyper.venom.basicblock import IRBasicBlock, IRLabel from vyper.venom.context import IRContext -from vyper.venom.passes import MakeSSA, RemoveUnusedVariablesPass +from vyper.venom.passes import RemoveUnusedVariablesPass def test_removeunused_msize_basic(): diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index d4af5fa99c..2196b20185 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -1,3 +1,5 @@ +from collections import defaultdict + from vyper.utils import OrderedSet from vyper.venom import effects from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis, ReachableAnalysis @@ -12,20 +14,20 @@ class RemoveUnusedVariablesPass(IRPass): dfg: DFGAnalysis work_list: OrderedSet[IRInstruction] - last_msize_position: dict[IRBasicBlock, int] + msizes: dict[IRBasicBlock, int] def run_pass(self): self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) self.reachable = self.analyses_cache.request_analysis(ReachableAnalysis).reachable - self.last_msize_position = {} + self.msizes = defaultdict(list) self.instruction_index = {} for bb in self.function.get_basic_blocks(): for idx, inst in enumerate(bb.instructions): inst = bb.instructions[idx] self.instruction_index[inst] = idx if inst.opcode == "msize": - self.last_msize_position[bb] = idx + self.msizes[bb].append(idx) work_list = OrderedSet() self.work_list = work_list @@ -37,23 +39,29 @@ def run_pass(self): inst = work_list.pop() self._process_instruction(inst) + for bb in self.function.get_basic_blocks(): + bb.clean_garbage() + self.analyses_cache.invalidate_analysis(LivenessAnalysis) self.analyses_cache.invalidate_analysis(DFGAnalysis) + def get_last_msize(self, bb): + msizes = self.msizes[bb] + if len(msizes) == 0: + return None + return max(msizes) + def _process_instruction(self, inst): if inst.output is None: return if inst.is_volatile or inst.is_bb_terminator: return bb = inst.parent - if effects.MSIZE in inst.get_write_effects(): + if effects.MSIZE in inst.get_write_effects() and bb in self.msizes: # msize after memory touch - if ( - bb in self.last_msize_position - and self.instruction_index[inst] < self.last_msize_position[bb] - ): + if self.instruction_index[inst] < self.get_last_msize(bb): return - if any(reachable_bb in self.reachable[bb] for reachable_bb in self.last_msize_position): + if any(reachable_bb in self.reachable[bb] for reachable_bb in self.msizes): return uses = self.dfg.get_uses(inst.output) @@ -65,4 +73,8 @@ def _process_instruction(self, inst): new_uses = self.dfg.get_uses(operand) self.work_list.addmany(new_uses) - inst.parent.remove_instruction(inst) + # if we remove an msize, update the index + if inst.opcode == "msize": + self.msizes[bb].remove(self.instruction_index[inst]) + + inst.parent.mark_for_removal(inst) From 7235a135776e26ebba3f7c210bcb11d395f2588a Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 9 Dec 2024 09:24:53 -0500 Subject: [PATCH 17/39] swipe mark_for_removal --- vyper/venom/basicblock.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/vyper/venom/basicblock.py b/vyper/venom/basicblock.py index a61eb54acb..c2b19a09bc 100644 --- a/vyper/venom/basicblock.py +++ b/vyper/venom/basicblock.py @@ -196,7 +196,7 @@ class IRInstruction: opcode: str operands: list[IROperand] - output: Optional[IROperand] + output: Optional[IRVariable] # set of live variables at this instruction liveness: OrderedSet[IRVariable] parent: "IRBasicBlock" @@ -208,7 +208,7 @@ def __init__( self, opcode: str, operands: list[IROperand] | Iterator[IROperand], - output: Optional[IROperand] = None, + output: Optional[IRVariable] = None, ): assert isinstance(opcode, str), "opcode must be an str" assert isinstance(operands, list | Iterator), "operands must be a list" @@ -442,6 +442,8 @@ def __init__(self, label: IRLabel, parent: "IRFunction") -> None: self.out_vars = OrderedSet() self.is_reachable = False + self._garbage_instructions: set[IRInstruction] = set() + def add_cfg_in(self, bb: "IRBasicBlock") -> None: self.cfg_in.add(bb) @@ -516,13 +518,20 @@ def insert_instruction(self, instruction: IRInstruction, index: Optional[int] = instruction.error_msg = self.parent.error_msg self.instructions.insert(index, instruction) + def mark_for_removal(self, instruction: IRInstruction) -> None: + self._garbage_instructions.add(instruction) + + def clear_dead_instructions(self) -> None: + if len(self._garbage_instructions) > 0: + self.instructions = [ + inst for inst in self.instructions if inst not in self._garbage_instructions + ] + self._garbage_instructions.clear() + def remove_instruction(self, instruction: IRInstruction) -> None: assert isinstance(instruction, IRInstruction), "instruction must be an IRInstruction" self.instructions.remove(instruction) - def clear_instructions(self) -> None: - self.instructions = [] - @property def phi_instructions(self) -> Iterator[IRInstruction]: for inst in self.instructions: From 0fe354abe320f5f025d9881389277c39a151bbbf Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 9 Dec 2024 09:37:07 -0500 Subject: [PATCH 18/39] small fixes --- vyper/venom/passes/remove_unused_variables.py | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index 2196b20185..91da40fc28 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -14,20 +14,20 @@ class RemoveUnusedVariablesPass(IRPass): dfg: DFGAnalysis work_list: OrderedSet[IRInstruction] - msizes: dict[IRBasicBlock, int] + _msizes: dict[IRBasicBlock, int] def run_pass(self): self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) self.reachable = self.analyses_cache.request_analysis(ReachableAnalysis).reachable - self.msizes = defaultdict(list) + self._msizes = defaultdict(list) self.instruction_index = {} for bb in self.function.get_basic_blocks(): for idx, inst in enumerate(bb.instructions): inst = bb.instructions[idx] self.instruction_index[inst] = idx if inst.opcode == "msize": - self.msizes[bb].append(idx) + self._msizes[bb].append(idx) work_list = OrderedSet() self.work_list = work_list @@ -40,29 +40,39 @@ def run_pass(self): self._process_instruction(inst) for bb in self.function.get_basic_blocks(): - bb.clean_garbage() + bb.clear_dead_instructions() self.analyses_cache.invalidate_analysis(LivenessAnalysis) self.analyses_cache.invalidate_analysis(DFGAnalysis) + def has_msize(self, bb): + return len(self._msizes[bb]) > 0 + def get_last_msize(self, bb): - msizes = self.msizes[bb] + msizes = self._msizes[bb] if len(msizes) == 0: return None return max(msizes) + def msize_barrier(self, inst): + bb = inst.parent + if not self.has_msize(bb): + return False + return self.instruction_index[inst] < self.get_last_msize(bb) + def _process_instruction(self, inst): if inst.output is None: return if inst.is_volatile or inst.is_bb_terminator: return bb = inst.parent - if effects.MSIZE in inst.get_write_effects() and bb in self.msizes: + if effects.MSIZE in inst.get_write_effects(): # msize after memory touch - if self.instruction_index[inst] < self.get_last_msize(bb): - return - if any(reachable_bb in self.reachable[bb] for reachable_bb in self.msizes): + if self.msize_barrier(inst): return + for next_bb in self._msizes: + if next_bb in self.reachable[bb] and self.has_msize(next_bb): + return uses = self.dfg.get_uses(inst.output) if len(uses) > 0: @@ -75,6 +85,6 @@ def _process_instruction(self, inst): # if we remove an msize, update the index if inst.opcode == "msize": - self.msizes[bb].remove(self.instruction_index[inst]) + self._msizes[bb].remove(self.instruction_index[inst]) - inst.parent.mark_for_removal(inst) + bb.mark_for_removal(inst) From 7e8aae56341768d088fe5c1a94abf8193f1d56e4 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 9 Dec 2024 09:43:40 -0500 Subject: [PATCH 19/39] handle memory effect in loop --- vyper/utils.py | 3 +++ vyper/venom/passes/remove_unused_variables.py | 27 ++++++++++++------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/vyper/utils.py b/vyper/utils.py index d635c78383..bab6e0872a 100644 --- a/vyper/utils.py +++ b/vyper/utils.py @@ -75,6 +75,9 @@ def dropmany(self, iterable): for item in iterable: self._data.pop(item, None) + def clear(self): + self._data.clear() + def difference(self, other): ret = self.copy() ret.dropmany(other) diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index 91da40fc28..13abb9e009 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -22,6 +22,8 @@ def run_pass(self): self._msizes = defaultdict(list) self.instruction_index = {} + self._msize_gen0 = OrderedSet() + for bb in self.function.get_basic_blocks(): for idx, inst in enumerate(bb.instructions): inst = bb.instructions[idx] @@ -55,24 +57,28 @@ def get_last_msize(self, bb): return max(msizes) def msize_barrier(self, inst): + # return true if there is an msize after memory touch bb = inst.parent if not self.has_msize(bb): return False - return self.instruction_index[inst] < self.get_last_msize(bb) + if self.instruction_index[inst] < self.get_last_msize(bb): + return True + + for next_bb in self._msizes: + if next_bb in self.reachable[bb] and self.has_msize(next_bb): + return True + return False def _process_instruction(self, inst): if inst.output is None: return if inst.is_volatile or inst.is_bb_terminator: return + bb = inst.parent - if effects.MSIZE in inst.get_write_effects(): - # msize after memory touch - if self.msize_barrier(inst): - return - for next_bb in self._msizes: - if next_bb in self.reachable[bb] and self.has_msize(next_bb): - return + if effects.MSIZE in inst.get_write_effects() and self.msize_barrier(inst): + self._msize_gen0.add(inst) + return uses = self.dfg.get_uses(inst.output) if len(uses) > 0: @@ -83,8 +89,11 @@ def _process_instruction(self, inst): new_uses = self.dfg.get_uses(operand) self.work_list.addmany(new_uses) - # if we remove an msize, update the index + # if we remove an msize, update the index and revisit all visited + # memory write instructions. if inst.opcode == "msize": self._msizes[bb].remove(self.instruction_index[inst]) + self.work_list.addmany(self._msize_gen0) + self._msize_gen0.clear() bb.mark_for_removal(inst) From 3b688607346ad758036aba5cd08228542036e63f Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 9 Dec 2024 09:48:02 -0500 Subject: [PATCH 20/39] Fix type hint Co-authored-by: HodanPlodky <36966616+HodanPlodky@users.noreply.github.com> --- vyper/venom/passes/remove_unused_variables.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index 13abb9e009..d72e0b25c7 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -14,7 +14,7 @@ class RemoveUnusedVariablesPass(IRPass): dfg: DFGAnalysis work_list: OrderedSet[IRInstruction] - _msizes: dict[IRBasicBlock, int] + _msizes: dict[IRBasicBlock, list] def run_pass(self): self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) From 7196c5655e9f99f539d3f7948e7419c547e48483 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 9 Dec 2024 16:18:07 +0100 Subject: [PATCH 21/39] test for msize in branches --- .../unit/compiler/venom/test_removeunused.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index cfa2efd093..aca4561b34 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -64,6 +64,37 @@ def test_removeunused_msize_loop(): assert bb.instructions[2].opcode == "jmp" +def test_removeunused_msize_branches(): + ctx = IRContext() + fn = ctx.create_function("_global") + + bb = fn.get_basic_block() + branch1 = IRBasicBlock(IRLabel("branch1"), fn) + branch2 = IRBasicBlock(IRLabel("branch2"), fn) + end = IRBasicBlock(IRLabel("end"), fn) + + fn.append_basic_block(branch1) + fn.append_basic_block(branch2) + fn.append_basic_block(end) + + par = bb.append_instruction("param") + bb.append_instruction("mload", 10) + bb.append_instruction("jmp", par, branch1.label, branch2.label) + + msize = branch1.append_instruction("msize") + branch1.append_instruction("mstore", msize, 10) + branch1.append_instruction("jmp", end.label) + + branch2.append_instruction("jmp", end.label) + + end.append_instruction("stop") + + ac = IRAnalysesCache(fn) + RemoveUnusedVariablesPass(ac, fn).run_pass() + + assert bb.instructions[1].opcode == "mload" + + # Should this work? def test_removeunused_unused_msize_loop(): ctx = IRContext() From 4ffdf9d79a6af8f1b7f8fdb12f0165e719270bc4 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 9 Dec 2024 16:24:21 +0100 Subject: [PATCH 22/39] fix jmp => jnz --- tests/unit/compiler/venom/test_removeunused.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index aca4561b34..8ecbe1fa2b 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -79,7 +79,7 @@ def test_removeunused_msize_branches(): par = bb.append_instruction("param") bb.append_instruction("mload", 10) - bb.append_instruction("jmp", par, branch1.label, branch2.label) + bb.append_instruction("jnz", par, branch1.label, branch2.label) msize = branch1.append_instruction("msize") branch1.append_instruction("mstore", msize, 10) From b35d0eab42b56b59e44a86c851662b287374e83e Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 9 Dec 2024 10:29:18 -0500 Subject: [PATCH 23/39] fix some bad logic --- tests/unit/compiler/venom/test_removeunused.py | 2 +- vyper/venom/passes/remove_unused_variables.py | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index 8ecbe1fa2b..23fbb924fd 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -92,7 +92,7 @@ def test_removeunused_msize_branches(): ac = IRAnalysesCache(fn) RemoveUnusedVariablesPass(ac, fn).run_pass() - assert bb.instructions[1].opcode == "mload" + assert bb.instructions[1].opcode == "mload", bb # Should this work? diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index d72e0b25c7..cb3c312d4e 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -56,18 +56,18 @@ def get_last_msize(self, bb): return None return max(msizes) - def msize_barrier(self, inst): + def msize_fence(self, inst): # return true if there is an msize after memory touch bb = inst.parent - if not self.has_msize(bb): - return False - if self.instruction_index[inst] < self.get_last_msize(bb): - return True for next_bb in self._msizes: if next_bb in self.reachable[bb] and self.has_msize(next_bb): return True - return False + + if not self.has_msize(bb): + return False + + return self.instruction_index[inst] < self.get_last_msize(bb) def _process_instruction(self, inst): if inst.output is None: @@ -76,7 +76,7 @@ def _process_instruction(self, inst): return bb = inst.parent - if effects.MSIZE in inst.get_write_effects() and self.msize_barrier(inst): + if effects.MSIZE in inst.get_write_effects() and self.msize_fence(inst): self._msize_gen0.add(inst) return From 303414e5512878f8be33f52ba0977ab7a38bb3de Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 18 Dec 2024 10:18:46 -0500 Subject: [PATCH 24/39] update tests with new machinery --- .../unit/compiler/venom/test_removeunused.py | 283 ++++++++---------- 1 file changed, 127 insertions(+), 156 deletions(-) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index 23fbb924fd..d8506ec556 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -1,185 +1,156 @@ +from tests.venom_utils import assert_ctx_eq, parse_from_basic_block, parse_venom from vyper.venom.analysis.analysis import IRAnalysesCache -from vyper.venom.basicblock import IRBasicBlock, IRLabel -from vyper.venom.context import IRContext from vyper.venom.passes import RemoveUnusedVariablesPass -def test_removeunused_msize_basic(): - ctx = IRContext() - fn = ctx.create_function("_global") - - bb = fn.get_basic_block() - bb.append_instruction("mload", 32) - msize = bb.append_instruction("msize") - bb.append_instruction("mload", 64) - bb.append_instruction("return", msize) +def _check_pre_post(pre, post, scope="basicblock"): + if scope == "basicblock": + parse = parse_from_basic_block + else: + parse = parse_venom - ac = IRAnalysesCache(fn) - RemoveUnusedVariablesPass(ac, fn).run_pass() + ctx = parse(pre) + for fn in ctx.functions.values(): + ac = IRAnalysesCache(fn) + RemoveUnusedVariablesPass(ac, fn).run_pass() - assert bb.instructions[0].opcode == "mload" - assert bb.instructions[0].operands[0].value == 32 - assert bb.instructions[1].opcode == "msize" - assert bb.instructions[2].opcode == "return" + assert_ctx_eq(ctx, parse(post)) -def test_removeunused_msize_two_msizes(): - ctx = IRContext() - fn = ctx.create_function("_global") +def _check_no_change(pre, scope="basicblock"): + _check_pre_post(pre, pre, scope=scope) - bb = fn.get_basic_block() - bb.append_instruction("mload", 32) - msize1 = bb.append_instruction("msize") - bb.append_instruction("mload", 64) - msize2 = bb.append_instruction("msize") - bb.append_instruction("return", msize1, msize2) +def test_removeunused_basic(): + """ + Check basic unused variable removal + """ + pre = """ + main: + %1 = add 10 20 + %2_unused = add 10 %1 + mstore 20 %1 + stop + """ + post = """ + main: + %1 = add 10 20 + mstore 20 %1 + stop + """ + _check_pre_post(pre, post) - ac = IRAnalysesCache(fn) - RemoveUnusedVariablesPass(ac, fn).run_pass() - assert bb.instructions[0].opcode == "mload" - assert bb.instructions[0].operands[0].value == 32 - assert bb.instructions[1].opcode == "msize" - assert bb.instructions[2].opcode == "mload" - assert bb.instructions[2].operands[0].value == 64 - assert bb.instructions[3].opcode == "msize" - assert bb.instructions[4].opcode == "return" +def test_removeunused_msize_basic(): + pre = """ + main: + %a = mload 32 + %b = msize + %c = mload 64 # safe to remove + return %b, %b + """ + post = """ + main: + %a = mload 32 + %b = msize + return %b, %b + """ + _check_pre_post(pre, post) -def test_removeunused_msize_loop(): - ctx = IRContext() - fn = ctx.create_function("_global") +def test_removeunused_msize_two_msizes(): + pre = """ + main: + %a = mload 32 + %b = msize + %c = mload 64 # not safe to remove + %d = msize + return %b, %d + """ + _check_no_change(pre) - bb = fn.get_basic_block() - msize = bb.append_instruction("msize") - bb.append_instruction("mload", msize) - bb.append_instruction("jmp", bb.label) - ac = IRAnalysesCache(fn) - RemoveUnusedVariablesPass(ac, fn).run_pass() +def test_removeunused_msize_loop(): + pre = """ + main: + %msize = msize - assert bb.instructions[0].opcode == "msize" - assert bb.instructions[1].opcode == "mload" - assert bb.instructions[1].operands[0] == msize - assert bb.instructions[2].opcode == "jmp" + # not safe to remove because the previous instruction is + # still reachable + %1 = mload %msize + jmp @main + """ + _check_no_change(pre) def test_removeunused_msize_branches(): - ctx = IRContext() - fn = ctx.create_function("_global") - - bb = fn.get_basic_block() - branch1 = IRBasicBlock(IRLabel("branch1"), fn) - branch2 = IRBasicBlock(IRLabel("branch2"), fn) - end = IRBasicBlock(IRLabel("end"), fn) - - fn.append_basic_block(branch1) - fn.append_basic_block(branch2) - fn.append_basic_block(end) - - par = bb.append_instruction("param") - bb.append_instruction("mload", 10) - bb.append_instruction("jnz", par, branch1.label, branch2.label) - - msize = branch1.append_instruction("msize") - branch1.append_instruction("mstore", msize, 10) - branch1.append_instruction("jmp", end.label) - - branch2.append_instruction("jmp", end.label) - - end.append_instruction("stop") + pre = """ + function global { + main: + %1 = param + %2 = mload 10 ; looks unused, but has MSIZE effect + jnz %1, @branch1, @branch2 + branch1: + %3 = msize ; blocks removal of `%2 = mload 10` + mstore 10, %3 + jmp @end + branch2: + jmp @end + end: + stop + } + """ + _check_no_change(pre, scope="function") - ac = IRAnalysesCache(fn) - RemoveUnusedVariablesPass(ac, fn).run_pass() - assert bb.instructions[1].opcode == "mload", bb - - -# Should this work? def test_removeunused_unused_msize_loop(): - ctx = IRContext() - fn = ctx.create_function("_global") - - bb = fn.get_basic_block() - bb.append_instruction("msize") - bb.append_instruction("mload", 10) - bb.append_instruction("jmp", bb.label) - - ac = IRAnalysesCache(fn) - RemoveUnusedVariablesPass(ac, fn).run_pass() + pre = """ + main: + %1_unused = msize + %2_unused = mload 10 + jmp @main + """ + post = """ + main: + jmp @main + """ + _check_pre_post(pre, post) - assert bb.instructions[0].opcode == "jmp" - -# Should this work? def test_removeunused_unused_msize(): - ctx = IRContext() - fn = ctx.create_function("_global") - - bb = fn.get_basic_block() - bb.append_instruction("mload", 10) - bb.append_instruction("msize") - bb.append_instruction("stop") - - ac = IRAnalysesCache(fn) - RemoveUnusedVariablesPass(ac, fn).run_pass() - - assert bb.instructions[0].opcode == "stop", bb - - -def test_removeunused_basic(): - ctx = IRContext() - fn = ctx.create_function("_global") - - bb = fn.get_basic_block() - var1 = bb.append_instruction("add", 10, 20) - bb.append_instruction("add", var1, 10) - bb.append_instruction("mstore", var1, 20) - bb.append_instruction("stop") - - ac = IRAnalysesCache(fn) - RemoveUnusedVariablesPass(ac, fn).run_pass() + pre = """ + main: + %1_unused = mload 10 + %2_unused = msize + stop + """ + post = """ + main: + stop + """ + _check_pre_post(pre, post) - assert bb.instructions[0].opcode == "add" - assert bb.instructions[0].operands[0].value == 10 - assert bb.instructions[0].operands[1].value == 20 - assert bb.instructions[1].opcode == "mstore" - assert bb.instructions[1].operands[0] == var1 - assert bb.instructions[1].operands[1].value == 20 - assert bb.instructions[2].opcode == "stop" def test_removeunused_loop(): - ctx = IRContext() - fn = ctx.create_function("_global") - - bb = fn.get_basic_block() - after_bb = IRBasicBlock(IRLabel("after"), fn) - fn.append_basic_block(after_bb) - - var1 = bb.append_instruction("store", 10) - bb.append_instruction("jmp", after_bb.label) - - var2 = fn.get_next_variable() - var_phi = after_bb.append_instruction("phi", bb.label, var1, after_bb.label, var2) - after_bb.append_instruction("add", var_phi, 1, ret=var2) - after_bb.append_instruction("add", var2, var_phi) - after_bb.append_instruction("mstore", var2, 10) - after_bb.append_instruction("jmp", after_bb.label) - - ac = IRAnalysesCache(fn) - RemoveUnusedVariablesPass(ac, fn).run_pass() - - assert bb.instructions[0].opcode == "store" - assert bb.instructions[0].operands[0].value == 10 - assert bb.instructions[1].opcode == "jmp" - - assert after_bb.instructions[0].opcode == "phi" - assert after_bb.instructions[1].opcode == "add" - assert after_bb.instructions[1].operands[0] == var_phi - assert after_bb.instructions[1].operands[1].value == 1 - assert after_bb.instructions[2].opcode == "mstore" - assert after_bb.instructions[2].operands[0] == var2 - assert after_bb.instructions[2].operands[1].value == 10 - assert after_bb.instructions[3].opcode == "jmp" - assert after_bb.instructions[3].operands[0] == after_bb.label + pre = """ + main: + %1 = 10 + jmp @after + after: + %p = phi @main, %1, @after, %2 + %2 = add %p, 1 + %3_unused add %2, %p + mstore 10, %2 + jmp @after + """ + post = """ + main: + %1 = 10 + jmp @after + after: + %p = phi @main, %1, @after, %2 + %2 = add %p, 1 + mstore 10, %2 + jmp @after + """ + _check_pre_post(pre, post) From 04c31d648d5be2ab913336c0e35334068a1a795b Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 18 Dec 2024 11:28:32 -0500 Subject: [PATCH 25/39] fix typos --- tests/unit/compiler/venom/test_removeunused.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index d8506ec556..d7aa481da3 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -26,15 +26,15 @@ def test_removeunused_basic(): """ pre = """ main: - %1 = add 10 20 - %2_unused = add 10 %1 - mstore 20 %1 + %1 = add 10, 20 + %2_unused = add 10, %1 + mstore 20, %1 stop """ post = """ main: - %1 = add 10 20 - mstore 20 %1 + %1 = add 10, 20 + mstore 20, %1 stop """ _check_pre_post(pre, post) @@ -139,7 +139,7 @@ def test_removeunused_loop(): after: %p = phi @main, %1, @after, %2 %2 = add %p, 1 - %3_unused add %2, %p + %3_unused = add %2, %p mstore 10, %2 jmp @after """ From d10df16894db71ab7660016c76f87813dfdcd3ca Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 18 Dec 2024 11:28:42 -0500 Subject: [PATCH 26/39] fix lint --- tests/unit/compiler/venom/test_removeunused.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index d7aa481da3..00a7620e47 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -20,6 +20,7 @@ def _check_pre_post(pre, post, scope="basicblock"): def _check_no_change(pre, scope="basicblock"): _check_pre_post(pre, pre, scope=scope) + def test_removeunused_basic(): """ Check basic unused variable removal @@ -130,7 +131,6 @@ def test_removeunused_unused_msize(): _check_pre_post(pre, post) - def test_removeunused_loop(): pre = """ main: From 08bec153e38c09e534d83ee5ed5620b0ef7aafc4 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 18 Dec 2024 11:31:15 -0500 Subject: [PATCH 27/39] update comments --- tests/unit/compiler/venom/test_removeunused.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index 00a7620e47..707bf247ac 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -46,7 +46,7 @@ def test_removeunused_msize_basic(): main: %a = mload 32 %b = msize - %c = mload 64 # safe to remove + %c_unused = mload 64 # safe to remove return %b, %b """ post = """ @@ -63,7 +63,7 @@ def test_removeunused_msize_two_msizes(): main: %a = mload 32 %b = msize - %c = mload 64 # not safe to remove + %c = mload 64 # not safe to remove - has MSIZE effect %d = msize return %b, %d """ From 578abe4fce6eb18aabe834dc2999e6c81f2634e9 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 18 Dec 2024 11:45:19 -0500 Subject: [PATCH 28/39] rename a variable and add more comments --- vyper/venom/passes/remove_unused_variables.py | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index cb3c312d4e..9a4995b858 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -21,13 +21,18 @@ def run_pass(self): self.reachable = self.analyses_cache.request_analysis(ReachableAnalysis).reachable self._msizes = defaultdict(list) - self.instruction_index = {} - self._msize_gen0 = OrderedSet() + self._blocked_by_msize: OrderedSet[IRInstruction] = OrderedSet() + + # map instructions to their indexes in the basic block. + # although the basic block can be updated during this pass, + # instruction_ordering only needs to be able to give us a total + # ordering of effects. + self.instruction_ordering: dict[IRInstruction, int] = {} for bb in self.function.get_basic_blocks(): for idx, inst in enumerate(bb.instructions): inst = bb.instructions[idx] - self.instruction_index[inst] = idx + self.instruction_ordering[inst] = idx if inst.opcode == "msize": self._msizes[bb].append(idx) @@ -67,7 +72,7 @@ def msize_fence(self, inst): if not self.has_msize(bb): return False - return self.instruction_index[inst] < self.get_last_msize(bb) + return self.instruction_ordering[inst] < self.get_last_msize(bb) def _process_instruction(self, inst): if inst.output is None: @@ -77,7 +82,7 @@ def _process_instruction(self, inst): bb = inst.parent if effects.MSIZE in inst.get_write_effects() and self.msize_fence(inst): - self._msize_gen0.add(inst) + self._blocked_by_msize.add(inst) return uses = self.dfg.get_uses(inst.output) @@ -90,10 +95,10 @@ def _process_instruction(self, inst): self.work_list.addmany(new_uses) # if we remove an msize, update the index and revisit all visited - # memory write instructions. + # memory instructions since they might now be free to be removed. if inst.opcode == "msize": - self._msizes[bb].remove(self.instruction_index[inst]) - self.work_list.addmany(self._msize_gen0) - self._msize_gen0.clear() + self._msizes[bb].remove(self.instruction_ordering[inst]) + self.work_list.addmany(self._blocked_by_msize) + self._blocked_by_msize.clear() bb.mark_for_removal(inst) From ebbde05ce07b869130755b0248b7610315b7f8d2 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 18 Dec 2024 11:46:44 -0500 Subject: [PATCH 29/39] add another test --- .../unit/compiler/venom/test_removeunused.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index 707bf247ac..88c8aabac5 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -40,6 +40,27 @@ def test_removeunused_basic(): """ _check_pre_post(pre, post) +def test_removeunused_chain(): + """ + Check removal of unused variable dependency chain + """ + pre = """ + main: + %1 = add 10, 20 + %2_unused = add 10, %1 + %3_unused = add 10, %2_unused + mstore 20, %1 + stop + """ + post = """ + main: + %1 = add 10, 20 + mstore 20, %1 + stop + """ + _check_pre_post(pre, post) + + def test_removeunused_msize_basic(): pre = """ From 0171aab99abf534845ea49e5ea7f8fffbc3d4e99 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 18 Dec 2024 11:51:47 -0500 Subject: [PATCH 30/39] move around tests, add test comments --- .../unit/compiler/venom/test_removeunused.py | 83 +++++++++++-------- 1 file changed, 49 insertions(+), 34 deletions(-) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index 88c8aabac5..56fab3bbc4 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -61,8 +61,35 @@ def test_removeunused_chain(): _check_pre_post(pre, post) +def test_removeunused_loop(): + """ + Test unused variable removal in loop + """ + pre = """ + main: + %1 = 10 + jmp @after + after: + %p = phi @main, %1, @after, %2 + %2 = add %p, 1 + %3_unused = add %2, %p + mstore 10, %2 + jmp @after + """ + post = """ + main: + %1 = 10 + jmp @after + after: + %p = phi @main, %1, @after, %2 + %2 = add %p, 1 + mstore 10, %2 + jmp @after + """ + _check_pre_post(pre, post) + -def test_removeunused_msize_basic(): +def test_removeunused_mload_basic(): pre = """ main: %a = mload 32 @@ -79,7 +106,7 @@ def test_removeunused_msize_basic(): _check_pre_post(pre, post) -def test_removeunused_msize_two_msizes(): +def test_removeunused_mload_two_msizes(): pre = """ main: %a = mload 32 @@ -94,17 +121,22 @@ def test_removeunused_msize_two_msizes(): def test_removeunused_msize_loop(): pre = """ main: - %msize = msize + %1 = msize # not safe to remove because the previous instruction is # still reachable - %1 = mload %msize + %2 = mload %1 + jmp @main """ _check_no_change(pre) def test_removeunused_msize_branches(): + """ + Test that mload removal is blocked by msize in a downstream basic + block + """ pre = """ function global { main: @@ -124,21 +156,11 @@ def test_removeunused_msize_branches(): _check_no_change(pre, scope="function") -def test_removeunused_unused_msize_loop(): - pre = """ - main: - %1_unused = msize - %2_unused = mload 10 - jmp @main +def test_remove_unused_mload_msize_chain(): """ - post = """ - main: - jmp @main + Test effect chain removal - remove mload which is initially blocked by + an msize but is free to be removed after the msize is removed. """ - _check_pre_post(pre, post) - - -def test_removeunused_unused_msize(): pre = """ main: %1_unused = mload 10 @@ -151,27 +173,20 @@ def test_removeunused_unused_msize(): """ _check_pre_post(pre, post) - -def test_removeunused_loop(): +def test_remove_unused_mload_msize_chain_loop(): + """ + Test effect chain removal - remove mload which is initially blocked by + an msize but is free to be removed after the msize is removed. + Loop version. + """ pre = """ main: - %1 = 10 - jmp @after - after: - %p = phi @main, %1, @after, %2 - %2 = add %p, 1 - %3_unused = add %2, %p - mstore 10, %2 - jmp @after + %1_unused = msize + %2_unused = mload 10 + jmp @main """ post = """ main: - %1 = 10 - jmp @after - after: - %p = phi @main, %1, @after, %2 - %2 = add %p, 1 - mstore 10, %2 - jmp @after + jmp @main """ _check_pre_post(pre, post) From ae59fee73b97d96957090b0bc0ff864a7523ecbe Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 20 Dec 2024 12:22:31 -0500 Subject: [PATCH 31/39] fix lint --- tests/unit/compiler/venom/test_removeunused.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index 56fab3bbc4..3564841e07 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -40,6 +40,7 @@ def test_removeunused_basic(): """ _check_pre_post(pre, post) + def test_removeunused_chain(): """ Check removal of unused variable dependency chain @@ -173,6 +174,7 @@ def test_remove_unused_mload_msize_chain(): """ _check_pre_post(pre, post) + def test_remove_unused_mload_msize_chain_loop(): """ Test effect chain removal - remove mload which is initially blocked by From 0223869a7721a1f94a4adeece7c8ddac4b17cd32 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 20 Dec 2024 12:30:34 -0500 Subject: [PATCH 32/39] add another test --- tests/unit/compiler/venom/test_removeunused.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index 3564841e07..3248cdf6d8 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -133,6 +133,23 @@ def test_removeunused_msize_loop(): _check_no_change(pre) +def test_removeunused_msize_reachable(): + pre = """ + main: + # not safe to remove because there is an msize in a reachable + # basic block + %1 = mload 0 + + jmp @next + next: + jmp @last + last: + %2 = msize + return %2, %2 + """ + _check_no_change(pre) + + def test_removeunused_msize_branches(): """ Test that mload removal is blocked by msize in a downstream basic From d58011643f1100d61b75f3231c31385f40ca7bd6 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 21 Dec 2024 21:01:19 -0500 Subject: [PATCH 33/39] simplify remove_unused_variables, add itouch instruction --- vyper/codegen/module.py | 2 +- vyper/evm/opcodes.py | 1 + vyper/ir/compile_ir.py | 11 ++++ vyper/venom/basicblock.py | 1 + vyper/venom/effects.py | 1 + vyper/venom/passes/remove_unused_variables.py | 50 ------------------- vyper/venom/venom_to_assembly.py | 6 +++ 7 files changed, 21 insertions(+), 51 deletions(-) diff --git a/vyper/codegen/module.py b/vyper/codegen/module.py index 1844569138..4c2db4d2e4 100644 --- a/vyper/codegen/module.py +++ b/vyper/codegen/module.py @@ -500,7 +500,7 @@ def generate_ir_for_module(module_t: ModuleT) -> tuple[IRnode, IRnode]: # assumption in general: (mload X) => msize == ceil32(X + 32) # see py-evm extend_memory: after_size = ceil32(start_position + size) if immutables_len > 0: - deploy_code.append(["iload", max(0, immutables_len - 32)]) + deploy_code.append(["itouch", max(0, immutables_len - 32)]) deploy_code.append(init_func_ir) deploy_code.append(["deploy", init_mem_used, runtime, immutables_len]) diff --git a/vyper/evm/opcodes.py b/vyper/evm/opcodes.py index 3049d7f911..2acd439096 100644 --- a/vyper/evm/opcodes.py +++ b/vyper/evm/opcodes.py @@ -201,6 +201,7 @@ "DEBUGGER": (None, 0, 0, 0), "ILOAD": (None, 1, 1, 6), "ISTORE": (None, 2, 0, 6), + "ITOUCH": (None, 1, 0, 6), "DLOAD": (None, 1, 1, 9), "DLOADBYTES": (None, 3, 0, 3), } diff --git a/vyper/ir/compile_ir.py b/vyper/ir/compile_ir.py index e87cf1b310..5deae57576 100644 --- a/vyper/ir/compile_ir.py +++ b/vyper/ir/compile_ir.py @@ -333,6 +333,17 @@ def _height_of(witharg): return o + elif code.value == "itouch": + loc = code.args[0] + + o = [] + o.extend(_data_ofst_of("_mem_deploy_end", loc, height)) + o.append("MLOAD") + o.append("POP") + + return o + + # "mstore" to the data section of (to-be-deployed) runtime code elif code.value == "istore": loc = code.args[0] diff --git a/vyper/venom/basicblock.py b/vyper/venom/basicblock.py index 37c4166a97..9a4e880fed 100644 --- a/vyper/venom/basicblock.py +++ b/vyper/venom/basicblock.py @@ -20,6 +20,7 @@ "invoke", "sstore", "istore", + "itouch", "tstore", "mstore", "calldatacopy", diff --git a/vyper/venom/effects.py b/vyper/venom/effects.py index 3084b4ddb0..4aa6e72151 100644 --- a/vyper/venom/effects.py +++ b/vyper/venom/effects.py @@ -36,6 +36,7 @@ def __iter__(self): "tstore": TRANSIENT, "mstore": MEMORY, "istore": IMMUTABLES, + "itouch": MSIZE, "call": ALL ^ IMMUTABLES, "delegatecall": ALL ^ IMMUTABLES, "staticcall": MEMORY | RETURNDATA, diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index 9a4995b858..dfa7807fc6 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -14,27 +14,9 @@ class RemoveUnusedVariablesPass(IRPass): dfg: DFGAnalysis work_list: OrderedSet[IRInstruction] - _msizes: dict[IRBasicBlock, list] def run_pass(self): self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) - self.reachable = self.analyses_cache.request_analysis(ReachableAnalysis).reachable - - self._msizes = defaultdict(list) - self._blocked_by_msize: OrderedSet[IRInstruction] = OrderedSet() - - # map instructions to their indexes in the basic block. - # although the basic block can be updated during this pass, - # instruction_ordering only needs to be able to give us a total - # ordering of effects. - self.instruction_ordering: dict[IRInstruction, int] = {} - - for bb in self.function.get_basic_blocks(): - for idx, inst in enumerate(bb.instructions): - inst = bb.instructions[idx] - self.instruction_ordering[inst] = idx - if inst.opcode == "msize": - self._msizes[bb].append(idx) work_list = OrderedSet() self.work_list = work_list @@ -52,28 +34,6 @@ def run_pass(self): self.analyses_cache.invalidate_analysis(LivenessAnalysis) self.analyses_cache.invalidate_analysis(DFGAnalysis) - def has_msize(self, bb): - return len(self._msizes[bb]) > 0 - - def get_last_msize(self, bb): - msizes = self._msizes[bb] - if len(msizes) == 0: - return None - return max(msizes) - - def msize_fence(self, inst): - # return true if there is an msize after memory touch - bb = inst.parent - - for next_bb in self._msizes: - if next_bb in self.reachable[bb] and self.has_msize(next_bb): - return True - - if not self.has_msize(bb): - return False - - return self.instruction_ordering[inst] < self.get_last_msize(bb) - def _process_instruction(self, inst): if inst.output is None: return @@ -81,9 +41,6 @@ def _process_instruction(self, inst): return bb = inst.parent - if effects.MSIZE in inst.get_write_effects() and self.msize_fence(inst): - self._blocked_by_msize.add(inst) - return uses = self.dfg.get_uses(inst.output) if len(uses) > 0: @@ -94,11 +51,4 @@ def _process_instruction(self, inst): new_uses = self.dfg.get_uses(operand) self.work_list.addmany(new_uses) - # if we remove an msize, update the index and revisit all visited - # memory instructions since they might now be free to be removed. - if inst.opcode == "msize": - self._msizes[bb].remove(self.instruction_ordering[inst]) - self.work_list.addmany(self._blocked_by_msize) - self._blocked_by_msize.clear() - bb.mark_for_removal(inst) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 048555a221..15a993777b 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -546,6 +546,12 @@ def _generate_evm_for_instruction( elif opcode == "assert_unreachable": end_symbol = mksymbol("reachable") assembly.extend([end_symbol, "JUMPI", "INVALID", end_symbol, "JUMPDEST"]) + elif opcode == "itouch": + addr = inst.operands[0] + assert isinstance(addr, IRLiteral) + assembly.extend(["_OFST", "_mem_deploy_end", addr.value]) + assembly.append("MLOAD") + assembly.append("POP") elif opcode == "iload": addr = inst.operands[0] if isinstance(addr, IRLiteral): From e487909c0342b6fdb6bd03aedbc0810102c0615d Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 30 Dec 2024 18:46:19 -0500 Subject: [PATCH 34/39] fix lint --- vyper/ir/compile_ir.py | 1 - vyper/venom/passes/remove_unused_variables.py | 6 ++---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/vyper/ir/compile_ir.py b/vyper/ir/compile_ir.py index 5deae57576..097a8563f5 100644 --- a/vyper/ir/compile_ir.py +++ b/vyper/ir/compile_ir.py @@ -343,7 +343,6 @@ def _height_of(witharg): return o - # "mstore" to the data section of (to-be-deployed) runtime code elif code.value == "istore": loc = code.args[0] diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index dfa7807fc6..47976810d9 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -1,9 +1,7 @@ -from collections import defaultdict from vyper.utils import OrderedSet -from vyper.venom import effects -from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis, ReachableAnalysis -from vyper.venom.basicblock import IRBasicBlock, IRInstruction +from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis +from vyper.venom.basicblock import IRInstruction from vyper.venom.passes.base_pass import IRPass From 7544544f0c980c7c61a0581836cbc5b3e56a6f52 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 30 Dec 2024 18:46:26 -0500 Subject: [PATCH 35/39] rename a variable --- vyper/venom/passes/remove_unused_variables.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index 47976810d9..eb0df03dd8 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -19,8 +19,8 @@ def run_pass(self): work_list = OrderedSet() self.work_list = work_list - uses = self.dfg.outputs.values() - work_list.addmany(uses) + instructions = self.dfg.outputs.values() + work_list.addmany(instructions) while len(work_list) > 0: inst = work_list.pop() From 0ad9a335338a6316e92e482e0ce99ed34a1c6272 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 30 Dec 2024 18:55:32 -0500 Subject: [PATCH 36/39] update tests - don't need as many tricky mload/msize test cases --- .../unit/compiler/venom/test_removeunused.py | 88 +++++-------------- vyper/venom/passes/remove_unused_variables.py | 1 - 2 files changed, 21 insertions(+), 68 deletions(-) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index 3248cdf6d8..41176036a8 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -1,24 +1,19 @@ -from tests.venom_utils import assert_ctx_eq, parse_from_basic_block, parse_venom +from tests.venom_utils import assert_ctx_eq, parse_from_basic_block from vyper.venom.analysis.analysis import IRAnalysesCache from vyper.venom.passes import RemoveUnusedVariablesPass -def _check_pre_post(pre, post, scope="basicblock"): - if scope == "basicblock": - parse = parse_from_basic_block - else: - parse = parse_venom - - ctx = parse(pre) +def _check_pre_post(pre, post): + ctx = parse_from_basic_block(pre) for fn in ctx.functions.values(): ac = IRAnalysesCache(fn) RemoveUnusedVariablesPass(ac, fn).run_pass() - assert_ctx_eq(ctx, parse(post)) + assert_ctx_eq(ctx, parse_from_basic_block(post)) -def _check_no_change(pre, scope="basicblock"): - _check_pre_post(pre, pre, scope=scope) +def _check_no_change(pre): + _check_pre_post(pre, pre) def test_removeunused_basic(): @@ -93,14 +88,14 @@ def test_removeunused_loop(): def test_removeunused_mload_basic(): pre = """ main: - %a = mload 32 + %a = itouch 32 %b = msize %c_unused = mload 64 # safe to remove return %b, %b """ post = """ main: - %a = mload 32 + %a = itouch 32 %b = msize return %b, %b """ @@ -112,72 +107,32 @@ def test_removeunused_mload_two_msizes(): main: %a = mload 32 %b = msize - %c = mload 64 # not safe to remove - has MSIZE effect + %c = mload 64 %d = msize return %b, %d """ - _check_no_change(pre) + post = """ + main: + %b = msize + %d = msize + return %b, %d + """ + _check_pre_post(pre, post) def test_removeunused_msize_loop(): pre = """ main: %1 = msize - - # not safe to remove because the previous instruction is - # still reachable - %2 = mload %1 - + itouch %1 # volatile jmp @main """ _check_no_change(pre) -def test_removeunused_msize_reachable(): - pre = """ - main: - # not safe to remove because there is an msize in a reachable - # basic block - %1 = mload 0 - - jmp @next - next: - jmp @last - last: - %2 = msize - return %2, %2 - """ - _check_no_change(pre) - - -def test_removeunused_msize_branches(): - """ - Test that mload removal is blocked by msize in a downstream basic - block - """ - pre = """ - function global { - main: - %1 = param - %2 = mload 10 ; looks unused, but has MSIZE effect - jnz %1, @branch1, @branch2 - branch1: - %3 = msize ; blocks removal of `%2 = mload 10` - mstore 10, %3 - jmp @end - branch2: - jmp @end - end: - stop - } - """ - _check_no_change(pre, scope="function") - - -def test_remove_unused_mload_msize_chain(): +def test_remove_unused_mload_msize(): """ - Test effect chain removal - remove mload which is initially blocked by - an msize but is free to be removed after the msize is removed. + Test removal of non-volatile mload and msize instructions """ pre = """ main: @@ -194,9 +149,8 @@ def test_remove_unused_mload_msize_chain(): def test_remove_unused_mload_msize_chain_loop(): """ - Test effect chain removal - remove mload which is initially blocked by - an msize but is free to be removed after the msize is removed. - Loop version. + Test removal of non-volatile mload and msize instructions. + Loop version """ pre = """ main: diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index eb0df03dd8..a1c3fa0e9c 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -1,4 +1,3 @@ - from vyper.utils import OrderedSet from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis from vyper.venom.basicblock import IRInstruction From 8944ba16b60aa4c19403badcf207020448cd808a Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 30 Dec 2024 19:24:32 -0500 Subject: [PATCH 37/39] add itouch to PASS_THRU_INSTRUCTIONS simplify some code. remove a dead comment --- tests/unit/compiler/venom/test_removeunused.py | 4 ++-- vyper/codegen/module.py | 2 +- vyper/venom/basicblock.py | 1 + vyper/venom/ir_node_to_venom.py | 3 +-- vyper/venom/venom_to_assembly.py | 6 ++++-- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py index 41176036a8..d4c3744eaa 100644 --- a/tests/unit/compiler/venom/test_removeunused.py +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -88,14 +88,14 @@ def test_removeunused_loop(): def test_removeunused_mload_basic(): pre = """ main: - %a = itouch 32 + itouch 32 %b = msize %c_unused = mload 64 # safe to remove return %b, %b """ post = """ main: - %a = itouch 32 + itouch 32 %b = msize return %b, %b """ diff --git a/vyper/codegen/module.py b/vyper/codegen/module.py index 4c2db4d2e4..f76292fda3 100644 --- a/vyper/codegen/module.py +++ b/vyper/codegen/module.py @@ -500,7 +500,7 @@ def generate_ir_for_module(module_t: ModuleT) -> tuple[IRnode, IRnode]: # assumption in general: (mload X) => msize == ceil32(X + 32) # see py-evm extend_memory: after_size = ceil32(start_position + size) if immutables_len > 0: - deploy_code.append(["itouch", max(0, immutables_len - 32)]) + deploy_code.append(["itouch", max(0, immutables_len )]) deploy_code.append(init_func_ir) deploy_code.append(["deploy", init_mem_used, runtime, immutables_len]) diff --git a/vyper/venom/basicblock.py b/vyper/venom/basicblock.py index 34fa030041..799fa88263 100644 --- a/vyper/venom/basicblock.py +++ b/vyper/venom/basicblock.py @@ -52,6 +52,7 @@ "sstore", "istore", "tstore", + "itouch", "dloadbytes", "calldatacopy", "mcopy", diff --git a/vyper/venom/ir_node_to_venom.py b/vyper/venom/ir_node_to_venom.py index f46457b77f..677e0d6eec 100644 --- a/vyper/venom/ir_node_to_venom.py +++ b/vyper/venom/ir_node_to_venom.py @@ -66,6 +66,7 @@ "mload", "iload", "istore", + "itouch", "dload", "dloadbytes", "sload", @@ -234,8 +235,6 @@ def _convert_ir_bb(fn, ir, symbols): # TODO: refactor these to not be globals global _break_target, _continue_target, _alloca_table - # keep a map from external functions to all possible entry points - ctx = fn.ctx fn.push_source(ir) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 15a993777b..36518346c5 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -548,8 +548,10 @@ def _generate_evm_for_instruction( assembly.extend([end_symbol, "JUMPI", "INVALID", end_symbol, "JUMPDEST"]) elif opcode == "itouch": addr = inst.operands[0] - assert isinstance(addr, IRLiteral) - assembly.extend(["_OFST", "_mem_deploy_end", addr.value]) + if isinstance(addr, IRLiteral): + assembly.extend(["_OFST", "_mem_deploy_end", addr.value]) + else: + assembly.extend(["_mem_deploy_end", "ADD"]) assembly.append("MLOAD") assembly.append("POP") elif opcode == "iload": From d1151a8e8764370f53e577746894011750db62b0 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 6 Jan 2025 10:03:17 -0500 Subject: [PATCH 38/39] fix lint --- vyper/codegen/module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/codegen/module.py b/vyper/codegen/module.py index f76292fda3..0dccacf4e5 100644 --- a/vyper/codegen/module.py +++ b/vyper/codegen/module.py @@ -500,7 +500,7 @@ def generate_ir_for_module(module_t: ModuleT) -> tuple[IRnode, IRnode]: # assumption in general: (mload X) => msize == ceil32(X + 32) # see py-evm extend_memory: after_size = ceil32(start_position + size) if immutables_len > 0: - deploy_code.append(["itouch", max(0, immutables_len )]) + deploy_code.append(["itouch", max(0, immutables_len)]) deploy_code.append(init_func_ir) deploy_code.append(["deploy", init_mem_used, runtime, immutables_len]) From 8d93190b5dcc51520ef7acdae08369be4af58917 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sun, 12 Jan 2025 10:54:30 -0500 Subject: [PATCH 39/39] roll back change --- vyper/codegen/module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/codegen/module.py b/vyper/codegen/module.py index 0dccacf4e5..4c2db4d2e4 100644 --- a/vyper/codegen/module.py +++ b/vyper/codegen/module.py @@ -500,7 +500,7 @@ def generate_ir_for_module(module_t: ModuleT) -> tuple[IRnode, IRnode]: # assumption in general: (mload X) => msize == ceil32(X + 32) # see py-evm extend_memory: after_size = ceil32(start_position + size) if immutables_len > 0: - deploy_code.append(["itouch", max(0, immutables_len)]) + deploy_code.append(["itouch", max(0, immutables_len - 32)]) deploy_code.append(init_func_ir) deploy_code.append(["deploy", init_mem_used, runtime, immutables_len])