diff --git a/tests/unit/compiler/venom/test_removeunused.py b/tests/unit/compiler/venom/test_removeunused.py new file mode 100644 index 0000000000..d4c3744eaa --- /dev/null +++ b/tests/unit/compiler/venom/test_removeunused.py @@ -0,0 +1,165 @@ +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): + 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_from_basic_block(post)) + + +def _check_no_change(pre): + _check_pre_post(pre, pre) + + +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) + + +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_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_mload_basic(): + pre = """ + main: + itouch 32 + %b = msize + %c_unused = mload 64 # safe to remove + return %b, %b + """ + post = """ + main: + itouch 32 + %b = msize + return %b, %b + """ + _check_pre_post(pre, post) + + +def test_removeunused_mload_two_msizes(): + pre = """ + main: + %a = mload 32 + %b = msize + %c = mload 64 + %d = msize + return %b, %d + """ + post = """ + main: + %b = msize + %d = msize + return %b, %d + """ + _check_pre_post(pre, post) + + +def test_removeunused_msize_loop(): + pre = """ + main: + %1 = msize + itouch %1 # volatile + jmp @main + """ + _check_no_change(pre) + + +def test_remove_unused_mload_msize(): + """ + Test removal of non-volatile mload and msize instructions + """ + pre = """ + main: + %1_unused = mload 10 + %2_unused = msize + stop + """ + post = """ + main: + stop + """ + _check_pre_post(pre, post) + + +def test_remove_unused_mload_msize_chain_loop(): + """ + Test removal of non-volatile mload and msize instructions. + Loop version + """ + pre = """ + main: + %1_unused = msize + %2_unused = mload 10 + jmp @main + """ + post = """ + main: + jmp @main + """ + _check_pre_post(pre, post) diff --git a/vyper/codegen/module.py b/vyper/codegen/module.py index 1844569138..8297530e49 100644 --- a/vyper/codegen/module.py +++ b/vyper/codegen/module.py @@ -490,7 +490,7 @@ def generate_ir_for_module(module_t: ModuleT) -> tuple[IRnode, IRnode]: # allocation do not clobber uninitialized immutables. # cf. GH issue 3101. # note mload/iload X touches bytes from X to X+32, and msize rounds up - # to the nearest 32, so `iload`ing `immutables_len - 32` guarantees + # to the nearest 32, so `itouch`ing `immutables_len - 32` guarantees # that `msize` will refer to a memory location of at least # ` + immutables_len` (where == # `_mem_deploy_end` as defined in the assembler). @@ -499,8 +499,10 @@ def generate_ir_for_module(module_t: ModuleT) -> tuple[IRnode, IRnode]: # mload 33 => msize == 96 # assumption in general: (mload X) => msize == ceil32(X + 32) # see py-evm extend_memory: after_size = ceil32(start_position + size) + # TODO: perhaps itouch should touch the desired end position + # internally, rather than doing the +32 calculation in the frontend. 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..10ad72d73d 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, 9), "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 936e6d5d72..3dbc1c270f 100644 --- a/vyper/ir/compile_ir.py +++ b/vyper/ir/compile_ir.py @@ -338,6 +338,16 @@ 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/utils.py b/vyper/utils.py index 39d3093478..a79cc6be60 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/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 2f90410cd5..eb1ab7e0f5 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..9607e3aae9 --- /dev/null +++ b/vyper/venom/analysis/reachable.py @@ -0,0 +1,42 @@ +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 + + +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 = defaultdict(OrderedSet) + + self._compute_reachable_r(self.function.entry) + + def _compute_reachable_r(self, bb): + if bb in self.reachable: + return + + s = bb.cfg_out.copy() + self.reachable[bb] = s + + for out_bb in bb.cfg_out: + self._compute_reachable_r(out_bb) + s.update(self.reachable[out_bb]) + + 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/basicblock.py b/vyper/venom/basicblock.py index 8d86da73e7..aba1d0596f 100644 --- a/vyper/venom/basicblock.py +++ b/vyper/venom/basicblock.py @@ -19,14 +19,11 @@ "create", "create2", "invoke", - "sload", "sstore", - "iload", "istore", - "tload", + "itouch", "tstore", "mstore", - "mload", "calldatacopy", "mcopy", "extcodecopy", @@ -56,6 +53,7 @@ "sstore", "istore", "tstore", + "itouch", "dloadbytes", "calldatacopy", "mcopy", diff --git a/vyper/venom/effects.py b/vyper/venom/effects.py index bbda481e14..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, @@ -84,11 +85,11 @@ def __iter__(self): writes = _writes.copy() for k, v in reads.items(): - if MEMORY in v: + 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: + if MEMORY in v or IMMUTABLES in v: writes[k] |= MSIZE 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/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index 73fe2112d7..a3d019324a 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -18,13 +18,16 @@ 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() self._process_instruction(inst) + for bb in self.function.get_basic_blocks(): + bb.clear_dead_instructions() + self.analyses_cache.invalidate_analysis(LivenessAnalysis) self.analyses_cache.invalidate_analysis(DFGAnalysis) @@ -33,6 +36,9 @@ def _process_instruction(self, inst): return if inst.is_volatile or inst.is_bb_terminator: return + + bb = inst.parent + uses = self.dfg.get_uses(inst.output) if len(uses) > 0: return @@ -42,4 +48,4 @@ def _process_instruction(self, inst): new_uses = self.dfg.get_uses(operand) self.work_list.addmany(new_uses) - inst.parent.remove_instruction(inst) + bb.mark_for_removal(inst) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 59734773af..ba8f88413e 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -546,6 +546,14 @@ 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] + 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": addr = inst.operands[0] if isinstance(addr, IRLiteral):