Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat[venom]: mark loads as non-volatile #4388

Open
wants to merge 54 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
51cbd35
feat[venom]: mark loads as non-volatile
charles-cooper Dec 4, 2024
9befa12
handle msize
charles-cooper Dec 7, 2024
863869e
Merge branch 'master' into feat/improve-volatile
charles-cooper Dec 7, 2024
fa85193
fix msize effects
charles-cooper Dec 7, 2024
ea2ba1d
fix typo
charles-cooper Dec 7, 2024
e3575a0
add a note
charles-cooper Dec 7, 2024
12b9604
improve the analysis
charles-cooper Dec 7, 2024
0baece8
typo
charles-cooper Dec 7, 2024
00d4f6f
fix: defaultdict
charles-cooper Dec 7, 2024
d8f6a2c
fix the analysis and the pass
charles-cooper Dec 7, 2024
2b95c71
basic tests
HodanPlodky Dec 9, 2024
22de2e3
loop test
HodanPlodky Dec 9, 2024
14894a3
unused msize
HodanPlodky Dec 9, 2024
f071b11
test without the msize
HodanPlodky Dec 9, 2024
6cef92b
fix of the order msize
HodanPlodky Dec 9, 2024
b9027fc
Merge pull request #53 from HodanPlodky/feat/improve-volatile
charles-cooper Dec 9, 2024
799964f
clean up for range
charles-cooper Dec 9, 2024
2433a4a
fix: remove msize from index
charles-cooper Dec 9, 2024
7235a13
swipe mark_for_removal
charles-cooper Dec 9, 2024
0fe354a
small fixes
charles-cooper Dec 9, 2024
7e8aae5
handle memory effect in loop
charles-cooper Dec 9, 2024
3b68860
Fix type hint
charles-cooper Dec 9, 2024
7196c56
test for msize in branches
HodanPlodky Dec 9, 2024
4ffdf9d
fix jmp => jnz
HodanPlodky Dec 9, 2024
2beca35
Merge pull request #54 from HodanPlodky/feat/improve-volatile
charles-cooper Dec 9, 2024
b35d0ea
fix some bad logic
charles-cooper Dec 9, 2024
a0d1b3b
Merge branch 'master' into feat/improve-volatile
charles-cooper Dec 18, 2024
303414e
update tests with new machinery
charles-cooper Dec 18, 2024
04c31d6
fix typos
charles-cooper Dec 18, 2024
d10df16
fix lint
charles-cooper Dec 18, 2024
08bec15
update comments
charles-cooper Dec 18, 2024
578abe4
rename a variable and add more comments
charles-cooper Dec 18, 2024
ebbde05
add another test
charles-cooper Dec 18, 2024
0171aab
move around tests, add test comments
charles-cooper Dec 18, 2024
79eb372
Merge branch 'master' into feat/improve-volatile
charles-cooper Dec 18, 2024
ae59fee
fix lint
charles-cooper Dec 20, 2024
d15e8a7
Merge branch 'master' into feat/improve-volatile
charles-cooper Dec 20, 2024
0223869
add another test
charles-cooper Dec 20, 2024
69ac671
Merge branch 'master' into feat/improve-volatile
charles-cooper Dec 20, 2024
476507c
Merge branch 'master' into feat/improve-volatile
charles-cooper Dec 22, 2024
d580116
simplify remove_unused_variables, add itouch instruction
charles-cooper Dec 22, 2024
e487909
fix lint
charles-cooper Dec 30, 2024
7544544
rename a variable
charles-cooper Dec 30, 2024
0ad9a33
update tests - don't need as many tricky mload/msize test cases
charles-cooper Dec 30, 2024
353eb7c
Merge branch 'master' into feat/improve-volatile
charles-cooper Dec 30, 2024
8944ba1
add itouch to PASS_THRU_INSTRUCTIONS
charles-cooper Dec 31, 2024
c8290ae
Merge branch 'master' into feat/improve-volatile
charles-cooper Jan 6, 2025
d1151a8
fix lint
charles-cooper Jan 6, 2025
9f8f9f9
Merge branch 'master' into feat/improve-volatile
charles-cooper Jan 9, 2025
8c946c6
Merge branch 'master' into feat/improve-volatile
charles-cooper Jan 12, 2025
8d93190
roll back change
charles-cooper Jan 12, 2025
d99d9ad
Merge branch 'master' into feat/improve-volatile
charles-cooper Jan 20, 2025
aaf5b37
update itouch cost
charles-cooper Jan 20, 2025
0d54ac0
update comment
charles-cooper Jan 20, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
165 changes: 165 additions & 0 deletions tests/unit/compiler/venom/test_removeunused.py
Original file line number Diff line number Diff line change
@@ -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)
6 changes: 4 additions & 2 deletions vyper/codegen/module.py
Original file line number Diff line number Diff line change
@@ -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_start> + immutables_len` (where <immutables_start> ==
# `_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)])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's update the comment above to reflect the new instruction


