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 49 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
49 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
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
1 change: 1 addition & 0 deletions vyper/venom/analysis/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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

Expand Down
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)

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)
4 changes: 0 additions & 4 deletions vyper/venom/basicblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,10 @@
"create",
"create2",
"invoke",
"sload",
"sstore",
"iload",
"istore",
"tload",
"tstore",
"mstore",
"mload",
"calldatacopy",
"mcopy",
"extcodecopy",
Expand Down
4 changes: 2 additions & 2 deletions vyper/venom/effects.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
21 changes: 20 additions & 1 deletion vyper/venom/passes/remove_unused_variables.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from vyper.utils import OrderedSet
from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis
from vyper.venom import effects
Fixed Show fixed Hide fixed
from vyper.venom.analysis import DFGAnalysis, LivenessAnalysis, ReachableAnalysis
Fixed Show fixed Hide fixed
from vyper.venom.basicblock import IRInstruction
from vyper.venom.passes.base_pass import IRPass

Expand All @@ -11,9 +12,19 @@ class RemoveUnusedVariablesPass(IRPass):

dfg: DFGAnalysis
work_list: OrderedSet[IRInstruction]
reads_msize: bool

def run_pass(self):
self.dfg = self.analyses_cache.request_analysis(DFGAnalysis)
self.reachable = self.analyses_cache.request_analysis(ReachableAnalysis).reachable

self.reads_msize = {}
self.instruction_index = {}
for bb in self.function.get_basic_blocks():
for idx, inst in enumerate(bb.instructions):
self.instruction_index[inst] = idx
if inst.opcode == "msize" and bb not in self.reads_msize:
self.reads_msize[bb] = idx
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should not this store last msize in basic block instead of first msize. This could be the problem in case which is showed in PR charles-cooper#53


work_list = OrderedSet()
self.work_list = work_list
Expand All @@ -33,6 +44,14 @@ def _process_instruction(self, inst):
return
if inst.is_volatile or inst.is_bb_terminator:
return
bb = inst.parent
if effects.MSIZE in inst.get_write_effects():
# msize after memory touch
if bb in self.reads_msize and self.instruction_index[inst] < self.reads_msize[bb]:
return
if any(reachable_bb in self.reachable[bb] for reachable_bb in self.reads_msize):
return

uses = self.dfg.get_uses(inst.output)
if len(uses) > 0:
return
Expand Down
Loading