Skip to content

Commit

Permalink
fix last-use analysis
Browse files Browse the repository at this point in the history
use_count inside of Expr is dangerous, since Expr() can be called
multiple times for a given node. separate the analysis.
  • Loading branch information
charles-cooper committed Sep 2, 2024
1 parent 7d8b64f commit e0261dc
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 12 deletions.
3 changes: 1 addition & 2 deletions examples/tokens/ERC1155ownable.vy
Original file line number Diff line number Diff line change
Expand Up @@ -404,5 +404,4 @@ def supportsInterface(interfaceId: bytes4) -> bool:
ERC165_INTERFACE_ID,
ERC1155_INTERFACE_ID,
ERC1155_INTERFACE_ID_METADATA,
]

]
6 changes: 2 additions & 4 deletions vyper/codegen/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ class VariableRecord:
defined_at: Any = None
is_internal: bool = False
alloca: Optional[Alloca] = None
use_count: int = 0
system: bool = False

# the following members are probably dead
Expand Down Expand Up @@ -123,7 +122,7 @@ def __init__(

self.settings = get_global_settings()

self._to_deallocate = []
self._to_deallocate = set()

def is_constant(self):
return self.constancy is Constancy.Constant or self.in_range_expr
Expand Down Expand Up @@ -217,13 +216,12 @@ def deallocate_variable(self, varname, var):
del self.vars[var.name]

def mark_for_deallocation(self, varname):
assert varname not in self._to_deallocate
# for variables get deallocated anyway
if varname in self.forvars:
return
if self.vars[varname].system:
return
self._to_deallocate.append(varname)
self._to_deallocate.add(varname)

# "mark-and-sweep", haha
def sweep(self):
Expand Down
4 changes: 2 additions & 2 deletions vyper/codegen/expr.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,9 @@ def parse_Name(self):
ret = var.as_ir_node()
ret._referenced_variables = {varinfo}

if var.use_count == varinfo._use_count and var.location == MEMORY:
last_use = self.expr._metadata.get("last_use", False)
if last_use and var.location == MEMORY:
self.context.mark_for_deallocation(varname)
var.use_count += 1

return ret

Expand Down
26 changes: 25 additions & 1 deletion vyper/codegen/function_definitions/common.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,37 @@
from collections import defaultdict
from dataclasses import dataclass
from functools import cached_property
from typing import Optional
from typing import TYPE_CHECKING, Optional

import vyper.ast as vy_ast
from vyper.codegen.context import Constancy, Context
from vyper.codegen.ir_node import IRnode
from vyper.codegen.memory_allocator import MemoryAllocator
from vyper.evm.opcodes import version_check
from vyper.exceptions import CompilerPanic
from vyper.semantics.types import VyperType
from vyper.semantics.types.function import ContractFunctionT, StateMutability
from vyper.semantics.types.module import ModuleT
from vyper.utils import MemoryPositions

if TYPE_CHECKING:
from vyper.semantics.analysis.base import VarInfo


def analyse_last_use(fn_ast: vy_ast.FunctionDef):
counts: dict[VarInfo, int] = defaultdict(lambda: 0)
for stmt in fn_ast.body:
for expr in stmt.get_descendants(vy_ast.ExprNode):
info = expr._expr_info
if info is None:
continue
for r in info._reads:
counts[r.variable] += 1
if r.variable._use_count < counts[r.variable]: # pragma: nocover
raise CompilerPanic("unreachable!")
if r.variable._use_count == counts[r.variable]:
expr._metadata["last_use"] = True


@dataclass
class FrameInfo:
Expand Down Expand Up @@ -114,6 +135,9 @@ def initialize_context(
):
init_ir_info(func_t)

assert isinstance(func_t.ast_def, vy_ast.FunctionDef) # help mypy
analyse_last_use(func_t.ast_def)

# calculate starting frame
callees = func_t.called_functions
# we start our function frame from the largest callee frame
Expand Down
3 changes: 1 addition & 2 deletions vyper/codegen/stmt.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ def __init__(self, node: vy_ast.VyperNode, context: Context) -> None:
fn = getattr(self, fn_name)
with context.internal_memory_scope():
self.ir_node = fn()
context.sweep()

assert isinstance(self.ir_node, IRnode), self.ir_node

Expand Down Expand Up @@ -346,8 +347,6 @@ def parse_body(code, context, ensure_terminated=False):
ir = parse_stmt(stmt, context)
ir_node.append(ir)

context.sweep()

# force using the return routine / exit_to cleanup for end of function
if ensure_terminated and context.return_type is None and not _is_terminated(code):
ir_node.append(parse_stmt(vy_ast.Return(value=None), context))
Expand Down
4 changes: 3 additions & 1 deletion vyper/semantics/analysis/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,9 @@ def visit(self, node, typ):
var_access = _get_variable_access(node)
if var_access is not None:
info._reads.add(var_access)
var_access.variable._use_count += 1

for r in info._reads:
r.variable._use_count += 1

if self.function_analyzer:
for s in self.function_analyzer.loop_variables:
Expand Down

0 comments on commit e0261dc

Please sign in to comment.