diff --git a/tests/functional/codegen/features/test_constructor.py b/tests/functional/codegen/features/test_constructor.py index 3b86fe3460..6cc7007bb2 100644 --- a/tests/functional/codegen/features/test_constructor.py +++ b/tests/functional/codegen/features/test_constructor.py @@ -1,7 +1,4 @@ -import pytest - from tests.evm_backends.base_env import _compile -from vyper.exceptions import StackTooDeep from vyper.utils import method_id @@ -169,7 +166,6 @@ def get_foo() -> uint256: assert c.get_foo() == 39 -@pytest.mark.venom_xfail(raises=StackTooDeep, reason="stack scheduler regression") def test_nested_dynamic_array_constructor_arg_2(env, get_contract): code = """ foo: int128 diff --git a/tests/functional/codegen/types/test_dynamic_array.py b/tests/functional/codegen/types/test_dynamic_array.py index 2a0f4e77e5..2f647ac38c 100644 --- a/tests/functional/codegen/types/test_dynamic_array.py +++ b/tests/functional/codegen/types/test_dynamic_array.py @@ -11,7 +11,6 @@ CompilerPanic, ImmutableViolation, OverflowException, - StackTooDeep, StateAccessViolation, TypeMismatch, ) @@ -737,7 +736,6 @@ def test_array_decimal_return3() -> DynArray[DynArray[decimal, 2], 2]: ] -@pytest.mark.venom_xfail(raises=StackTooDeep, reason="stack scheduler regression") def test_mult_list(get_contract): code = """ nest3: DynArray[DynArray[DynArray[uint256, 2], 2], 2] diff --git a/tests/unit/compiler/venom/test_sccp.py b/tests/unit/compiler/venom/test_sccp.py index 375dfd5dac..0d46b61acd 100644 --- a/tests/unit/compiler/venom/test_sccp.py +++ b/tests/unit/compiler/venom/test_sccp.py @@ -167,8 +167,8 @@ def test_cont_phi_case(): assert sccp.lattice[IRVariable("%2")].value == 32 assert sccp.lattice[IRVariable("%3")].value == 64 assert sccp.lattice[IRVariable("%4")].value == 96 - assert sccp.lattice[IRVariable("%5", version=1)].value == 106 - assert sccp.lattice[IRVariable("%5", version=2)] == LatticeEnum.BOTTOM + assert sccp.lattice[IRVariable("%5", version=2)].value == 106 + assert sccp.lattice[IRVariable("%5", version=1)] == LatticeEnum.BOTTOM assert sccp.lattice[IRVariable("%5")].value == 2 @@ -207,8 +207,9 @@ def test_cont_phi_const_case(): assert sccp.lattice[IRVariable("%2")].value == 32 assert sccp.lattice[IRVariable("%3")].value == 64 assert sccp.lattice[IRVariable("%4")].value == 96 - assert sccp.lattice[IRVariable("%5", version=1)].value == 106 - assert sccp.lattice[IRVariable("%5", version=2)].value == 97 + # dependent on cfg traversal order + assert sccp.lattice[IRVariable("%5", version=2)].value == 106 + assert sccp.lattice[IRVariable("%5", version=1)].value == 97 assert sccp.lattice[IRVariable("%5")].value == 2 diff --git a/vyper/venom/analysis/cfg.py b/vyper/venom/analysis/cfg.py index 90b18b353c..700fd73f26 100644 --- a/vyper/venom/analysis/cfg.py +++ b/vyper/venom/analysis/cfg.py @@ -1,6 +1,8 @@ +from typing import Iterator + from vyper.utils import OrderedSet from vyper.venom.analysis import IRAnalysis -from vyper.venom.basicblock import CFG_ALTERING_INSTRUCTIONS +from vyper.venom.basicblock import CFG_ALTERING_INSTRUCTIONS, IRBasicBlock class CFGAnalysis(IRAnalysis): @@ -8,28 +10,45 @@ class CFGAnalysis(IRAnalysis): Compute control flow graph information for each basic block in the function. """ + _dfs: OrderedSet[IRBasicBlock] + def analyze(self) -> None: fn = self.function + self._dfs = OrderedSet() + for bb in fn.get_basic_blocks(): bb.cfg_in = OrderedSet() bb.cfg_out = OrderedSet() bb.out_vars = OrderedSet() + bb.is_reachable = False for bb in fn.get_basic_blocks(): - assert len(bb.instructions) > 0, "Basic block should not be empty" - last_inst = bb.instructions[-1] - assert last_inst.is_bb_terminator, f"Last instruction should be a terminator {bb}" + assert bb.is_terminated - for inst in bb.instructions: - if inst.opcode in CFG_ALTERING_INSTRUCTIONS: - ops = inst.get_label_operands() - for op in ops: - fn.get_basic_block(op.value).add_cfg_in(bb) + term = bb.instructions[-1] + if term.opcode in CFG_ALTERING_INSTRUCTIONS: + ops = term.get_label_operands() + # order of cfg_out matters to performance! + for op in reversed(list(ops)): + next_bb = fn.get_basic_block(op.value) + bb.add_cfg_out(next_bb) + next_bb.add_cfg_in(bb) - # Fill in the "out" set for each basic block - for bb in fn.get_basic_blocks(): - for in_bb in bb.cfg_in: - in_bb.add_cfg_out(bb) + self._compute_dfs_r(self.function.entry) + + def _compute_dfs_r(self, bb): + if bb.is_reachable: + return + bb.is_reachable = True + + for out_bb in bb.cfg_out: + self._compute_dfs_r(out_bb) + + self._dfs.add(bb) + + @property + def dfs_walk(self) -> Iterator[IRBasicBlock]: + return iter(self._dfs) def invalidate(self): from vyper.venom.analysis import DFGAnalysis, DominatorTreeAnalysis, LivenessAnalysis @@ -37,5 +56,7 @@ def invalidate(self): 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) diff --git a/vyper/venom/analysis/dominators.py b/vyper/venom/analysis/dominators.py index e360df36b9..b60f9bdab9 100644 --- a/vyper/venom/analysis/dominators.py +++ b/vyper/venom/analysis/dominators.py @@ -1,3 +1,5 @@ +from functools import cached_property + from vyper.exceptions import CompilerPanic from vyper.utils import OrderedSet from vyper.venom.analysis import CFGAnalysis, IRAnalysis @@ -14,8 +16,6 @@ class DominatorTreeAnalysis(IRAnalysis): fn: IRFunction entry_block: IRBasicBlock - dfs_order: dict[IRBasicBlock, int] - dfs_walk: list[IRBasicBlock] dominators: dict[IRBasicBlock, OrderedSet[IRBasicBlock]] immediate_dominators: dict[IRBasicBlock, IRBasicBlock] dominated: dict[IRBasicBlock, OrderedSet[IRBasicBlock]] @@ -27,16 +27,13 @@ def analyze(self): """ self.fn = self.function self.entry_block = self.fn.entry - self.dfs_order = {} - self.dfs_walk = [] self.dominators = {} self.immediate_dominators = {} self.dominated = {} self.dominator_frontiers = {} - self.analyses_cache.request_analysis(CFGAnalysis) + self.cfg = self.analyses_cache.request_analysis(CFGAnalysis) - self._compute_dfs(self.entry_block, OrderedSet()) self._compute_dominators() self._compute_idoms() self._compute_df() @@ -131,21 +128,13 @@ def _intersect(self, bb1, bb2): bb2 = self.immediate_dominators[bb2] return bb1 - def _compute_dfs(self, entry: IRBasicBlock, visited): - """ - Depth-first search to compute the DFS order of the basic blocks. This - is used to compute the dominator tree. The sequence of basic blocks in - the DFS order is stored in `self.dfs_walk`. The DFS order of each basic - block is stored in `self.dfs_order`. - """ - visited.add(entry) - - for bb in entry.cfg_out: - if bb not in visited: - self._compute_dfs(bb, visited) + @cached_property + def dfs_walk(self) -> list[IRBasicBlock]: + return list(self.cfg.dfs_walk) - self.dfs_walk.append(entry) - self.dfs_order[entry] = len(self.dfs_walk) + @cached_property + def dfs_order(self) -> dict[IRBasicBlock, int]: + return {bb: idx for idx, bb in enumerate(self.dfs_walk)} def as_graph(self) -> str: """ diff --git a/vyper/venom/analysis/liveness.py b/vyper/venom/analysis/liveness.py index b5d65961b7..b9b0f29672 100644 --- a/vyper/venom/analysis/liveness.py +++ b/vyper/venom/analysis/liveness.py @@ -64,7 +64,7 @@ def _calculate_out_vars(self, bb: IRBasicBlock) -> bool: bb.out_vars = OrderedSet() for out_bb in bb.cfg_out: target_vars = self.input_vars_from(bb, out_bb) - bb.out_vars = bb.out_vars.union(target_vars) + bb.out_vars.update(target_vars) return out_vars != bb.out_vars # calculate the input variables into self from source diff --git a/vyper/venom/basicblock.py b/vyper/venom/basicblock.py index eb4f7e67ba..f73f847a62 100644 --- a/vyper/venom/basicblock.py +++ b/vyper/venom/basicblock.py @@ -185,15 +185,6 @@ def __init__(self, value: str, is_symbol: bool = False) -> None: self.value = value self.is_symbol = is_symbol - def __eq__(self, other): - # no need for is_symbol to participate in equality - return super().__eq__(other) - - def __hash__(self): - # __hash__ is required when __eq__ is overridden -- - # https://docs.python.org/3/reference/datamodel.html#object.__hash__ - return super().__hash__() - class IRInstruction: """ @@ -393,7 +384,6 @@ class IRBasicBlock: # stack items which this basic block produces out_vars: OrderedSet[IRVariable] - reachable: OrderedSet["IRBasicBlock"] is_reachable: bool = False def __init__(self, label: IRLabel, parent: "IRFunction") -> None: @@ -404,7 +394,6 @@ def __init__(self, label: IRLabel, parent: "IRFunction") -> None: self.cfg_in = OrderedSet() self.cfg_out = OrderedSet() self.out_vars = OrderedSet() - self.reachable = OrderedSet() self.is_reachable = False def add_cfg_in(self, bb: "IRBasicBlock") -> None: @@ -495,6 +484,32 @@ def replace_operands(self, replacements: dict) -> None: for instruction in self.instructions: instruction.replace_operands(replacements) + def fix_phi_instructions(self): + cfg_in_labels = tuple(bb.label for bb in self.cfg_in) + + needs_sort = False + for inst in self.instructions: + if inst.opcode != "phi": + continue + + labels = inst.get_label_operands() + for label in labels: + if label not in cfg_in_labels: + needs_sort = True + inst.remove_phi_operand(label) + + op_len = len(inst.operands) + if op_len == 2: + inst.opcode = "store" + inst.operands = [inst.operands[1]] + elif op_len == 0: + inst.opcode = "nop" + inst.output = None + inst.operands = [] + + if needs_sort: + self.instructions.sort(key=lambda inst: inst.opcode != "phi") + def get_assignments(self): """ Get all assignments in basic block. diff --git a/vyper/venom/function.py b/vyper/venom/function.py index 2f2d477460..503435ca0e 100644 --- a/vyper/venom/function.py +++ b/vyper/venom/function.py @@ -1,8 +1,7 @@ from typing import Iterator, Optional from vyper.codegen.ir_node import IRnode -from vyper.utils import OrderedSet -from vyper.venom.basicblock import CFG_ALTERING_INSTRUCTIONS, IRBasicBlock, IRLabel, IRVariable +from vyper.venom.basicblock import IRBasicBlock, IRLabel, IRVariable class IRFunction: @@ -89,60 +88,33 @@ def get_last_variable(self) -> str: return f"%{self.last_variable}" def remove_unreachable_blocks(self) -> int: - self._compute_reachability() + # Remove unreachable basic blocks + # pre: requires CFG analysis! + # NOTE: should this be a pass? - removed = [] + removed = set() - # Remove unreachable basic blocks for bb in self.get_basic_blocks(): if not bb.is_reachable: - removed.append(bb) + removed.add(bb) for bb in removed: self.remove_basic_block(bb) # Remove phi instructions that reference removed basic blocks - for bb in removed: - for out_bb in bb.cfg_out: - out_bb.remove_cfg_in(bb) - for inst in out_bb.instructions: - if inst.opcode != "phi": - continue - in_labels = inst.get_label_operands() - if bb.label in in_labels: - inst.remove_phi_operand(bb.label) - op_len = len(inst.operands) - if op_len == 2: - inst.opcode = "store" - inst.operands = [inst.operands[1]] - elif op_len == 0: - out_bb.remove_instruction(inst) - - return len(removed) - - def _compute_reachability(self) -> None: - """ - Compute reachability of basic blocks. - """ for bb in self.get_basic_blocks(): - bb.reachable = OrderedSet() - bb.is_reachable = False + modified = False + for in_bb in list(bb.cfg_in): + if in_bb not in removed: + continue - self._compute_reachability_from(self.entry) + modified = True + bb.remove_cfg_in(in_bb) - def _compute_reachability_from(self, bb: IRBasicBlock) -> None: - """ - Compute reachability of basic blocks from bb. - """ - if bb.is_reachable: - return - bb.is_reachable = True - for inst in bb.instructions: - if inst.opcode in CFG_ALTERING_INSTRUCTIONS: - for op in inst.get_label_operands(): - out_bb = self.get_basic_block(op.value) - bb.reachable.add(out_bb) - self._compute_reachability_from(out_bb) + if modified or True: + bb.fix_phi_instructions() + + return len(removed) @property def normalized(self) -> bool: diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index d85e09c9b4..2bdd0ace44 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -5,7 +5,7 @@ from vyper.exceptions import CompilerPanic, StaticAssertionException from vyper.utils import OrderedSet -from vyper.venom.analysis import CFGAnalysis, DFGAnalysis, DominatorTreeAnalysis, IRAnalysesCache +from vyper.venom.analysis import CFGAnalysis, DFGAnalysis, IRAnalysesCache from vyper.venom.basicblock import ( IRBasicBlock, IRInstruction, @@ -50,7 +50,6 @@ class SCCP(IRPass): """ fn: IRFunction - dom: DominatorTreeAnalysis dfg: DFGAnalysis lattice: Lattice work_list: list[WorkListItem] @@ -66,14 +65,13 @@ def __init__(self, analyses_cache: IRAnalysesCache, function: IRFunction): def run_pass(self): self.fn = self.function - self.dom = self.analyses_cache.request_analysis(DominatorTreeAnalysis) self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) self._calculate_sccp(self.fn.entry) self._propagate_constants() if self.cfg_dirty: self.analyses_cache.force_analysis(CFGAnalysis) - self._fix_phi_nodes() + self.fn.remove_unreachable_blocks() self.analyses_cache.invalidate_analysis(DFGAnalysis) @@ -269,7 +267,7 @@ def _propagate_constants(self): with their actual values. It also replaces conditional jumps with unconditional jumps if the condition is a constant value. """ - for bb in self.dom.dfs_walk: + for bb in self.function.get_basic_blocks(): for inst in bb.instructions: self._replace_constants(inst) @@ -315,33 +313,6 @@ def _replace_constants(self, inst: IRInstruction): if isinstance(lat, IRLiteral): inst.operands[i] = lat - def _fix_phi_nodes(self): - # fix basic blocks whose cfg in was changed - # maybe this should really be done in _visit_phi - for bb in self.fn.get_basic_blocks(): - cfg_in_labels = OrderedSet(in_bb.label for in_bb in bb.cfg_in) - - needs_sort = False - for inst in bb.instructions: - if inst.opcode != "phi": - break - needs_sort |= self._fix_phi_inst(inst, cfg_in_labels) - - # move phi instructions to the top of the block - if needs_sort: - bb.instructions.sort(key=lambda inst: inst.opcode != "phi") - - def _fix_phi_inst(self, inst: IRInstruction, cfg_in_labels: OrderedSet): - operands = [op for label, op in inst.phi_operands if label in cfg_in_labels] - - if len(operands) != 1: - return False - - assert inst.output is not None - inst.opcode = "store" - inst.operands = operands - return True - def _meet(x: LatticeItem, y: LatticeItem) -> LatticeItem: if x == LatticeEnum.TOP: diff --git a/vyper/venom/passes/simplify_cfg.py b/vyper/venom/passes/simplify_cfg.py index acf37376e0..10535c2144 100644 --- a/vyper/venom/passes/simplify_cfg.py +++ b/vyper/venom/passes/simplify_cfg.py @@ -122,16 +122,16 @@ def run_pass(self): for _ in range(fn.num_basic_blocks): changes = self._optimize_empty_basicblocks() + self.analyses_cache.force_analysis(CFGAnalysis) changes += fn.remove_unreachable_blocks() if changes == 0: break else: raise CompilerPanic("Too many iterations removing empty basic blocks") - self.analyses_cache.force_analysis(CFGAnalysis) - for _ in range(fn.num_basic_blocks): # essentially `while True` self._collapse_chained_blocks(entry) + self.analyses_cache.force_analysis(CFGAnalysis) if fn.remove_unreachable_blocks() == 0: break else: diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 264ec35eee..68c8fc3fd7 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -10,7 +10,12 @@ optimize_assembly, ) from vyper.utils import MemoryPositions, OrderedSet -from vyper.venom.analysis import IRAnalysesCache, LivenessAnalysis, VarEquivalenceAnalysis +from vyper.venom.analysis import ( + CFGAnalysis, + IRAnalysesCache, + LivenessAnalysis, + VarEquivalenceAnalysis, +) from vyper.venom.basicblock import ( IRBasicBlock, IRInstruction, @@ -152,6 +157,7 @@ def generate_evm(self, no_optimize: bool = False) -> list[str]: NormalizationPass(ac, fn).run_pass() self.liveness_analysis = ac.request_analysis(LivenessAnalysis) self.equivalence = ac.request_analysis(VarEquivalenceAnalysis) + ac.request_analysis(CFGAnalysis) assert fn.normalized, "Non-normalized CFG!" @@ -308,7 +314,7 @@ def _generate_evm_for_basicblock_r( ref.extend(asm) - for bb in basicblock.reachable: + for bb in basicblock.cfg_out: self._generate_evm_for_basicblock_r(ref, bb, stack.copy()) # pop values from stack at entry to bb