-
-
Notifications
You must be signed in to change notification settings - Fork 819
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
charles-cooper
wants to merge
54
commits into
vyperlang:master
Choose a base branch
from
charles-cooper:feat/improve-volatile
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
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 9befa12
handle msize
charles-cooper 863869e
Merge branch 'master' into feat/improve-volatile
charles-cooper fa85193
fix msize effects
charles-cooper ea2ba1d
fix typo
charles-cooper e3575a0
add a note
charles-cooper 12b9604
improve the analysis
charles-cooper 0baece8
typo
charles-cooper 00d4f6f
fix: defaultdict
charles-cooper d8f6a2c
fix the analysis and the pass
charles-cooper 2b95c71
basic tests
HodanPlodky 22de2e3
loop test
HodanPlodky 14894a3
unused msize
HodanPlodky f071b11
test without the msize
HodanPlodky 6cef92b
fix of the order msize
HodanPlodky b9027fc
Merge pull request #53 from HodanPlodky/feat/improve-volatile
charles-cooper 799964f
clean up for range
charles-cooper 2433a4a
fix: remove msize from index
charles-cooper 7235a13
swipe mark_for_removal
charles-cooper 0fe354a
small fixes
charles-cooper 7e8aae5
handle memory effect in loop
charles-cooper 3b68860
Fix type hint
charles-cooper 7196c56
test for msize in branches
HodanPlodky 4ffdf9d
fix jmp => jnz
HodanPlodky 2beca35
Merge pull request #54 from HodanPlodky/feat/improve-volatile
charles-cooper b35d0ea
fix some bad logic
charles-cooper a0d1b3b
Merge branch 'master' into feat/improve-volatile
charles-cooper 303414e
update tests with new machinery
charles-cooper 04c31d6
fix typos
charles-cooper d10df16
fix lint
charles-cooper 08bec15
update comments
charles-cooper 578abe4
rename a variable and add more comments
charles-cooper ebbde05
add another test
charles-cooper 0171aab
move around tests, add test comments
charles-cooper 79eb372
Merge branch 'master' into feat/improve-volatile
charles-cooper ae59fee
fix lint
charles-cooper d15e8a7
Merge branch 'master' into feat/improve-volatile
charles-cooper 0223869
add another test
charles-cooper 69ac671
Merge branch 'master' into feat/improve-volatile
charles-cooper 476507c
Merge branch 'master' into feat/improve-volatile
charles-cooper d580116
simplify remove_unused_variables, add itouch instruction
charles-cooper e487909
fix lint
charles-cooper 7544544
rename a variable
charles-cooper 0ad9a33
update tests - don't need as many tricky mload/msize test cases
charles-cooper 353eb7c
Merge branch 'master' into feat/improve-volatile
charles-cooper 8944ba1
add itouch to PASS_THRU_INSTRUCTIONS
charles-cooper c8290ae
Merge branch 'master' into feat/improve-volatile
charles-cooper d1151a8
fix lint
charles-cooper 9f8f9f9
Merge branch 'master' into feat/improve-volatile
charles-cooper 8c946c6
Merge branch 'master' into feat/improve-volatile
charles-cooper 8d93190
roll back change
charles-cooper d99d9ad
Merge branch 'master' into feat/improve-volatile
charles-cooper aaf5b37
update itouch cost
charles-cooper 0d54ac0
update comment
charles-cooper File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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) | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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