From 00165c8a6b0316619c38a564ed3c54a88efb19e7 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 14 Oct 2024 11:42:45 -0400 Subject: [PATCH 01/20] feat[venom]: improve liveness computation traversing in reverse topsort order improves stack scheduling slightly (codesize improvement of about 0.35%) this commit also adds a topsort method to CFGAnalysis, and speeds it up by only checking the terminator instruction instead of iterating over all the instructions in every basic block. --- vyper/venom/analysis/cfg.py | 36 ++++++++++++++++++++++---------- vyper/venom/analysis/liveness.py | 13 ++++++------ vyper/venom/function.py | 2 ++ 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/vyper/venom/analysis/cfg.py b/vyper/venom/analysis/cfg.py index e4f130bc18..918ca29b09 100644 --- a/vyper/venom/analysis/cfg.py +++ b/vyper/venom/analysis/cfg.py @@ -9,6 +9,8 @@ class CFGAnalysis(IRAnalysis): """ def analyze(self) -> None: + self._topsort = None + fn = self.function for bb in fn.get_basic_blocks(): bb.cfg_in = OrderedSet() @@ -17,19 +19,31 @@ def analyze(self) -> None: 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}" + terminator = bb.instructions[-1] + assert terminator.is_bb_terminator, f"Last instruction should be a terminator {bb}" - 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) + if terminator.opcode in CFG_ALTERING_INSTRUCTIONS: + ops = terminator.get_label_operands() + for op in ops: + next_bb = fn.get_basic_block(op.value) + next_bb.add_cfg_in(bb) + bb.add_cfg_out(next_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) + + def topsort(self): + if self._topsort is None: + self._topsort = OrderedSet() + self._topsort_r(self.function.entry) + + return iter(self._topsort) + + def _topsort_r(self, bb): + if bb in self._topsort: + return + self._topsort.add(bb) + + for next_bb in bb.cfg_out: + self._topsort_r(next_bb) def invalidate(self): from vyper.venom.analysis import DominatorTreeAnalysis, LivenessAnalysis diff --git a/vyper/venom/analysis/liveness.py b/vyper/venom/analysis/liveness.py index b5d65961b7..98b6793214 100644 --- a/vyper/venom/analysis/liveness.py +++ b/vyper/venom/analysis/liveness.py @@ -12,21 +12,22 @@ class LivenessAnalysis(IRAnalysis): """ def analyze(self): - self.analyses_cache.request_analysis(CFGAnalysis) + cfg = self.analyses_cache.request_analysis(CFGAnalysis) self._reset_liveness() - self._worklist = deque() - self._worklist.extend(self.function.get_basic_blocks()) + worklist = deque(reversed(list(cfg.topsort()))) + + while len(worklist) > 0: + bb = worklist.popleft() - while len(self._worklist) > 0: changed = False - bb = self._worklist.popleft() changed |= self._calculate_out_vars(bb) changed |= self._calculate_liveness(bb) + # recompute liveness for basic blocks pointing into # this basic block if changed: - self._worklist.extend(bb.cfg_in) + worklist.extend(bb.cfg_in) def _reset_liveness(self) -> None: for bb in self.function.get_basic_blocks(): diff --git a/vyper/venom/function.py b/vyper/venom/function.py index fb0dabc99a..36295f3d2b 100644 --- a/vyper/venom/function.py +++ b/vyper/venom/function.py @@ -141,6 +141,8 @@ def _compute_reachability_from(self, bb: IRBasicBlock) -> None: if inst.opcode in CFG_ALTERING_INSTRUCTIONS: for op in inst.get_label_operands(): out_bb = self.get_basic_block(op.value) + # note: bb.reachable is "shallow" reachability, not + # "deep" reachability -- it is equivalent to cfg_out. bb.reachable.add(out_bb) self._compute_reachability_from(out_bb) From 8619371e6a9565dc5758e4e5562b02903ee48bc8 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 14 Oct 2024 11:47:45 -0400 Subject: [PATCH 02/20] fix lint --- vyper/venom/analysis/cfg.py | 1 - 1 file changed, 1 deletion(-) diff --git a/vyper/venom/analysis/cfg.py b/vyper/venom/analysis/cfg.py index 918ca29b09..bbfc228799 100644 --- a/vyper/venom/analysis/cfg.py +++ b/vyper/venom/analysis/cfg.py @@ -29,7 +29,6 @@ def analyze(self) -> None: next_bb.add_cfg_in(bb) bb.add_cfg_out(next_bb) - def topsort(self): if self._topsort is None: self._topsort = OrderedSet() From 3f8ec649bfb30ca80ec37075180b96cae8d88484 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 25 Oct 2024 13:27:28 -0400 Subject: [PATCH 03/20] remove dom tree from sccp pass --- vyper/venom/passes/sccp/sccp.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index d85e09c9b4..b547fc76be 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -50,7 +50,6 @@ class SCCP(IRPass): """ fn: IRFunction - dom: DominatorTreeAnalysis dfg: DFGAnalysis lattice: Lattice work_list: list[WorkListItem] @@ -66,7 +65,6 @@ 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() @@ -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) From 24f90a6d8670e491074734e7003e080071dd71e1 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 25 Oct 2024 13:45:25 -0400 Subject: [PATCH 04/20] add dfs to cfg analysis. refactor out of dom tree analysis --- vyper/venom/analysis/cfg.py | 36 ++++++++++++++++++++++++++---- vyper/venom/analysis/dominators.py | 29 +++++++++--------------- vyper/venom/passes/sccp/sccp.py | 2 +- 3 files changed, 43 insertions(+), 24 deletions(-) diff --git a/vyper/venom/analysis/cfg.py b/vyper/venom/analysis/cfg.py index 90b18b353c..2a5142954f 100644 --- a/vyper/venom/analysis/cfg.py +++ b/vyper/venom/analysis/cfg.py @@ -1,6 +1,8 @@ +from typing import Iterator, Optional + 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,6 +10,9 @@ class CFGAnalysis(IRAnalysis): Compute control flow graph information for each basic block in the function. """ + _dfs: Optional[list[IRBasicBlock]] = None + _topsort: Optional[OrderedSet[IRBasicBlock]] = None + def analyze(self) -> None: fn = self.function for bb in fn.get_basic_blocks(): @@ -16,9 +21,7 @@ def analyze(self) -> None: bb.out_vars = OrderedSet() 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: @@ -31,11 +34,36 @@ def analyze(self) -> None: for in_bb in bb.cfg_in: in_bb.add_cfg_out(bb) + def _compute_dfs_r(self, bb=None): + self._dfs = [] + + if bb is None: + bb = self.function.entry + + self._dfs.append(bb) + for out_bb in bb.cfg_out: + self._compute_dfs_r(out_bb) + + def dfs(self) -> Iterator[IRBasicBlock]: + if self._dfs is None: + self._compute_dfs_r() + + assert self._dfs is not None # help mypy + return iter(self._dfs) + + def topsort(self) -> Iterator[IRBasicBlock]: + if self._topsort is None: + self._topsort = OrderedSet(self.dfs()) + return iter(self._topsort) + 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 + self._topsort = 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..4dcce4d327 100644 --- a/vyper/venom/analysis/dominators.py +++ b/vyper/venom/analysis/dominators.py @@ -1,3 +1,6 @@ +from functools import cached_property +from typing import Iterator + from vyper.exceptions import CompilerPanic from vyper.utils import OrderedSet from vyper.venom.analysis import CFGAnalysis, IRAnalysis @@ -14,8 +17,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,14 +28,12 @@ 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() @@ -131,21 +130,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) -> Iterator[IRBasicBlock]: + return 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/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index b547fc76be..f82a605e2d 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, From 9b8d3f392d7f408a7311048ff25044dfc1a000ca Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 25 Oct 2024 13:56:34 -0400 Subject: [PATCH 05/20] fix from bad merge --- vyper/venom/analysis/cfg.py | 23 +++++++++-------------- vyper/venom/analysis/dominators.py | 5 ++--- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/vyper/venom/analysis/cfg.py b/vyper/venom/analysis/cfg.py index 2a5142954f..0f5db14959 100644 --- a/vyper/venom/analysis/cfg.py +++ b/vyper/venom/analysis/cfg.py @@ -11,7 +11,6 @@ class CFGAnalysis(IRAnalysis): """ _dfs: Optional[list[IRBasicBlock]] = None - _topsort: Optional[OrderedSet[IRBasicBlock]] = None def analyze(self) -> None: fn = self.function @@ -34,28 +33,24 @@ def analyze(self) -> None: for in_bb in bb.cfg_in: in_bb.add_cfg_out(bb) - def _compute_dfs_r(self, bb=None): - self._dfs = [] + def _compute_dfs_r(self, bb): + assert self._dfs is not None # help mypy - if bb is None: - bb = self.function.entry + if bb in self._dfs: + return - self._dfs.append(bb) + self._dfs.add(bb) for out_bb in bb.cfg_out: self._compute_dfs_r(out_bb) - def dfs(self) -> Iterator[IRBasicBlock]: + @property + def dfs_walk(self) -> Iterator[IRBasicBlock]: if self._dfs is None: - self._compute_dfs_r() + self._dfs = OrderedSet() + self._compute_dfs_r(self.function.entry) - assert self._dfs is not None # help mypy return iter(self._dfs) - def topsort(self) -> Iterator[IRBasicBlock]: - if self._topsort is None: - self._topsort = OrderedSet(self.dfs()) - return iter(self._topsort) - def invalidate(self): from vyper.venom.analysis import DFGAnalysis, DominatorTreeAnalysis, LivenessAnalysis diff --git a/vyper/venom/analysis/dominators.py b/vyper/venom/analysis/dominators.py index 4dcce4d327..bace223649 100644 --- a/vyper/venom/analysis/dominators.py +++ b/vyper/venom/analysis/dominators.py @@ -35,7 +35,6 @@ def analyze(self): 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,8 +130,8 @@ def _intersect(self, bb1, bb2): return bb1 @cached_property - def dfs_walk(self) -> Iterator[IRBasicBlock]: - return self.cfg.dfs_walk + def dfs_walk(self) -> list[IRBasicBlock]: + return list(self.cfg.dfs_walk) @cached_property def dfs_order(self) -> dict[IRBasicBlock, int]: From d1951a0b850cbb7f76a07991fbbb086c078ba655 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 25 Oct 2024 14:01:55 -0400 Subject: [PATCH 06/20] fix dfs search - leaves first --- vyper/venom/analysis/cfg.py | 17 +++++++++++------ vyper/venom/analysis/dominators.py | 1 - 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/vyper/venom/analysis/cfg.py b/vyper/venom/analysis/cfg.py index 0f5db14959..dd8b522b88 100644 --- a/vyper/venom/analysis/cfg.py +++ b/vyper/venom/analysis/cfg.py @@ -10,7 +10,7 @@ class CFGAnalysis(IRAnalysis): Compute control flow graph information for each basic block in the function. """ - _dfs: Optional[list[IRBasicBlock]] = None + _dfs: Optional[OrderedSet[IRBasicBlock]] = None def analyze(self) -> None: fn = self.function @@ -33,15 +33,20 @@ def analyze(self) -> None: for in_bb in bb.cfg_in: in_bb.add_cfg_out(bb) - def _compute_dfs_r(self, bb): + def _compute_dfs_r(self, bb, visited=None): assert self._dfs is not None # help mypy + if visited is None: + visited = OrderedSet() - if bb in self._dfs: + if bb in visited: return - self._dfs.add(bb) + visited.add(bb) + for out_bb in bb.cfg_out: - self._compute_dfs_r(out_bb) + self._compute_dfs_r(out_bb, visited) + + self._dfs.add(bb) @property def dfs_walk(self) -> Iterator[IRBasicBlock]: @@ -49,6 +54,7 @@ def dfs_walk(self) -> Iterator[IRBasicBlock]: self._dfs = OrderedSet() self._compute_dfs_r(self.function.entry) + assert self._dfs is not None # help mypy return iter(self._dfs) def invalidate(self): @@ -58,7 +64,6 @@ def invalidate(self): self.analyses_cache.invalidate_analysis(LivenessAnalysis) self._dfs = None - self._topsort = 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 bace223649..b60f9bdab9 100644 --- a/vyper/venom/analysis/dominators.py +++ b/vyper/venom/analysis/dominators.py @@ -1,5 +1,4 @@ from functools import cached_property -from typing import Iterator from vyper.exceptions import CompilerPanic from vyper.utils import OrderedSet From d1e18d6ddb858d6afa38fc4f1c24aaf49108c8a1 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 26 Oct 2024 17:39:21 -0400 Subject: [PATCH 07/20] simplify cfg calculation --- vyper/venom/analysis/cfg.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/vyper/venom/analysis/cfg.py b/vyper/venom/analysis/cfg.py index dd8b522b88..c8581d8818 100644 --- a/vyper/venom/analysis/cfg.py +++ b/vyper/venom/analysis/cfg.py @@ -22,16 +22,13 @@ def analyze(self) -> None: for bb in fn.get_basic_blocks(): 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) - - # 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) + term = bb.instructions[-1] + if term.opcode in CFG_ALTERING_INSTRUCTIONS: + ops = term.get_label_operands() + for op in ops: + next_bb = fn.get_basic_block(op.value) + next_bb.add_cfg_in(bb) + bb.add_cfg_out(next_bb) def _compute_dfs_r(self, bb, visited=None): assert self._dfs is not None # help mypy From 30fa6334d7558510e19076c043b8e0dffaf136b4 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 26 Oct 2024 17:52:58 -0400 Subject: [PATCH 08/20] fix a regression --- vyper/venom/analysis/cfg.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/vyper/venom/analysis/cfg.py b/vyper/venom/analysis/cfg.py index c8581d8818..b021293a77 100644 --- a/vyper/venom/analysis/cfg.py +++ b/vyper/venom/analysis/cfg.py @@ -28,7 +28,11 @@ def analyze(self) -> None: for op in ops: next_bb = fn.get_basic_block(op.value) next_bb.add_cfg_in(bb) - bb.add_cfg_out(next_bb) + + for bb in fn.get_basic_blocks(): + for in_bb in bb.cfg_in: + # order here matters to performance + in_bb.add_cfg_out(bb) def _compute_dfs_r(self, bb, visited=None): assert self._dfs is not None # help mypy From 67d13829ff2731ecb87933f379b0990e040f698e Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 26 Oct 2024 18:06:59 -0400 Subject: [PATCH 09/20] refactor: remove IRFunction.compute_reachability it is redundant with CFGAnalysis cfg_out --- vyper/venom/analysis/cfg.py | 27 +++++++++++---------------- vyper/venom/function.py | 29 ++--------------------------- vyper/venom/passes/simplify_cfg.py | 4 ++-- vyper/venom/venom_to_assembly.py | 10 ++++++++-- 4 files changed, 23 insertions(+), 47 deletions(-) diff --git a/vyper/venom/analysis/cfg.py b/vyper/venom/analysis/cfg.py index b021293a77..72cf5aa569 100644 --- a/vyper/venom/analysis/cfg.py +++ b/vyper/venom/analysis/cfg.py @@ -1,4 +1,4 @@ -from typing import Iterator, Optional +from typing import Iterator from vyper.utils import OrderedSet from vyper.venom.analysis import IRAnalysis @@ -10,14 +10,17 @@ class CFGAnalysis(IRAnalysis): Compute control flow graph information for each basic block in the function. """ - _dfs: Optional[OrderedSet[IRBasicBlock]] = None + _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 bb.is_terminated @@ -31,31 +34,23 @@ def analyze(self) -> None: for bb in fn.get_basic_blocks(): for in_bb in bb.cfg_in: - # order here matters to performance + # order here matters to performance! in_bb.add_cfg_out(bb) - def _compute_dfs_r(self, bb, visited=None): - assert self._dfs is not None # help mypy - if visited is None: - visited = OrderedSet() + self._compute_dfs_r(self.function.entry) - if bb in visited: + def _compute_dfs_r(self, bb): + if bb.is_reachable: return - - visited.add(bb) + bb.is_reachable = True for out_bb in bb.cfg_out: - self._compute_dfs_r(out_bb, visited) + self._compute_dfs_r(out_bb) self._dfs.add(bb) @property def dfs_walk(self) -> Iterator[IRBasicBlock]: - if self._dfs is None: - self._dfs = OrderedSet() - self._compute_dfs_r(self.function.entry) - - assert self._dfs is not None # help mypy return iter(self._dfs) def invalidate(self): diff --git a/vyper/venom/function.py b/vyper/venom/function.py index fb0dabc99a..8bb5d9ecf8 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,7 +88,7 @@ def get_last_variable(self) -> str: return f"%{self.last_variable}" def remove_unreachable_blocks(self) -> int: - self._compute_reachability() + # pre: requires CFG analysis! removed = [] @@ -120,30 +119,6 @@ def remove_unreachable_blocks(self) -> int: 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 - - self._compute_reachability_from(self.entry) - - 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) - @property def normalized(self) -> bool: """ 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 From a06d546f85919f509357a79c3c23bcd314b43942 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 26 Oct 2024 18:11:04 -0400 Subject: [PATCH 10/20] change cfg_out order this shaves 1.5% off bytecode size --- vyper/venom/analysis/cfg.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/vyper/venom/analysis/cfg.py b/vyper/venom/analysis/cfg.py index 72cf5aa569..700fd73f26 100644 --- a/vyper/venom/analysis/cfg.py +++ b/vyper/venom/analysis/cfg.py @@ -28,15 +28,12 @@ def analyze(self) -> None: term = bb.instructions[-1] if term.opcode in CFG_ALTERING_INSTRUCTIONS: ops = term.get_label_operands() - for op in ops: + # 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) - for bb in fn.get_basic_blocks(): - for in_bb in bb.cfg_in: - # order here matters to performance! - in_bb.add_cfg_out(bb) - self._compute_dfs_r(self.function.entry) def _compute_dfs_r(self, bb): From f1984775052bc706464ac6f8af99a21ad3ac5060 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 26 Oct 2024 18:15:30 -0400 Subject: [PATCH 11/20] add a note --- vyper/venom/function.py | 1 + 1 file changed, 1 insertion(+) diff --git a/vyper/venom/function.py b/vyper/venom/function.py index 8bb5d9ecf8..a01dafaa68 100644 --- a/vyper/venom/function.py +++ b/vyper/venom/function.py @@ -89,6 +89,7 @@ def get_last_variable(self) -> str: def remove_unreachable_blocks(self) -> int: # pre: requires CFG analysis! + # NOTE: should this be a pass? removed = [] From 452c82ebf0c0b4b47bb349971b99a52d18e588f6 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sun, 27 Oct 2024 02:22:22 +0000 Subject: [PATCH 12/20] some StackTooDeep are fixed --- tests/functional/codegen/features/test_constructor.py | 1 - tests/functional/codegen/types/test_dynamic_array.py | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/functional/codegen/features/test_constructor.py b/tests/functional/codegen/features/test_constructor.py index 3b86fe3460..2482681865 100644 --- a/tests/functional/codegen/features/test_constructor.py +++ b/tests/functional/codegen/features/test_constructor.py @@ -169,7 +169,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..69c6cf0ac8 100644 --- a/tests/functional/codegen/types/test_dynamic_array.py +++ b/tests/functional/codegen/types/test_dynamic_array.py @@ -737,7 +737,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] From ee500885cede5fdc07b1d78265ddb9edb3a645c5 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 26 Oct 2024 22:19:10 -0400 Subject: [PATCH 13/20] fix sccp tests --- tests/unit/compiler/venom/test_sccp.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) 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 From d3967f5e0172f7ccf608b215d4ead478ad4f8abf Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 26 Oct 2024 22:33:12 -0400 Subject: [PATCH 14/20] fix lint --- tests/functional/codegen/features/test_constructor.py | 3 --- tests/functional/codegen/types/test_dynamic_array.py | 1 - 2 files changed, 4 deletions(-) diff --git a/tests/functional/codegen/features/test_constructor.py b/tests/functional/codegen/features/test_constructor.py index 2482681865..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 diff --git a/tests/functional/codegen/types/test_dynamic_array.py b/tests/functional/codegen/types/test_dynamic_array.py index 69c6cf0ac8..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, ) From 240d9d28a5e3ffc63b316483bd7baf5d33c792bd Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 26 Oct 2024 22:40:48 -0400 Subject: [PATCH 15/20] use update instead of union --- vyper/venom/analysis/liveness.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From 78da77cc84e00ac9833e769e3c905a88958e54ae Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 26 Oct 2024 22:49:38 -0400 Subject: [PATCH 16/20] skip unreachable bbs --- vyper/venom/function.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/vyper/venom/function.py b/vyper/venom/function.py index a01dafaa68..54a76c6d08 100644 --- a/vyper/venom/function.py +++ b/vyper/venom/function.py @@ -104,19 +104,26 @@ def remove_unreachable_blocks(self) -> int: # Remove phi instructions that reference removed basic blocks for bb in removed: for out_bb in bb.cfg_out: + if not out_bb.is_reachable: + continue + 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) + inst.output = None + inst.opcode = "nop" + inst.operands = [] return len(removed) From 2fe13975b0d992f2b4daf22942edf1963ab58d79 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 26 Oct 2024 23:15:12 -0400 Subject: [PATCH 17/20] refactor fix_phi_nodes shared code between remove_unreachable_blocks and sccp --- vyper/venom/basicblock.py | 26 ++++++++++++++++++++++++ vyper/venom/function.py | 35 +++++++++++---------------------- vyper/venom/passes/sccp/sccp.py | 29 +-------------------------- 3 files changed, 39 insertions(+), 51 deletions(-) diff --git a/vyper/venom/basicblock.py b/vyper/venom/basicblock.py index eb4f7e67ba..c556f3ecfa 100644 --- a/vyper/venom/basicblock.py +++ b/vyper/venom/basicblock.py @@ -495,6 +495,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 54a76c6d08..cab78ac477 100644 --- a/vyper/venom/function.py +++ b/vyper/venom/function.py @@ -88,42 +88,31 @@ def get_last_variable(self) -> str: return f"%{self.last_variable}" def remove_unreachable_blocks(self) -> int: + # 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: - if not out_bb.is_reachable: + for bb in self.get_basic_blocks(): + modified = False + for in_bb in list(bb.cfg_in): + if in_bb not in removed: continue - 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: - inst.output = None - inst.opcode = "nop" - inst.operands = [] + modified = True + bb.remove_cfg_in(in_bb) + + if modified or True: + bb.fix_phi_instructions() return len(removed) diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index f82a605e2d..2bdd0ace44 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -71,7 +71,7 @@ def run_pass(self): 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) @@ -313,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: From 27518a502ad2880b000a93288aa3eea311d15aa4 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 26 Oct 2024 23:52:06 -0400 Subject: [PATCH 18/20] remove useless __eq__ and __hash__ implementations --- vyper/venom/basicblock.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/vyper/venom/basicblock.py b/vyper/venom/basicblock.py index c556f3ecfa..bac0538940 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: """ From 1b117c8e9f51701501fe0ba277d063d4bf4a4a69 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 28 Oct 2024 20:29:34 -0400 Subject: [PATCH 19/20] remove `IRBasicBlock.reachable` --- vyper/venom/basicblock.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/vyper/venom/basicblock.py b/vyper/venom/basicblock.py index bac0538940..f73f847a62 100644 --- a/vyper/venom/basicblock.py +++ b/vyper/venom/basicblock.py @@ -384,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: @@ -395,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: From 3863ed2da0c9ae4e9de062dedac7ace1d1485044 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 29 Oct 2024 09:38:01 -0400 Subject: [PATCH 20/20] fix from merge --- vyper/venom/analysis/liveness.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/venom/analysis/liveness.py b/vyper/venom/analysis/liveness.py index 63d187316e..fb845dc198 100644 --- a/vyper/venom/analysis/liveness.py +++ b/vyper/venom/analysis/liveness.py @@ -15,7 +15,7 @@ def analyze(self): cfg = self.analyses_cache.request_analysis(CFGAnalysis) self._reset_liveness() - worklist = deque(reversed(list(cfg.topsort()))) + worklist = deque(cfg.dfs_walk) while len(worklist) > 0: bb = worklist.popleft()