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 51 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
51 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
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
156 changes: 156 additions & 0 deletions tests/unit/compiler/venom/test_removeunused.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
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_msize_basic():
pre = """
main:
%a = mload 32
%b = msize
%c = 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_msize_two_msizes():
pre = """
main:
%a = mload 32
%b = msize
%c = mload 64 # not safe to remove
%d = msize
return %b, %d
"""
_check_no_change(pre)


def test_removeunused_msize_loop():
pre = """
main:
%msize = msize

# not safe to remove because the previous instruction is
# still reachable
%1 = mload %msize
jmp @main
"""
_check_no_change(pre)


def test_removeunused_msize_branches():
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_removeunused_unused_msize_loop():
pre = """
main:
%1_unused = msize
%2_unused = mload 10
jmp @main
"""
post = """
main:
jmp @main
"""
_check_pre_post(pre, post)


def test_removeunused_unused_msize():
pre = """
main:
%1_unused = mload 10
%2_unused = msize
stop
"""
post = """
main:
stop
"""
_check_pre_post(pre, post)



def test_removeunused_loop():
pre = """
main:
%1 = 10
jmp @after
after:
%p = phi @main, %1, @after, %2
%2 = add %p, 1
%3_unused add %2, %p
charles-cooper marked this conversation as resolved.
Show resolved Hide resolved
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)
3 changes: 3 additions & 0 deletions vyper/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
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)
23 changes: 14 additions & 9 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 Expand Up @@ -206,7 +202,7 @@ class IRInstruction:

opcode: str
operands: list[IROperand]
output: Optional[IROperand]
output: Optional[IRVariable]
# set of live variables at this instruction
liveness: OrderedSet[IRVariable]
parent: "IRBasicBlock"
Expand All @@ -218,7 +214,7 @@ def __init__(
self,
opcode: str,
operands: list[IROperand] | Iterator[IROperand],
output: Optional[IROperand] = None,
output: Optional[IRVariable] = None,
):
assert isinstance(opcode, str), "opcode must be an str"
assert isinstance(operands, list | Iterator), "operands must be a list"
Expand Down Expand Up @@ -452,6 +448,8 @@ def __init__(self, label: IRLabel, parent: "IRFunction") -> None:
self.out_vars = OrderedSet()
self.is_reachable = False

self._garbage_instructions: set[IRInstruction] = set()

def add_cfg_in(self, bb: "IRBasicBlock") -> None:
self.cfg_in.add(bb)

Expand Down Expand Up @@ -526,13 +524,20 @@ def insert_instruction(self, instruction: IRInstruction, index: Optional[int] =
instruction.error_msg = self.parent.error_msg
self.instructions.insert(index, instruction)

def mark_for_removal(self, instruction: IRInstruction) -> None:
self._garbage_instructions.add(instruction)

def clear_dead_instructions(self) -> None:
if len(self._garbage_instructions) > 0:
self.instructions = [
inst for inst in self.instructions if inst not in self._garbage_instructions
]
self._garbage_instructions.clear()

def remove_instruction(self, instruction: IRInstruction) -> None:
assert isinstance(instruction, IRInstruction), "instruction must be an IRInstruction"
self.instructions.remove(instruction)

def clear_instructions(self) -> None:
self.instructions = []

@property
def phi_instructions(self) -> Iterator[IRInstruction]:
for inst in self.instructions:
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
Loading
Loading