feat[venom]: mark loads as non-volatile

tests/unit/compiler/venom/test_removeunused.py

@@ -0,0 +1,211 @@
+from tests.venom_utils import assert_ctx_eq, parse_from_basic_block, parse_venom
+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)
+    for fn in ctx.functions.values():
+        ac = IRAnalysesCache(fn)
+        RemoveUnusedVariablesPass(ac, fn).run_pass()
+
+    assert_ctx_eq(ctx, parse(post))
+
+
+def _check_no_change(pre, scope="basicblock"):
+    _check_pre_post(pre, pre, scope=scope)
+
+
+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:
+        %a = mload 32
+        %b = msize
+        %c_unused = 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_mload_two_msizes():
+    pre = """
+    main:
+        %a = mload 32
+        %b = msize
+        %c = mload 64  # not safe to remove - has MSIZE effect
+        %d = msize
+        return %b, %d
+    """
+    _check_no_change(pre)
+
+
+def test_removeunused_msize_loop():
+    pre = """
+    main:
+        %1 = msize
+
+        # not safe to remove because the previous instruction is
+        # still reachable
+        %2 = mload %1
+
+        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():
+    """
+    Test effect chain removal - remove mload which is initially blocked by
+    an msize but is free to be removed after the msize is removed.
+    """
+    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 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_unused = msize
+        %2_unused = mload 10
+        jmp @main
+    """
+    post = """
+    main:
+        jmp @main
+    """
+    _check_pre_post(pre, post)

vyper/codegen/module.py

@@ -500,7 +500,7 @@
         # 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])

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),
 }

vyper/ir/compile_ir.py

@@ -333,6 +333,17 @@
 
         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]

vyper/utils.py

@@ -75,6 +75,9 @@
         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)

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

vyper/venom/analysis/cfg.py

@@ -51,10 +51,16 @@
         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
 

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)

vyper/venom/basicblock.py

@@ -18,14 +18,11 @@
         "create",
         "create2",
         "invoke",
-        "sload",
         "sstore",
-        "iload",
         "istore",
-        "tload",
+        "itouch",
         "tstore",
         "mstore",
-        "mload",
         "calldatacopy",
         "mcopy",
         "extcodecopy",

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

vyper/venom/passes/remove_unused_variables.py

@@ -1,6 +1,9 @@
+from collections import defaultdict
+
 from vyper.utils import OrderedSet
-from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis
-from vyper.venom.basicblock import IRInstruction
+from vyper.venom import effects
+from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis, ReachableAnalysis
+from vyper.venom.basicblock import IRBasicBlock, IRInstruction
 from vyper.venom.passes.base_pass import IRPass
 
 
@@ -25,6 +28,9 @@
             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 +39,9 @@
             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 +51,4 @@
             new_uses = self.dfg.get_uses(operand)
             self.work_list.addmany(new_uses)
 
-        inst.parent.remove_instruction(inst)
+        bb.mark_for_removal(inst)