-
-
Notifications
You must be signed in to change notification settings - Fork 821
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
base: master
Are you sure you want to change the base?
Changes from 51 commits
51cbd35
9befa12
863869e
fa85193
ea2ba1d
e3575a0
12b9604
0baece8
00d4f6f
d8f6a2c
2b95c71
22de2e3
14894a3
f071b11
6cef92b
b9027fc
799964f
2433a4a
7235a13
0fe354a
7e8aae5
3b68860
7196c56
4ffdf9d
2beca35
b35d0ea
a0d1b3b
303414e
04c31d6
d10df16
08bec15
578abe4
ebbde05
0171aab
79eb372
ae59fee
d15e8a7
0223869
69ac671
476507c
d580116
e487909
7544544
0ad9a33
353eb7c
8944ba1
c8290ae
d1151a8
9f8f9f9
8c946c6
8d93190
d99d9ad
aaf5b37
0d54ac0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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) |
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, 6), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't the cost be higher than There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm, that's true There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (btw i think issues with gas estimates are at best a UX issue since we don't use them for anything besides providing gas estimates in the ABI and IR printouts) |
||
"DLOAD": (None, 1, 1, 9), | ||
"DLOADBYTES": (None, 3, 0, 3), | ||
} | ||
|
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) | ||
|
||
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) | ||
There was a problem hiding this comment.
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