feat[venom]: mark loads as non-volatile

tests/unit/compiler/venom/test_removeunused.py

@@ -0,0 +1,192 @@
+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_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/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)

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 @@ 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
 

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

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

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 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
 
 
@@ -11,9 +14,27 @@ 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
@@ -25,14 +46,45 @@ def run_pass(self):
             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)
 
+    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
         if inst.is_volatile or inst.is_bb_terminator:
             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:
             return
@@ -42,4 +94,11 @@ 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 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)