deploy_code.append(init_func_ir)
deploy_code.append(["deploy", init_mem_used, runtime, immutables_len])
1 change: 1 addition & 0 deletions vyper/evm/opcodes.py
Original file line number Diff line number Diff line change
@@ -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),
}
10 changes: 10 additions & 0 deletions vyper/ir/compile_ir.py
Original file line number Diff line number Diff line change
@@ -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]
3 changes: 3 additions & 0 deletions vyper/utils.py
Original file line number Diff line number Diff line change
@@ -75,6 +75,9 @@
for item in iterable:
self._data.pop(item, None)

def clear(self):
self._data.clear()

Check warning on line 79 in vyper/utils.py

Codecov / codecov/patch

vyper/utils.py#L79

Added line #L79 was not covered by tests

def difference(self, other):
ret = self.copy()
ret.dropmany(other)
1 change: 1 addition & 0 deletions vyper/venom/analysis/__init__.py
Original file line number Diff line number Diff line change
@@ -4,3 +4,4 @@
from .dominators import DominatorTreeAnalysis
from .equivalent_vars import VarEquivalenceAnalysis
from .liveness import LivenessAnalysis
from .reachable import ReachableAnalysis
8 changes: 7 additions & 1 deletion vyper/venom/analysis/cfg.py
Original file line number Diff line number Diff line change
@@ -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

42 changes: 42 additions & 0 deletions vyper/venom/analysis/reachable.py
Original file line number Diff line number Diff line change
@@ -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)

Check warning on line 18 in vyper/venom/analysis/reachable.py

Codecov / codecov/patch

vyper/venom/analysis/reachable.py#L17-L18

Added lines #L17 - L18 were not covered by tests

self._compute_reachable_r(self.function.entry)

Check warning on line 20 in vyper/venom/analysis/reachable.py

Codecov / codecov/patch

vyper/venom/analysis/reachable.py#L20

Added line #L20 was not covered by tests

def _compute_reachable_r(self, bb):
if bb in self.reachable:
return

Check warning on line 24 in vyper/venom/analysis/reachable.py

Codecov / codecov/patch

vyper/venom/analysis/reachable.py#L24

Added line #L24 was not covered by tests

s = bb.cfg_out.copy()
self.reachable[bb] = s

Check warning on line 27 in vyper/venom/analysis/reachable.py

Codecov / codecov/patch

vyper/venom/analysis/reachable.py#L26-L27

Added lines #L26 - L27 were not covered by tests

for out_bb in bb.cfg_out:
self._compute_reachable_r(out_bb)
s.update(self.reachable[out_bb])

Check warning on line 31 in vyper/venom/analysis/reachable.py

Codecov / codecov/patch

vyper/venom/analysis/reachable.py#L30-L31

Added lines #L30 - L31 were not covered by tests

def invalidate(self):
from vyper.venom.analysis import DFGAnalysis, DominatorTreeAnalysis, LivenessAnalysis

Check warning on line 34 in vyper/venom/analysis/reachable.py

Codecov / codecov/patch

vyper/venom/analysis/reachable.py#L34

Added line #L34 was not covered by tests

self.analyses_cache.invalidate_analysis(DominatorTreeAnalysis)
self.analyses_cache.invalidate_analysis(LivenessAnalysis)

Check warning on line 37 in vyper/venom/analysis/reachable.py

Codecov / codecov/patch

vyper/venom/analysis/reachable.py#L36-L37

Added lines #L36 - L37 were not covered by tests

self._dfs = None

Check warning on line 39 in vyper/venom/analysis/reachable.py

Codecov / codecov/patch

vyper/venom/analysis/reachable.py#L39

Added line #L39 was not covered by tests

# be conservative - assume cfg invalidation invalidates dfg
self.analyses_cache.invalidate_analysis(DFGAnalysis)

Check warning on line 42 in vyper/venom/analysis/reachable.py

Codecov / codecov/patch

vyper/venom/analysis/reachable.py#L42

Added line #L42 was not covered by tests
6 changes: 2 additions & 4 deletions vyper/venom/basicblock.py
Original file line number Diff line number Diff line change
@@ -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",
5 changes: 3 additions & 2 deletions vyper/venom/effects.py
Original file line number Diff line number Diff line change
@@ -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
3 changes: 1 addition & 2 deletions vyper/venom/ir_node_to_venom.py
Original file line number Diff line number Diff line change
@@ -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)

12 changes: 9 additions & 3 deletions vyper/venom/passes/remove_unused_variables.py
Original file line number Diff line number Diff line change
@@ -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)
Loading