Skip to content

Commit

Permalink
fix[venom]: liveness analysis in some loops (#3732)
Browse files Browse the repository at this point in the history
fixes a small issue with the calculation of liveness when variables are
consumed inside of loops by refactoring the liveness calculation
algorithm to an iterative version over the recursive one

additional QOL refactoring:
- simplify convert_ir_basicblock - rename it to ir_node_to_venom
- fix bb well-formedness for deploy ir
- don't do deploy IR detection; rely on caller in CompilerData to call for
  both deploy and runtime IR
- remove findIRnode, it's no longer needed
- rename _convert_ir_basicblock to _convert_ir_bb
- add _convert_ir_bb_list helper to handle arglists

---------

Co-authored-by: Charles Cooper <[email protected]>
  • Loading branch information
harkal and charles-cooper authored Jan 17, 2024
1 parent 81c6d8e commit c42b077
Show file tree
Hide file tree
Showing 7 changed files with 162 additions and 153 deletions.
41 changes: 41 additions & 0 deletions tests/unit/compiler/venom/test_convert_basicblock_simple.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
from vyper.codegen.ir_node import IRnode
from vyper.venom.ir_node_to_venom import ir_node_to_venom


def test_simple():
ir = IRnode.from_list(["calldatacopy", 32, 0, ["calldatasize"]])
ir_node = IRnode.from_list(ir)
venom = ir_node_to_venom(ir_node)
assert venom is not None

bb = venom.basic_blocks[0]
assert bb.instructions[0].opcode == "calldatasize"
assert bb.instructions[1].opcode == "calldatacopy"


def test_simple_2():
ir = [
"seq",
[
"seq",
[
"mstore",
["add", 64, 0],
[
"with",
"x",
["calldataload", ["add", 4, 0]],
[
"with",
"ans",
["add", "x", 1],
["seq", ["assert", ["ge", "ans", "x"]], "ans"],
],
],
],
],
32,
]
ir_node = IRnode.from_list(ir)
venom = ir_node_to_venom(ir_node)
assert venom is not None
5 changes: 4 additions & 1 deletion vyper/compiler/phases.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,10 @@ def function_signatures(self) -> dict[str, ContractFunctionT]:

@cached_property
def venom_functions(self):
return generate_ir(self.ir_nodes, self.settings.optimize)
deploy_ir, runtime_ir = self._ir_output
deploy_venom = generate_ir(deploy_ir, self.settings.optimize)
runtime_venom = generate_ir(runtime_ir, self.settings.optimize)
return deploy_venom, runtime_venom

@cached_property
def assembly(self) -> list:
Expand Down
10 changes: 4 additions & 6 deletions vyper/venom/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
ir_pass_remove_unreachable_blocks,
)
from vyper.venom.function import IRFunction
from vyper.venom.ir_node_to_venom import convert_ir_basicblock
from vyper.venom.ir_node_to_venom import ir_node_to_venom
from vyper.venom.passes.constant_propagation import ir_pass_constant_propagation
from vyper.venom.passes.dft import DFTPass
from vyper.venom.venom_to_assembly import VenomCompiler
Expand Down Expand Up @@ -61,11 +61,9 @@ def _run_passes(ctx: IRFunction, optimize: OptimizationLevel) -> None:
break


def generate_ir(ir: IRnode, optimize: OptimizationLevel) -> tuple[IRFunction, IRFunction]:
def generate_ir(ir: IRnode, optimize: OptimizationLevel) -> IRFunction:
# Convert "old" IR to "new" IR
ctx, ctx_runtime = convert_ir_basicblock(ir)

ctx = ir_node_to_venom(ir)
_run_passes(ctx, optimize)
_run_passes(ctx_runtime, optimize)

return ctx, ctx_runtime
return ctx
36 changes: 20 additions & 16 deletions vyper/venom/analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,12 @@ def _reset_liveness(ctx: IRFunction) -> None:
inst.liveness = OrderedSet()


def _calculate_liveness_bb(bb: IRBasicBlock) -> None:
def _calculate_liveness(bb: IRBasicBlock) -> bool:
"""
Compute liveness of each instruction in the basic block.
Returns True if liveness changed
"""
orig_liveness = bb.instructions[0].liveness.copy()
liveness = bb.out_vars.copy()
for instruction in reversed(bb.instructions):
ops = instruction.get_inputs()
Expand All @@ -60,29 +62,31 @@ def _calculate_liveness_bb(bb: IRBasicBlock) -> None:
liveness.remove(out)
instruction.liveness = liveness

return orig_liveness != bb.instructions[0].liveness

def _calculate_liveness_r(bb: IRBasicBlock, visited: dict) -> None:
assert isinstance(visited, dict)
for out_bb in bb.cfg_out:
if visited.get(bb) == out_bb:
continue
visited[bb] = out_bb

# recurse
_calculate_liveness_r(out_bb, visited)

def _calculate_out_vars(bb: IRBasicBlock) -> bool:
"""
Compute out_vars of basic block.
Returns True if out_vars changed
"""
out_vars = bb.out_vars.copy()
for out_bb in bb.cfg_out:
target_vars = input_vars_from(bb, out_bb)

# the output stack layout for bb. it produces a stack layout
# which works for all possible cfg_outs from the bb.
bb.out_vars = bb.out_vars.union(target_vars)

_calculate_liveness_bb(bb)
return out_vars != bb.out_vars


def calculate_liveness(ctx: IRFunction) -> None:
_reset_liveness(ctx)
_calculate_liveness_r(ctx.basic_blocks[0], dict())
while True:
changed = False
for bb in ctx.basic_blocks:
changed |= _calculate_out_vars(bb)
changed |= _calculate_liveness(bb)

if not changed:
break


# calculate the input variables into self from source
Expand Down
4 changes: 2 additions & 2 deletions vyper/venom/basicblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -410,8 +410,8 @@ def copy(self):
def __repr__(self) -> str:
s = (
f"{repr(self.label)}: IN={[bb.label for bb in self.cfg_in]}"
f" OUT={[bb.label for bb in self.cfg_out]} => {self.out_vars} \n"
f" OUT={[bb.label for bb in self.cfg_out]} => {self.out_vars}\n"
)
for instruction in self.instructions:
s += f" {instruction}\n"
s += f" {str(instruction).strip()}\n"
return s
2 changes: 1 addition & 1 deletion vyper/venom/function.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,4 @@ def __repr__(self) -> str:
str += "Data segment:\n"
for inst in self.data_segment:
str += f"{inst}\n"
return str
return str.strip()
Loading

0 comments on commit c42b077

Please sign in to comment.