From 13944eb012cf250acefd10f618bbc520e0a6d41e Mon Sep 17 00:00:00 2001 From: Hodan Date: Wed, 10 Jul 2024 17:04:08 +0200 Subject: [PATCH 01/17] feat[venom]: add loop invariant hoisting pass --- vyper/venom/analysis/loop_detection.py | 45 +++++++++++ vyper/venom/passes/loop_invariant_hosting.py | 81 ++++++++++++++++++++ 2 files changed, 126 insertions(+) create mode 100644 vyper/venom/analysis/loop_detection.py create mode 100644 vyper/venom/passes/loop_invariant_hosting.py diff --git a/vyper/venom/analysis/loop_detection.py b/vyper/venom/analysis/loop_detection.py new file mode 100644 index 0000000000..086920d7da --- /dev/null +++ b/vyper/venom/analysis/loop_detection.py @@ -0,0 +1,45 @@ +from vyper.venom.analysis.analysis import IRAnalysis +from vyper.venom.analysis.cfg import CFGAnalysis +from vyper.venom.basicblock import IRBasicBlock + + +class LoopDetection(IRAnalysis): + """ + Detects loops and computes basic blocks sets + which comprised these loops + """ + + # key = start of the loop (last bb not in the loop), value all the block that loop contains + loops: dict[IRBasicBlock, list[IRBasicBlock]] + + def analyze(self): + self.analyses_cache.request_analysis(CFGAnalysis) + self.loops: dict[IRBasicBlock, list[IRBasicBlock]] = dict() + done: dict[IRBasicBlock, bool] = dict( + (bb, False) for bb in self.function.get_basic_blocks() + ) + entry = self.function.entry + self.dfs(entry, done, []) + + def invalidate(self): + return super().invalidate() + + def dfs(self, bb: IRBasicBlock, done: dict[IRBasicBlock, bool], path: list[IRBasicBlock]): + if bb in path: + index = path.index(bb) + assert index >= 1, "Loop must have one basic block before it" + assert ( + path[index - 1] not in self.loops.keys() + ), "From one basic block can start only one loop" + done[bb] = True + self.loops[path[index - 1]] = path[index:].copy() + return + + path.append(bb) + for neighbour in bb.cfg_out: + if not done[neighbour]: + self.dfs(neighbour, done, path) + path.pop() + + done[bb] = True + return diff --git a/vyper/venom/passes/loop_invariant_hosting.py b/vyper/venom/passes/loop_invariant_hosting.py new file mode 100644 index 0000000000..1c043eaf1c --- /dev/null +++ b/vyper/venom/passes/loop_invariant_hosting.py @@ -0,0 +1,81 @@ +from vyper.utils import OrderedSet +from vyper.venom.analysis.cfg import CFGAnalysis +from vyper.venom.analysis.dfg import DFGAnalysis +from vyper.venom.analysis.liveness import LivenessAnalysis +from vyper.venom.analysis.loop_detection import LoopDetection +from vyper.venom.basicblock import ( + BB_TERMINATORS, + CFG_ALTERING_INSTRUCTIONS, + VOLATILE_INSTRUCTIONS, + IRBasicBlock, + IRInstruction, + IRVariable, +) +from vyper.venom.function import IRFunction +from vyper.venom.passes.base_pass import IRPass + + +class LoopInvariantHoisting(IRPass): + from typing import Iterator + + function: IRFunction + loops: dict[IRBasicBlock, list[IRBasicBlock]] + dfg : DFGAnalysis + + def run_pass(self): + self.analyses_cache.request_analysis(CFGAnalysis) + loops = self.analyses_cache.request_analysis(LoopDetection) + self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) + self.loops = loops.loops + while True: + hoistable: list[tuple[IRBasicBlock, int, IRInstruction]] = self._get_hoistable() + if len(hoistable) == 0: + break + self._hoist(hoistable) + # I have this inside the loop because I dont need to + # invalidate if you dont hoist anything + self.analyses_cache.invalidate_analysis(LivenessAnalysis) + + def _hoist(self, hoistable: list[tuple[IRBasicBlock, int, IRInstruction]]): + for loop_idx, bb_idx, inst in hoistable: + loop = self.loops[loop_idx] + loop[bb_idx].remove_instruction(inst) + bb_before: IRBasicBlock = loop_idx + bb_before.insert_instruction(inst, index=len(bb_before.instructions) - 1) + + # returns touble of index of loop and instruction + def _get_hoistable(self) -> list[tuple[IRBasicBlock, int, IRInstruction]]: + result: list[tuple[IRBasicBlock, int, IRInstruction]] = [] + for from_bb, loop in self.loops.items(): + for bb_idx, bb in enumerate(loop): + result.extend(self._get_hoistable_bb(bb, from_bb, bb_idx)) + return result + + def _get_hoistable_bb( + self, bb: IRBasicBlock, loop_idx: IRBasicBlock, bb_idx + ) -> list[tuple[IRBasicBlock, int, IRInstruction]]: + result: list[tuple[IRBasicBlock, int, IRInstruction]] = [] + for instruction in bb.instructions: + if self._can_hoist_instruction(instruction, self.loops[loop_idx]): + result.append((loop_idx, bb_idx, instruction)) + + return result + + def _can_hoist_instruction(self, instruction: IRInstruction, loop: list[IRBasicBlock]) -> bool: + if ( + instruction.opcode in VOLATILE_INSTRUCTIONS + or instruction.opcode in BB_TERMINATORS + or instruction.opcode in CFG_ALTERING_INSTRUCTIONS + ): + return False + for bb in loop: + if self._in_bb(instruction, bb): + return False + return True + + def _in_bb(self, instruction: IRInstruction, bb: IRBasicBlock): + for in_var in instruction.get_input_variables(): + source_ins = self.dfg._dfg_outputs[in_var] + if source_ins in bb.instructions: + return True + return False From f626b6828f6bfbca5db3a66748c7a923d5bec3f7 Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 12 Jul 2024 14:48:52 +0200 Subject: [PATCH 02/17] feat[venom]: add unit test for loop invariant hoisting --- .../venom/test_loop_invariant_hoisting.py | 118 ++++++++++++++++++ vyper/venom/passes/loop_invariant_hosting.py | 22 ++-- 2 files changed, 131 insertions(+), 9 deletions(-) create mode 100644 tests/unit/compiler/venom/test_loop_invariant_hoisting.py diff --git a/tests/unit/compiler/venom/test_loop_invariant_hoisting.py b/tests/unit/compiler/venom/test_loop_invariant_hoisting.py new file mode 100644 index 0000000000..951c9bf993 --- /dev/null +++ b/tests/unit/compiler/venom/test_loop_invariant_hoisting.py @@ -0,0 +1,118 @@ +import pytest + +from vyper.venom.analysis.analysis import IRAnalysesCache +from vyper.venom.basicblock import IRBasicBlock, IRLabel, IRVariable +from vyper.venom.function import IRFunction +from vyper.venom.context import IRContext +from vyper.venom.analysis.loop_detection import LoopDetection +from vyper.venom.passes.loop_invariant_hosting import LoopInvariantHoisting + + +def _create_loops(fn, depth, loop_id, body_fn = lambda _ : (), top = True): + bb = fn.get_basic_block() + cond = IRBasicBlock(IRLabel(f"cond{loop_id}{depth}"), fn) + body = IRBasicBlock(IRLabel(f"body{loop_id}{depth}"), fn) + if top: + exit_block = IRBasicBlock(IRLabel(f"exit_top{loop_id}{depth}"), fn) + else: + exit_block = IRBasicBlock(IRLabel(f"exit{loop_id}{depth}"), fn) + fn.append_basic_block(cond) + fn.append_basic_block(body) + + bb.append_instruction("jmp", cond.label) + + cond_var = IRVariable(f"cond_var{loop_id}{depth}") + cond.append_instruction("iszero", 0, ret=cond_var) + assert isinstance(cond_var, IRVariable) + cond.append_instruction("jnz", cond_var, body.label, exit_block.label) + body_fn(fn, loop_id, depth) + if depth > 1: + _create_loops(fn, depth - 1, loop_id, body_fn, top=False) + bb = fn.get_basic_block() + bb.append_instruction("jmp", cond.label) + fn.append_basic_block(exit_block) + +def _simple_body(fn, loop_id, depth): + assert isinstance(fn, IRFunction) + bb = fn.get_basic_block() + add_var = IRVariable(f"add_var{loop_id}{depth}") + bb.append_instruction("add", 1, 2, ret=add_var) + +def _hoistable_body(fn, loop_id, depth): + assert isinstance(fn, IRFunction) + bb = fn.get_basic_block() + add_var_a = IRVariable(f"add_var_a{loop_id}{depth}") + bb.append_instruction("add", 1, 2, ret=add_var_a) + add_var_b = IRVariable(f"add_var_b{loop_id}{depth}") + bb.append_instruction("add", add_var_a, 2, ret=add_var_b) + +@pytest.mark.parametrize("depth", range(1, 5)) +@pytest.mark.parametrize("count", range(1, 5)) +def test_loop_detection_analysis(depth, count): + ctx = IRContext() + fn = ctx.create_function("_global") + + for c in range(count): + _create_loops(fn, depth, c, _simple_body) + + bb = fn.get_basic_block() + bb.append_instruction("ret") + + ac = IRAnalysesCache(fn) + analysis = ac.request_analysis(LoopDetection) + assert len(analysis.loops) == depth * count + +@pytest.mark.parametrize("depth", range(1, 5)) +@pytest.mark.parametrize("count", range(1, 5)) +def test_loop_invariant_hoisting_simple(depth, count): + ctx = IRContext() + fn = ctx.create_function("_global") + + for c in range(count): + _create_loops(fn, depth, c, _simple_body) + + bb = fn.get_basic_block() + bb.append_instruction("ret") + + ac = IRAnalysesCache(fn) + LoopInvariantHoisting(ac, fn).run_pass() + + entry = fn.entry + assignments = list(map(lambda x : x.value, entry.get_assignments())) + for bb in filter(lambda bb: bb.label.name.startswith("exit_top"), fn.get_basic_blocks()): + assignments.extend(map(lambda x : x.value, bb.get_assignments())) + + assert len(assignments) == depth * count * 2 + for loop_id in range(count): + for d in range(1, depth + 1): + assert f"%add_var{loop_id}{d}" in assignments, repr(fn) + assert f"%cond_var{loop_id}{d}" in assignments, repr(fn) + + +@pytest.mark.parametrize("depth", range(1, 5)) +@pytest.mark.parametrize("count", range(1, 5)) +def test_loop_invariant_hoisting_dependant(depth, count): + ctx = IRContext() + fn = ctx.create_function("_global") + + for c in range(count): + _create_loops(fn, depth, c, _hoistable_body) + + bb = fn.get_basic_block() + bb.append_instruction("ret") + + ac = IRAnalysesCache(fn) + LoopInvariantHoisting(ac, fn).run_pass() + + entry = fn.entry + assignments = list(map(lambda x : x.value, entry.get_assignments())) + for bb in filter(lambda bb: bb.label.name.startswith("exit_top"), fn.get_basic_blocks()): + assignments.extend(map(lambda x : x.value, bb.get_assignments())) + + assert len(assignments) == depth * count * 3 + for loop_id in range(count): + for d in range(1, depth + 1): + assert f"%add_var_a{loop_id}{d}" in assignments, repr(fn) + assert f"%add_var_b{loop_id}{d}" in assignments, repr(fn) + assert f"%cond_var{loop_id}{d}" in assignments, repr(fn) + diff --git a/vyper/venom/passes/loop_invariant_hosting.py b/vyper/venom/passes/loop_invariant_hosting.py index 1c043eaf1c..33b9f471b5 100644 --- a/vyper/venom/passes/loop_invariant_hosting.py +++ b/vyper/venom/passes/loop_invariant_hosting.py @@ -24,14 +24,19 @@ class LoopInvariantHoisting(IRPass): def run_pass(self): self.analyses_cache.request_analysis(CFGAnalysis) - loops = self.analyses_cache.request_analysis(LoopDetection) self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) + loops = self.analyses_cache.request_analysis(LoopDetection) self.loops = loops.loops while True: - hoistable: list[tuple[IRBasicBlock, int, IRInstruction]] = self._get_hoistable() - if len(hoistable) == 0: + change = False + for from_bb, loop in self.loops.items(): + hoistable: list[tuple[IRBasicBlock, int, IRInstruction]] = self._get_hoistable_loop(from_bb, loop) + if len(hoistable) == 0: + continue + change |= True + self._hoist(hoistable) + if not change: break - self._hoist(hoistable) # I have this inside the loop because I dont need to # invalidate if you dont hoist anything self.analyses_cache.invalidate_analysis(LivenessAnalysis) @@ -43,12 +48,10 @@ def _hoist(self, hoistable: list[tuple[IRBasicBlock, int, IRInstruction]]): bb_before: IRBasicBlock = loop_idx bb_before.insert_instruction(inst, index=len(bb_before.instructions) - 1) - # returns touble of index of loop and instruction - def _get_hoistable(self) -> list[tuple[IRBasicBlock, int, IRInstruction]]: + def _get_hoistable_loop(self, from_bb : IRBasicBlock, loop : list[IRBasicBlock]) -> list[tuple[IRBasicBlock, int, IRInstruction]]: result: list[tuple[IRBasicBlock, int, IRInstruction]] = [] - for from_bb, loop in self.loops.items(): - for bb_idx, bb in enumerate(loop): - result.extend(self._get_hoistable_bb(bb, from_bb, bb_idx)) + for bb_idx, bb in enumerate(loop): + result.extend(self._get_hoistable_bb(bb, from_bb, bb_idx)) return result def _get_hoistable_bb( @@ -75,6 +78,7 @@ def _can_hoist_instruction(self, instruction: IRInstruction, loop: list[IRBasicB def _in_bb(self, instruction: IRInstruction, bb: IRBasicBlock): for in_var in instruction.get_input_variables(): + assert isinstance(in_var, IRVariable) source_ins = self.dfg._dfg_outputs[in_var] if source_ins in bb.instructions: return True From 942c7311a650a5bc12711bd544a5c63a712f7493 Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 12 Jul 2024 16:52:14 +0200 Subject: [PATCH 03/17] feat[venom]: add unit test for loop invariant hoisting --- .../venom/test_loop_invariant_hoisting.py | 75 ++++++++++++++----- vyper/venom/__init__.py | 2 + vyper/venom/analysis/loop_detection.py | 15 ++-- vyper/venom/passes/loop_invariant_hosting.py | 17 +++-- 4 files changed, 75 insertions(+), 34 deletions(-) diff --git a/tests/unit/compiler/venom/test_loop_invariant_hoisting.py b/tests/unit/compiler/venom/test_loop_invariant_hoisting.py index 951c9bf993..4a3e6afcf3 100644 --- a/tests/unit/compiler/venom/test_loop_invariant_hoisting.py +++ b/tests/unit/compiler/venom/test_loop_invariant_hoisting.py @@ -1,14 +1,14 @@ import pytest from vyper.venom.analysis.analysis import IRAnalysesCache +from vyper.venom.analysis.loop_detection import LoopDetectionAnalysis from vyper.venom.basicblock import IRBasicBlock, IRLabel, IRVariable -from vyper.venom.function import IRFunction from vyper.venom.context import IRContext -from vyper.venom.analysis.loop_detection import LoopDetection +from vyper.venom.function import IRFunction from vyper.venom.passes.loop_invariant_hosting import LoopInvariantHoisting -def _create_loops(fn, depth, loop_id, body_fn = lambda _ : (), top = True): +def _create_loops(fn, depth, loop_id, body_fn=lambda _: (), top=True): bb = fn.get_basic_block() cond = IRBasicBlock(IRLabel(f"cond{loop_id}{depth}"), fn) body = IRBasicBlock(IRLabel(f"body{loop_id}{depth}"), fn) @@ -20,7 +20,7 @@ def _create_loops(fn, depth, loop_id, body_fn = lambda _ : (), top = True): fn.append_basic_block(body) bb.append_instruction("jmp", cond.label) - + cond_var = IRVariable(f"cond_var{loop_id}{depth}") cond.append_instruction("iszero", 0, ret=cond_var) assert isinstance(cond_var, IRVariable) @@ -32,12 +32,14 @@ def _create_loops(fn, depth, loop_id, body_fn = lambda _ : (), top = True): bb.append_instruction("jmp", cond.label) fn.append_basic_block(exit_block) + def _simple_body(fn, loop_id, depth): assert isinstance(fn, IRFunction) bb = fn.get_basic_block() add_var = IRVariable(f"add_var{loop_id}{depth}") bb.append_instruction("add", 1, 2, ret=add_var) + def _hoistable_body(fn, loop_id, depth): assert isinstance(fn, IRFunction) bb = fn.get_basic_block() @@ -46,8 +48,9 @@ def _hoistable_body(fn, loop_id, depth): add_var_b = IRVariable(f"add_var_b{loop_id}{depth}") bb.append_instruction("add", add_var_a, 2, ret=add_var_b) -@pytest.mark.parametrize("depth", range(1, 5)) -@pytest.mark.parametrize("count", range(1, 5)) + +@pytest.mark.parametrize("depth", range(1, 4)) +@pytest.mark.parametrize("count", range(1, 4)) def test_loop_detection_analysis(depth, count): ctx = IRContext() fn = ctx.create_function("_global") @@ -57,13 +60,14 @@ def test_loop_detection_analysis(depth, count): bb = fn.get_basic_block() bb.append_instruction("ret") - + ac = IRAnalysesCache(fn) - analysis = ac.request_analysis(LoopDetection) + analysis = ac.request_analysis(LoopDetectionAnalysis) assert len(analysis.loops) == depth * count - -@pytest.mark.parametrize("depth", range(1, 5)) -@pytest.mark.parametrize("count", range(1, 5)) + + +@pytest.mark.parametrize("depth", range(1, 4)) +@pytest.mark.parametrize("count", range(1, 4)) def test_loop_invariant_hoisting_simple(depth, count): ctx = IRContext() fn = ctx.create_function("_global") @@ -73,14 +77,14 @@ def test_loop_invariant_hoisting_simple(depth, count): bb = fn.get_basic_block() bb.append_instruction("ret") - + ac = IRAnalysesCache(fn) LoopInvariantHoisting(ac, fn).run_pass() entry = fn.entry - assignments = list(map(lambda x : x.value, entry.get_assignments())) + assignments = list(map(lambda x: x.value, entry.get_assignments())) for bb in filter(lambda bb: bb.label.name.startswith("exit_top"), fn.get_basic_blocks()): - assignments.extend(map(lambda x : x.value, bb.get_assignments())) + assignments.extend(map(lambda x: x.value, bb.get_assignments())) assert len(assignments) == depth * count * 2 for loop_id in range(count): @@ -89,8 +93,8 @@ def test_loop_invariant_hoisting_simple(depth, count): assert f"%cond_var{loop_id}{d}" in assignments, repr(fn) -@pytest.mark.parametrize("depth", range(1, 5)) -@pytest.mark.parametrize("count", range(1, 5)) +@pytest.mark.parametrize("depth", range(1, 4)) +@pytest.mark.parametrize("count", range(1, 4)) def test_loop_invariant_hoisting_dependant(depth, count): ctx = IRContext() fn = ctx.create_function("_global") @@ -100,14 +104,14 @@ def test_loop_invariant_hoisting_dependant(depth, count): bb = fn.get_basic_block() bb.append_instruction("ret") - + ac = IRAnalysesCache(fn) LoopInvariantHoisting(ac, fn).run_pass() entry = fn.entry - assignments = list(map(lambda x : x.value, entry.get_assignments())) + assignments = list(map(lambda x: x.value, entry.get_assignments())) for bb in filter(lambda bb: bb.label.name.startswith("exit_top"), fn.get_basic_blocks()): - assignments.extend(map(lambda x : x.value, bb.get_assignments())) + assignments.extend(map(lambda x: x.value, bb.get_assignments())) assert len(assignments) == depth * count * 3 for loop_id in range(count): @@ -116,3 +120,36 @@ def test_loop_invariant_hoisting_dependant(depth, count): assert f"%add_var_b{loop_id}{d}" in assignments, repr(fn) assert f"%cond_var{loop_id}{d}" in assignments, repr(fn) +def _unhoistable_body(fn, loop_id, depth): + assert isinstance(fn, IRFunction) + bb = fn.get_basic_block() + add_var_a = IRVariable(f"add_var_a{loop_id}{depth}") + bb.append_instruction("mload", 64, ret=add_var_a) + add_var_b = IRVariable(f"add_var_b{loop_id}{depth}") + bb.append_instruction("add", add_var_a, 2, ret=add_var_b) + +@pytest.mark.parametrize("depth", range(1, 4)) +@pytest.mark.parametrize("count", range(1, 4)) +def test_loop_invariant_hoisting_unhoistable(depth, count): + ctx = IRContext() + fn = ctx.create_function("_global") + + for c in range(count): + _create_loops(fn, depth, c, _unhoistable_body) + + bb = fn.get_basic_block() + bb.append_instruction("ret") + + ac = IRAnalysesCache(fn) + LoopInvariantHoisting(ac, fn).run_pass() + + entry = fn.entry + assignments = list(map(lambda x: x.value, entry.get_assignments())) + for bb in filter(lambda bb: bb.label.name.startswith("exit_top"), fn.get_basic_blocks()): + assignments.extend(map(lambda x: x.value, bb.get_assignments())) + + assert len(assignments) == depth * count + for loop_id in range(count): + for d in range(1, depth + 1): + assert f"%cond_var{loop_id}{d}" in assignments, repr(fn) + diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index afd79fc44f..e5454e7338 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -13,6 +13,7 @@ from vyper.venom.passes.branch_optimization import BranchOptimizationPass from vyper.venom.passes.dft import DFTPass from vyper.venom.passes.extract_literals import ExtractLiteralsPass +from vyper.venom.passes.loop_invariant_hosting import LoopInvariantHoisting from vyper.venom.passes.make_ssa import MakeSSA from vyper.venom.passes.mem2var import Mem2Var from vyper.venom.passes.remove_unused_variables import RemoveUnusedVariablesPass @@ -55,6 +56,7 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: AlgebraicOptimizationPass(ac, fn).run_pass() BranchOptimizationPass(ac, fn).run_pass() ExtractLiteralsPass(ac, fn).run_pass() + LoopInvariantHoisting(ac, fn).run_pass() RemoveUnusedVariablesPass(ac, fn).run_pass() DFTPass(ac, fn).run_pass() diff --git a/vyper/venom/analysis/loop_detection.py b/vyper/venom/analysis/loop_detection.py index 086920d7da..9b15dbe7a5 100644 --- a/vyper/venom/analysis/loop_detection.py +++ b/vyper/venom/analysis/loop_detection.py @@ -1,9 +1,10 @@ +from vyper.utils import OrderedSet from vyper.venom.analysis.analysis import IRAnalysis from vyper.venom.analysis.cfg import CFGAnalysis from vyper.venom.basicblock import IRBasicBlock -class LoopDetection(IRAnalysis): +class LoopDetectionAnalysis(IRAnalysis): """ Detects loops and computes basic blocks sets which comprised these loops @@ -15,31 +16,29 @@ class LoopDetection(IRAnalysis): def analyze(self): self.analyses_cache.request_analysis(CFGAnalysis) self.loops: dict[IRBasicBlock, list[IRBasicBlock]] = dict() - done: dict[IRBasicBlock, bool] = dict( - (bb, False) for bb in self.function.get_basic_blocks() - ) + done = OrderedSet() entry = self.function.entry self.dfs(entry, done, []) def invalidate(self): return super().invalidate() - def dfs(self, bb: IRBasicBlock, done: dict[IRBasicBlock, bool], path: list[IRBasicBlock]): + def dfs(self, bb: IRBasicBlock, done: OrderedSet[IRBasicBlock], path: list[IRBasicBlock]): if bb in path: index = path.index(bb) assert index >= 1, "Loop must have one basic block before it" assert ( path[index - 1] not in self.loops.keys() ), "From one basic block can start only one loop" - done[bb] = True + done.add(bb) self.loops[path[index - 1]] = path[index:].copy() return path.append(bb) for neighbour in bb.cfg_out: - if not done[neighbour]: + if neighbour not in done: self.dfs(neighbour, done, path) path.pop() - done[bb] = True + done.add(bb) return diff --git a/vyper/venom/passes/loop_invariant_hosting.py b/vyper/venom/passes/loop_invariant_hosting.py index 33b9f471b5..2c7fcc2329 100644 --- a/vyper/venom/passes/loop_invariant_hosting.py +++ b/vyper/venom/passes/loop_invariant_hosting.py @@ -1,8 +1,7 @@ -from vyper.utils import OrderedSet from vyper.venom.analysis.cfg import CFGAnalysis from vyper.venom.analysis.dfg import DFGAnalysis from vyper.venom.analysis.liveness import LivenessAnalysis -from vyper.venom.analysis.loop_detection import LoopDetection +from vyper.venom.analysis.loop_detection import LoopDetectionAnalysis from vyper.venom.basicblock import ( BB_TERMINATORS, CFG_ALTERING_INSTRUCTIONS, @@ -20,24 +19,26 @@ class LoopInvariantHoisting(IRPass): function: IRFunction loops: dict[IRBasicBlock, list[IRBasicBlock]] - dfg : DFGAnalysis + dfg: DFGAnalysis def run_pass(self): self.analyses_cache.request_analysis(CFGAnalysis) self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) - loops = self.analyses_cache.request_analysis(LoopDetection) + loops = self.analyses_cache.request_analysis(LoopDetectionAnalysis) self.loops = loops.loops while True: change = False for from_bb, loop in self.loops.items(): - hoistable: list[tuple[IRBasicBlock, int, IRInstruction]] = self._get_hoistable_loop(from_bb, loop) + hoistable: list[tuple[IRBasicBlock, int, IRInstruction]] = self._get_hoistable_loop( + from_bb, loop + ) if len(hoistable) == 0: continue change |= True self._hoist(hoistable) if not change: break - # I have this inside the loop because I dont need to + # I have this inside the loop because I dont need to # invalidate if you dont hoist anything self.analyses_cache.invalidate_analysis(LivenessAnalysis) @@ -48,7 +49,9 @@ def _hoist(self, hoistable: list[tuple[IRBasicBlock, int, IRInstruction]]): bb_before: IRBasicBlock = loop_idx bb_before.insert_instruction(inst, index=len(bb_before.instructions) - 1) - def _get_hoistable_loop(self, from_bb : IRBasicBlock, loop : list[IRBasicBlock]) -> list[tuple[IRBasicBlock, int, IRInstruction]]: + def _get_hoistable_loop( + self, from_bb: IRBasicBlock, loop: list[IRBasicBlock] + ) -> list[tuple[IRBasicBlock, int, IRInstruction]]: result: list[tuple[IRBasicBlock, int, IRInstruction]] = [] for bb_idx, bb in enumerate(loop): result.extend(self._get_hoistable_bb(bb, from_bb, bb_idx)) From 399a0a4b3386c50ca9d2db34ad9bba0385c1861e Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 15 Jul 2024 10:08:29 +0200 Subject: [PATCH 04/17] feat[venom]: clean up and comments --- vyper/venom/analysis/loop_detection.py | 7 ++++--- vyper/venom/passes/loop_invariant_hosting.py | 4 ++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/vyper/venom/analysis/loop_detection.py b/vyper/venom/analysis/loop_detection.py index 9b15dbe7a5..917bafed81 100644 --- a/vyper/venom/analysis/loop_detection.py +++ b/vyper/venom/analysis/loop_detection.py @@ -6,11 +6,12 @@ class LoopDetectionAnalysis(IRAnalysis): """ - Detects loops and computes basic blocks sets - which comprised these loops + Detects loops and computes basic blocks + and the block which is before the loop """ - # key = start of the loop (last bb not in the loop), value all the block that loop contains + # key = start of the loop (last bb not in the loop) + # value = all the block that loop contains loops: dict[IRBasicBlock, list[IRBasicBlock]] def analyze(self): diff --git a/vyper/venom/passes/loop_invariant_hosting.py b/vyper/venom/passes/loop_invariant_hosting.py index 2c7fcc2329..665216376a 100644 --- a/vyper/venom/passes/loop_invariant_hosting.py +++ b/vyper/venom/passes/loop_invariant_hosting.py @@ -15,6 +15,10 @@ class LoopInvariantHoisting(IRPass): + """ + This pass detects invariants in loops and hoists them above the loop body. + Any VOLATILE_INSTRUCTIONS, BB_TERMINATORS CFG_ALTERING_INSTRUCTIONS are ignored + """ from typing import Iterator function: IRFunction From 2a8dd4a81a2ba0ae540343d4da698e11593fb1e4 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 15 Jul 2024 19:27:58 +0200 Subject: [PATCH 05/17] fix[venom]: incorrect loop detection fix --- vyper/venom/analysis/loop_detection.py | 57 +++++++++++++------- vyper/venom/passes/loop_invariant_hosting.py | 33 ++++++------ 2 files changed, 56 insertions(+), 34 deletions(-) diff --git a/vyper/venom/analysis/loop_detection.py b/vyper/venom/analysis/loop_detection.py index 917bafed81..fdc036359a 100644 --- a/vyper/venom/analysis/loop_detection.py +++ b/vyper/venom/analysis/loop_detection.py @@ -12,34 +12,55 @@ class LoopDetectionAnalysis(IRAnalysis): # key = start of the loop (last bb not in the loop) # value = all the block that loop contains - loops: dict[IRBasicBlock, list[IRBasicBlock]] + loops: dict[IRBasicBlock, OrderedSet[IRBasicBlock]] + + done: OrderedSet[IRBasicBlock] + visited: OrderedSet[IRBasicBlock] def analyze(self): self.analyses_cache.request_analysis(CFGAnalysis) - self.loops: dict[IRBasicBlock, list[IRBasicBlock]] = dict() - done = OrderedSet() + self.loops: dict[IRBasicBlock, OrderedSet[IRBasicBlock]] = dict() + self.done = OrderedSet() + self.visited = OrderedSet() entry = self.function.entry - self.dfs(entry, done, []) + self.dfs(entry) def invalidate(self): return super().invalidate() - def dfs(self, bb: IRBasicBlock, done: OrderedSet[IRBasicBlock], path: list[IRBasicBlock]): - if bb in path: - index = path.index(bb) - assert index >= 1, "Loop must have one basic block before it" - assert ( - path[index - 1] not in self.loops.keys() - ), "From one basic block can start only one loop" - done.add(bb) - self.loops[path[index - 1]] = path[index:].copy() + def dfs(self, bb: IRBasicBlock, before : IRBasicBlock = None): + if bb in self.visited: + assert before is not None, "Loop must have one basic block before it" + loop = self.collect_path(before, bb) + in_bb = bb.cfg_in.difference({before}) + assert len(in_bb) == 1, "Loop must have one input basic block" + input_bb = in_bb.first() + self.loops[input_bb] = loop + self.done.add(bb) return - path.append(bb) + self.visited.add(bb) + for neighbour in bb.cfg_out: - if neighbour not in done: - self.dfs(neighbour, done, path) - path.pop() + if neighbour not in self.done: + self.dfs(neighbour, bb) - done.add(bb) + self.done.add(bb) return + + def collect_path(self, bb_from : IRBasicBlock, bb_to: IRBasicBlock) -> OrderedSet[IRBasicBlock]: + loop = OrderedSet() + collect_visit = OrderedSet() + self.collect_path_inner(bb_from, bb_to, loop, collect_visit) + return loop + + def collect_path_inner(self, act_bb : IRBasicBlock, bb_to: IRBasicBlock, loop : OrderedSet[IRBasicBlock], collect_visit : OrderedSet[IRBasicBlock]): + if act_bb in collect_visit: + return + collect_visit.add(act_bb) + loop.add(act_bb) + if act_bb == bb_to: + return + + for before in act_bb.cfg_in: + self.collect_path_inner(before, bb_to, loop, collect_visit) diff --git a/vyper/venom/passes/loop_invariant_hosting.py b/vyper/venom/passes/loop_invariant_hosting.py index 665216376a..469396e26b 100644 --- a/vyper/venom/passes/loop_invariant_hosting.py +++ b/vyper/venom/passes/loop_invariant_hosting.py @@ -13,6 +13,8 @@ from vyper.venom.function import IRFunction from vyper.venom.passes.base_pass import IRPass +from vyper.utils import OrderedSet + class LoopInvariantHoisting(IRPass): """ @@ -22,7 +24,7 @@ class LoopInvariantHoisting(IRPass): from typing import Iterator function: IRFunction - loops: dict[IRBasicBlock, list[IRBasicBlock]] + loops: dict[IRBasicBlock, OrderedSet[IRBasicBlock]] dfg: DFGAnalysis def run_pass(self): @@ -33,7 +35,7 @@ def run_pass(self): while True: change = False for from_bb, loop in self.loops.items(): - hoistable: list[tuple[IRBasicBlock, int, IRInstruction]] = self._get_hoistable_loop( + hoistable: list[tuple[IRBasicBlock, IRBasicBlock, IRInstruction]] = self._get_hoistable_loop( from_bb, loop ) if len(hoistable) == 0: @@ -46,32 +48,31 @@ def run_pass(self): # invalidate if you dont hoist anything self.analyses_cache.invalidate_analysis(LivenessAnalysis) - def _hoist(self, hoistable: list[tuple[IRBasicBlock, int, IRInstruction]]): - for loop_idx, bb_idx, inst in hoistable: - loop = self.loops[loop_idx] - loop[bb_idx].remove_instruction(inst) + def _hoist(self, hoistable: list[tuple[IRBasicBlock, IRBasicBlock, IRInstruction]]): + for loop_idx, bb, inst in hoistable: + bb.remove_instruction(inst) bb_before: IRBasicBlock = loop_idx bb_before.insert_instruction(inst, index=len(bb_before.instructions) - 1) def _get_hoistable_loop( - self, from_bb: IRBasicBlock, loop: list[IRBasicBlock] - ) -> list[tuple[IRBasicBlock, int, IRInstruction]]: - result: list[tuple[IRBasicBlock, int, IRInstruction]] = [] - for bb_idx, bb in enumerate(loop): - result.extend(self._get_hoistable_bb(bb, from_bb, bb_idx)) + self, from_bb: IRBasicBlock, loop: OrderedSet[IRBasicBlock] + ) -> list[tuple[IRBasicBlock, IRBasicBlock, IRInstruction]]: + result: list[tuple[IRBasicBlock, IRBasicBlock, IRInstruction]] = [] + for bb in loop: + result.extend(self._get_hoistable_bb(bb, from_bb)) return result def _get_hoistable_bb( - self, bb: IRBasicBlock, loop_idx: IRBasicBlock, bb_idx - ) -> list[tuple[IRBasicBlock, int, IRInstruction]]: - result: list[tuple[IRBasicBlock, int, IRInstruction]] = [] + self, bb: IRBasicBlock, loop_idx: IRBasicBlock + ) -> list[tuple[IRBasicBlock, IRBasicBlock, IRInstruction]]: + result: list[tuple[IRBasicBlock, IRBasicBlock, IRInstruction]] = [] for instruction in bb.instructions: if self._can_hoist_instruction(instruction, self.loops[loop_idx]): - result.append((loop_idx, bb_idx, instruction)) + result.append((loop_idx, bb, instruction)) return result - def _can_hoist_instruction(self, instruction: IRInstruction, loop: list[IRBasicBlock]) -> bool: + def _can_hoist_instruction(self, instruction: IRInstruction, loop: OrderedSet[IRBasicBlock]) -> bool: if ( instruction.opcode in VOLATILE_INSTRUCTIONS or instruction.opcode in BB_TERMINATORS From 302aa218006d7e00dbc0e96b64029abf005dbe39 Mon Sep 17 00:00:00 2001 From: Hodan Date: Wed, 17 Jul 2024 15:43:40 +0200 Subject: [PATCH 06/17] fix[venom]: removed hoisting just constants --- vyper/venom/passes/loop_invariant_hosting.py | 45 ++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/vyper/venom/passes/loop_invariant_hosting.py b/vyper/venom/passes/loop_invariant_hosting.py index 469396e26b..cabb7a8a42 100644 --- a/vyper/venom/passes/loop_invariant_hosting.py +++ b/vyper/venom/passes/loop_invariant_hosting.py @@ -2,6 +2,7 @@ from vyper.venom.analysis.dfg import DFGAnalysis from vyper.venom.analysis.liveness import LivenessAnalysis from vyper.venom.analysis.loop_detection import LoopDetectionAnalysis +from vyper.venom.analysis.dup_requirements import DupRequirementsAnalysis from vyper.venom.basicblock import ( BB_TERMINATORS, CFG_ALTERING_INSTRUCTIONS, @@ -9,6 +10,7 @@ IRBasicBlock, IRInstruction, IRVariable, + IRLiteral ) from vyper.venom.function import IRFunction from vyper.venom.passes.base_pass import IRPass @@ -77,11 +79,26 @@ def _can_hoist_instruction(self, instruction: IRInstruction, loop: OrderedSet[IR instruction.opcode in VOLATILE_INSTRUCTIONS or instruction.opcode in BB_TERMINATORS or instruction.opcode in CFG_ALTERING_INSTRUCTIONS + or instruction.opcode == "returndatasize" ): return False for bb in loop: if self._in_bb(instruction, bb): return False + + if ( + instruction.opcode == "store" + and len(instruction.operands) == 1 + and isinstance(instruction.operands[0], IRLiteral) + ): + all_can = True + for used_instruction in self.dfg.get_uses(instruction.output): + if not self._can_hoist_instruction_ignore_stores(used_instruction, loop): + all_can = False + break + if not all_can: + return False + return True def _in_bb(self, instruction: IRInstruction, bb: IRBasicBlock): @@ -91,3 +108,31 @@ def _in_bb(self, instruction: IRInstruction, bb: IRBasicBlock): if source_ins in bb.instructions: return True return False + + def _can_hoist_instruction_ignore_stores(self, instruction: IRInstruction, loop: OrderedSet[IRBasicBlock]) -> bool: + if ( + instruction.opcode in VOLATILE_INSTRUCTIONS + or instruction.opcode in BB_TERMINATORS + or instruction.opcode in CFG_ALTERING_INSTRUCTIONS + or instruction.opcode == "returndatasize" + ): + return False + for bb in loop: + if self._in_bb_ignore_store(instruction, bb): + return False + return True + + def _in_bb_ignore_store(self, instruction: IRInstruction, bb: IRBasicBlock): + for in_var in instruction.get_input_variables(): + assert isinstance(in_var, IRVariable) + source_ins = self.dfg._dfg_outputs[in_var] + if ( + source_ins.opcode == "store" + and len(source_ins.operands) == 1 + and isinstance(source_ins.operands[0], int) + ): + continue + + if source_ins in bb.instructions: + return True + return False From 013840c7c9684a359d6d07fa0c91253ff115ab8b Mon Sep 17 00:00:00 2001 From: Hodan Date: Thu, 18 Jul 2024 13:29:17 +0200 Subject: [PATCH 07/17] fix[venom]: removed unnecessary hoisting --- vyper/venom/__init__.py | 2 +- vyper/venom/passes/loop_invariant_hosting.py | 48 +++++++++----------- 2 files changed, 22 insertions(+), 28 deletions(-) diff --git a/vyper/venom/__init__.py b/vyper/venom/__init__.py index e5454e7338..a9cb1f1c42 100644 --- a/vyper/venom/__init__.py +++ b/vyper/venom/__init__.py @@ -56,8 +56,8 @@ def _run_passes(fn: IRFunction, optimize: OptimizationLevel) -> None: AlgebraicOptimizationPass(ac, fn).run_pass() BranchOptimizationPass(ac, fn).run_pass() ExtractLiteralsPass(ac, fn).run_pass() - LoopInvariantHoisting(ac, fn).run_pass() RemoveUnusedVariablesPass(ac, fn).run_pass() + LoopInvariantHoisting(ac, fn).run_pass() DFTPass(ac, fn).run_pass() diff --git a/vyper/venom/passes/loop_invariant_hosting.py b/vyper/venom/passes/loop_invariant_hosting.py index cabb7a8a42..db1a1eb043 100644 --- a/vyper/venom/passes/loop_invariant_hosting.py +++ b/vyper/venom/passes/loop_invariant_hosting.py @@ -18,6 +18,21 @@ from vyper.utils import OrderedSet +def _ignore_instruction(instruction : IRInstruction) -> bool: + return ( + instruction.is_volatile + or instruction.is_bb_terminator + or instruction.opcode == "returndatasize" + or instruction.opcode == "phi" + ) + +def _is_correct_store(instruction : IRInstruction) -> bool: + return ( + instruction.opcode == "store" + and len(instruction.operands) == 1 + and isinstance(instruction.operands[0], IRLiteral) + ) + class LoopInvariantHoisting(IRPass): """ This pass detects invariants in loops and hoists them above the loop body. @@ -75,29 +90,16 @@ def _get_hoistable_bb( return result def _can_hoist_instruction(self, instruction: IRInstruction, loop: OrderedSet[IRBasicBlock]) -> bool: - if ( - instruction.opcode in VOLATILE_INSTRUCTIONS - or instruction.opcode in BB_TERMINATORS - or instruction.opcode in CFG_ALTERING_INSTRUCTIONS - or instruction.opcode == "returndatasize" - ): + if _ignore_instruction(instruction): return False for bb in loop: if self._in_bb(instruction, bb): return False - if ( - instruction.opcode == "store" - and len(instruction.operands) == 1 - and isinstance(instruction.operands[0], IRLiteral) - ): - all_can = True + if _is_correct_store(instruction): for used_instruction in self.dfg.get_uses(instruction.output): if not self._can_hoist_instruction_ignore_stores(used_instruction, loop): - all_can = False - break - if not all_can: - return False + return False return True @@ -110,12 +112,7 @@ def _in_bb(self, instruction: IRInstruction, bb: IRBasicBlock): return False def _can_hoist_instruction_ignore_stores(self, instruction: IRInstruction, loop: OrderedSet[IRBasicBlock]) -> bool: - if ( - instruction.opcode in VOLATILE_INSTRUCTIONS - or instruction.opcode in BB_TERMINATORS - or instruction.opcode in CFG_ALTERING_INSTRUCTIONS - or instruction.opcode == "returndatasize" - ): + if _ignore_instruction(instruction): return False for bb in loop: if self._in_bb_ignore_store(instruction, bb): @@ -126,13 +123,10 @@ def _in_bb_ignore_store(self, instruction: IRInstruction, bb: IRBasicBlock): for in_var in instruction.get_input_variables(): assert isinstance(in_var, IRVariable) source_ins = self.dfg._dfg_outputs[in_var] - if ( - source_ins.opcode == "store" - and len(source_ins.operands) == 1 - and isinstance(source_ins.operands[0], int) - ): + if _is_correct_store(source_ins): continue if source_ins in bb.instructions: return True return False + From 1ee0068abf2b24864f5b69b92f2cbc03b4ac935f Mon Sep 17 00:00:00 2001 From: Hodan Date: Thu, 18 Jul 2024 16:22:57 +0200 Subject: [PATCH 08/17] feat[venom]: clean up after changes --- vyper/venom/analysis/loop_detection.py | 18 ++++++---- vyper/venom/passes/loop_invariant_hosting.py | 37 +++++++++----------- 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/vyper/venom/analysis/loop_detection.py b/vyper/venom/analysis/loop_detection.py index fdc036359a..a2a578448e 100644 --- a/vyper/venom/analysis/loop_detection.py +++ b/vyper/venom/analysis/loop_detection.py @@ -6,7 +6,7 @@ class LoopDetectionAnalysis(IRAnalysis): """ - Detects loops and computes basic blocks + Detects loops and computes basic blocks and the block which is before the loop """ @@ -28,7 +28,7 @@ def analyze(self): def invalidate(self): return super().invalidate() - def dfs(self, bb: IRBasicBlock, before : IRBasicBlock = None): + def dfs(self, bb: IRBasicBlock, before: IRBasicBlock = None): if bb in self.visited: assert before is not None, "Loop must have one basic block before it" loop = self.collect_path(before, bb) @@ -47,20 +47,26 @@ def dfs(self, bb: IRBasicBlock, before : IRBasicBlock = None): self.done.add(bb) return - - def collect_path(self, bb_from : IRBasicBlock, bb_to: IRBasicBlock) -> OrderedSet[IRBasicBlock]: + + def collect_path(self, bb_from: IRBasicBlock, bb_to: IRBasicBlock) -> OrderedSet[IRBasicBlock]: loop = OrderedSet() collect_visit = OrderedSet() self.collect_path_inner(bb_from, bb_to, loop, collect_visit) return loop - def collect_path_inner(self, act_bb : IRBasicBlock, bb_to: IRBasicBlock, loop : OrderedSet[IRBasicBlock], collect_visit : OrderedSet[IRBasicBlock]): + def collect_path_inner( + self, + act_bb: IRBasicBlock, + bb_to: IRBasicBlock, + loop: OrderedSet[IRBasicBlock], + collect_visit: OrderedSet[IRBasicBlock], + ): if act_bb in collect_visit: return collect_visit.add(act_bb) loop.add(act_bb) if act_bb == bb_to: return - + for before in act_bb.cfg_in: self.collect_path_inner(before, bb_to, loop, collect_visit) diff --git a/vyper/venom/passes/loop_invariant_hosting.py b/vyper/venom/passes/loop_invariant_hosting.py index db1a1eb043..1bb739b2a8 100644 --- a/vyper/venom/passes/loop_invariant_hosting.py +++ b/vyper/venom/passes/loop_invariant_hosting.py @@ -1,24 +1,13 @@ +from vyper.utils import OrderedSet from vyper.venom.analysis.cfg import CFGAnalysis from vyper.venom.analysis.dfg import DFGAnalysis from vyper.venom.analysis.liveness import LivenessAnalysis from vyper.venom.analysis.loop_detection import LoopDetectionAnalysis -from vyper.venom.analysis.dup_requirements import DupRequirementsAnalysis -from vyper.venom.basicblock import ( - BB_TERMINATORS, - CFG_ALTERING_INSTRUCTIONS, - VOLATILE_INSTRUCTIONS, - IRBasicBlock, - IRInstruction, - IRVariable, - IRLiteral -) +from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRLiteral, IRVariable from vyper.venom.function import IRFunction from vyper.venom.passes.base_pass import IRPass -from vyper.utils import OrderedSet - - -def _ignore_instruction(instruction : IRInstruction) -> bool: +def _ignore_instruction(instruction: IRInstruction) -> bool: return ( instruction.is_volatile or instruction.is_bb_terminator @@ -26,18 +15,21 @@ def _ignore_instruction(instruction : IRInstruction) -> bool: or instruction.opcode == "phi" ) -def _is_correct_store(instruction : IRInstruction) -> bool: + +def _is_correct_store(instruction: IRInstruction) -> bool: return ( instruction.opcode == "store" and len(instruction.operands) == 1 and isinstance(instruction.operands[0], IRLiteral) ) + class LoopInvariantHoisting(IRPass): """ This pass detects invariants in loops and hoists them above the loop body. Any VOLATILE_INSTRUCTIONS, BB_TERMINATORS CFG_ALTERING_INSTRUCTIONS are ignored """ + from typing import Iterator function: IRFunction @@ -52,9 +44,9 @@ def run_pass(self): while True: change = False for from_bb, loop in self.loops.items(): - hoistable: list[tuple[IRBasicBlock, IRBasicBlock, IRInstruction]] = self._get_hoistable_loop( - from_bb, loop - ) + hoistable: list[ + tuple[IRBasicBlock, IRBasicBlock, IRInstruction] + ] = self._get_hoistable_loop(from_bb, loop) if len(hoistable) == 0: continue change |= True @@ -89,7 +81,9 @@ def _get_hoistable_bb( return result - def _can_hoist_instruction(self, instruction: IRInstruction, loop: OrderedSet[IRBasicBlock]) -> bool: + def _can_hoist_instruction( + self, instruction: IRInstruction, loop: OrderedSet[IRBasicBlock] + ) -> bool: if _ignore_instruction(instruction): return False for bb in loop: @@ -111,7 +105,9 @@ def _in_bb(self, instruction: IRInstruction, bb: IRBasicBlock): return True return False - def _can_hoist_instruction_ignore_stores(self, instruction: IRInstruction, loop: OrderedSet[IRBasicBlock]) -> bool: + def _can_hoist_instruction_ignore_stores( + self, instruction: IRInstruction, loop: OrderedSet[IRBasicBlock] + ) -> bool: if _ignore_instruction(instruction): return False for bb in loop: @@ -129,4 +125,3 @@ def _in_bb_ignore_store(self, instruction: IRInstruction, bb: IRBasicBlock): if source_ins in bb.instructions: return True return False - From f703408717c4a1f1ac527a2b1e4aa88fc7f8f231 Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 19 Jul 2024 17:16:09 +0200 Subject: [PATCH 09/17] fix[venom]: changes from review and better loophoisting structure (skipping over the literals) --- vyper/venom/analysis/loop_detection.py | 23 ++--- vyper/venom/passes/loop_invariant_hosting.py | 102 +++++++++---------- 2 files changed, 56 insertions(+), 69 deletions(-) diff --git a/vyper/venom/analysis/loop_detection.py b/vyper/venom/analysis/loop_detection.py index a2a578448e..fbf3610a86 100644 --- a/vyper/venom/analysis/loop_detection.py +++ b/vyper/venom/analysis/loop_detection.py @@ -23,15 +23,12 @@ def analyze(self): self.done = OrderedSet() self.visited = OrderedSet() entry = self.function.entry - self.dfs(entry) + self._dfs_r(entry) - def invalidate(self): - return super().invalidate() - - def dfs(self, bb: IRBasicBlock, before: IRBasicBlock = None): + def _dfs_r(self, bb: IRBasicBlock, before: IRBasicBlock = None): if bb in self.visited: assert before is not None, "Loop must have one basic block before it" - loop = self.collect_path(before, bb) + loop = self._collect_path(before, bb) in_bb = bb.cfg_in.difference({before}) assert len(in_bb) == 1, "Loop must have one input basic block" input_bb = in_bb.first() @@ -43,18 +40,18 @@ def dfs(self, bb: IRBasicBlock, before: IRBasicBlock = None): for neighbour in bb.cfg_out: if neighbour not in self.done: - self.dfs(neighbour, bb) + self._dfs_r(neighbour, bb) self.done.add(bb) return - def collect_path(self, bb_from: IRBasicBlock, bb_to: IRBasicBlock) -> OrderedSet[IRBasicBlock]: - loop = OrderedSet() - collect_visit = OrderedSet() - self.collect_path_inner(bb_from, bb_to, loop, collect_visit) + def _collect_path(self, bb_from: IRBasicBlock, bb_to: IRBasicBlock) -> OrderedSet[IRBasicBlock]: + loop: OrderedSet[IRBasicBlock] = OrderedSet() + collect_visit: OrderedSet[IRBasicBlock] = OrderedSet() + self._collect_path_r(bb_from, bb_to, loop, collect_visit) return loop - def collect_path_inner( + def _collect_path_r( self, act_bb: IRBasicBlock, bb_to: IRBasicBlock, @@ -69,4 +66,4 @@ def collect_path_inner( return for before in act_bb.cfg_in: - self.collect_path_inner(before, bb_to, loop, collect_visit) + self._collect_path_r(before, bb_to, loop, collect_visit) diff --git a/vyper/venom/passes/loop_invariant_hosting.py b/vyper/venom/passes/loop_invariant_hosting.py index 1bb739b2a8..ebf42fdfbf 100644 --- a/vyper/venom/passes/loop_invariant_hosting.py +++ b/vyper/venom/passes/loop_invariant_hosting.py @@ -3,10 +3,11 @@ from vyper.venom.analysis.dfg import DFGAnalysis from vyper.venom.analysis.liveness import LivenessAnalysis from vyper.venom.analysis.loop_detection import LoopDetectionAnalysis -from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRLiteral, IRVariable +from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRVariable from vyper.venom.function import IRFunction from vyper.venom.passes.base_pass import IRPass + def _ignore_instruction(instruction: IRInstruction) -> bool: return ( instruction.is_volatile @@ -17,11 +18,7 @@ def _ignore_instruction(instruction: IRInstruction) -> bool: def _is_correct_store(instruction: IRInstruction) -> bool: - return ( - instruction.opcode == "store" - and len(instruction.operands) == 1 - and isinstance(instruction.operands[0], IRLiteral) - ) + return instruction.opcode == "store" class LoopInvariantHoisting(IRPass): @@ -30,98 +27,91 @@ class LoopInvariantHoisting(IRPass): Any VOLATILE_INSTRUCTIONS, BB_TERMINATORS CFG_ALTERING_INSTRUCTIONS are ignored """ - from typing import Iterator - function: IRFunction - loops: dict[IRBasicBlock, OrderedSet[IRBasicBlock]] + loop_analysis: dict[IRBasicBlock, OrderedSet[IRBasicBlock]] dfg: DFGAnalysis def run_pass(self): self.analyses_cache.request_analysis(CFGAnalysis) self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) loops = self.analyses_cache.request_analysis(LoopDetectionAnalysis) - self.loops = loops.loops + self.loop_analysis = loops.loops + invalidate_dependant = False while True: change = False - for from_bb, loop in self.loops.items(): - hoistable: list[ - tuple[IRBasicBlock, IRBasicBlock, IRInstruction] - ] = self._get_hoistable_loop(from_bb, loop) + for from_bb, loop in self.loop_analysis.items(): + hoistable: list[IRInstruction] = self._get_hoistable_loop(from_bb, loop) if len(hoistable) == 0: continue change |= True - self._hoist(hoistable) + self._hoist(from_bb, hoistable) if not change: break - # I have this inside the loop because I dont need to - # invalidate if you dont hoist anything + invalidate_dependant = True + + # only need to invalidate if you did some hoisting + if invalidate_dependant: self.analyses_cache.invalidate_analysis(LivenessAnalysis) - def _hoist(self, hoistable: list[tuple[IRBasicBlock, IRBasicBlock, IRInstruction]]): - for loop_idx, bb, inst in hoistable: + def _hoist(self, target_bb: IRBasicBlock, hoistable: list[IRInstruction]): + for inst in hoistable: + bb = inst.parent bb.remove_instruction(inst) - bb_before: IRBasicBlock = loop_idx - bb_before.insert_instruction(inst, index=len(bb_before.instructions) - 1) + target_bb.insert_instruction(inst, index=len(target_bb.instructions) - 1) def _get_hoistable_loop( self, from_bb: IRBasicBlock, loop: OrderedSet[IRBasicBlock] - ) -> list[tuple[IRBasicBlock, IRBasicBlock, IRInstruction]]: - result: list[tuple[IRBasicBlock, IRBasicBlock, IRInstruction]] = [] + ) -> list[IRInstruction]: + result: list[IRInstruction] = [] for bb in loop: result.extend(self._get_hoistable_bb(bb, from_bb)) return result - def _get_hoistable_bb( - self, bb: IRBasicBlock, loop_idx: IRBasicBlock - ) -> list[tuple[IRBasicBlock, IRBasicBlock, IRInstruction]]: - result: list[tuple[IRBasicBlock, IRBasicBlock, IRInstruction]] = [] + def _get_hoistable_bb(self, bb: IRBasicBlock, loop_idx: IRBasicBlock) -> list[IRInstruction]: + result: list[IRInstruction] = [] for instruction in bb.instructions: - if self._can_hoist_instruction(instruction, self.loops[loop_idx]): - result.append((loop_idx, bb, instruction)) + if self._can_hoist_instruction_ignore_stores(instruction, self.loop_analysis[loop_idx]): + result.extend(self._store_dependencies(instruction, loop_idx)) + result.append(instruction) return result - def _can_hoist_instruction( - self, instruction: IRInstruction, loop: OrderedSet[IRBasicBlock] - ) -> bool: - if _ignore_instruction(instruction): - return False - for bb in loop: - if self._in_bb(instruction, bb): - return False - - if _is_correct_store(instruction): - for used_instruction in self.dfg.get_uses(instruction.output): - if not self._can_hoist_instruction_ignore_stores(used_instruction, loop): - return False - - return True - - def _in_bb(self, instruction: IRInstruction, bb: IRBasicBlock): - for in_var in instruction.get_input_variables(): - assert isinstance(in_var, IRVariable) - source_ins = self.dfg._dfg_outputs[in_var] - if source_ins in bb.instructions: - return True - return False + # query store dependacies of instruction (they are not handled otherwise) + def _store_dependencies( + self, inst: IRInstruction, loop_idx: IRBasicBlock + ) -> list[IRInstruction]: + result: list[IRInstruction] = [] + for var in inst.get_input_variables(): + source_inst = self.dfg.get_producing_instruction(var) + assert isinstance(source_inst, IRInstruction) + if _is_correct_store(source_inst): + for bb in self.loop_analysis[loop_idx]: + if source_inst.parent == bb: + result.append(source_inst) + return result + # since the stores are always hoistable this ignores + # stores in analysis (their are hoisted if some instrution is dependent on them) def _can_hoist_instruction_ignore_stores( self, instruction: IRInstruction, loop: OrderedSet[IRBasicBlock] ) -> bool: - if _ignore_instruction(instruction): + if _ignore_instruction(instruction) or _is_correct_store(instruction): return False for bb in loop: - if self._in_bb_ignore_store(instruction, bb): + if self._dependant_in_bb(instruction, bb): return False return True - def _in_bb_ignore_store(self, instruction: IRInstruction, bb: IRBasicBlock): + def _dependant_in_bb(self, instruction: IRInstruction, bb: IRBasicBlock): for in_var in instruction.get_input_variables(): assert isinstance(in_var, IRVariable) source_ins = self.dfg._dfg_outputs[in_var] + + # ignores stores since all stores are independant + # and can be always hoisted if _is_correct_store(source_ins): continue - if source_ins in bb.instructions: + if source_ins.parent == bb: return True return False From c5dbf05644dc2ff6995670506423e8adc6d1032e Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 22 Jul 2024 10:44:42 +0200 Subject: [PATCH 10/17] fix[venom]: ignore code offset calculation --- tests/unit/compiler/venom/test_loop_invariant_hoisting.py | 3 ++- vyper/venom/passes/loop_invariant_hosting.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/unit/compiler/venom/test_loop_invariant_hoisting.py b/tests/unit/compiler/venom/test_loop_invariant_hoisting.py index 4a3e6afcf3..50f7db1f48 100644 --- a/tests/unit/compiler/venom/test_loop_invariant_hoisting.py +++ b/tests/unit/compiler/venom/test_loop_invariant_hoisting.py @@ -120,6 +120,7 @@ def test_loop_invariant_hoisting_dependant(depth, count): assert f"%add_var_b{loop_id}{d}" in assignments, repr(fn) assert f"%cond_var{loop_id}{d}" in assignments, repr(fn) + def _unhoistable_body(fn, loop_id, depth): assert isinstance(fn, IRFunction) bb = fn.get_basic_block() @@ -128,6 +129,7 @@ def _unhoistable_body(fn, loop_id, depth): add_var_b = IRVariable(f"add_var_b{loop_id}{depth}") bb.append_instruction("add", add_var_a, 2, ret=add_var_b) + @pytest.mark.parametrize("depth", range(1, 4)) @pytest.mark.parametrize("count", range(1, 4)) def test_loop_invariant_hoisting_unhoistable(depth, count): @@ -152,4 +154,3 @@ def test_loop_invariant_hoisting_unhoistable(depth, count): for loop_id in range(count): for d in range(1, depth + 1): assert f"%cond_var{loop_id}{d}" in assignments, repr(fn) - diff --git a/vyper/venom/passes/loop_invariant_hosting.py b/vyper/venom/passes/loop_invariant_hosting.py index ebf42fdfbf..d6ce99ec24 100644 --- a/vyper/venom/passes/loop_invariant_hosting.py +++ b/vyper/venom/passes/loop_invariant_hosting.py @@ -3,7 +3,7 @@ from vyper.venom.analysis.dfg import DFGAnalysis from vyper.venom.analysis.liveness import LivenessAnalysis from vyper.venom.analysis.loop_detection import LoopDetectionAnalysis -from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRVariable +from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRLabel, IRVariable from vyper.venom.function import IRFunction from vyper.venom.passes.base_pass import IRPass @@ -14,6 +14,7 @@ def _ignore_instruction(instruction: IRInstruction) -> bool: or instruction.is_bb_terminator or instruction.opcode == "returndatasize" or instruction.opcode == "phi" + or (instruction.opcode == "add" and isinstance(instruction.operands[1], IRLabel)) ) From cd655fcf9418b50458d13c81607e271ad28f7dcd Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 22 Jul 2024 15:32:33 +0200 Subject: [PATCH 11/17] fix[venom]: changes according to review --- .../compiler/venom/test_loop_invariant_hoisting.py | 4 ++-- vyper/venom/analysis/loop_detection.py | 13 +++++++------ vyper/venom/passes/loop_invariant_hosting.py | 14 +++++++------- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/tests/unit/compiler/venom/test_loop_invariant_hoisting.py b/tests/unit/compiler/venom/test_loop_invariant_hoisting.py index 50f7db1f48..95dbe2ffaf 100644 --- a/tests/unit/compiler/venom/test_loop_invariant_hoisting.py +++ b/tests/unit/compiler/venom/test_loop_invariant_hoisting.py @@ -1,7 +1,7 @@ import pytest from vyper.venom.analysis.analysis import IRAnalysesCache -from vyper.venom.analysis.loop_detection import LoopDetectionAnalysis +from vyper.venom.analysis.loop_detection import NaturalLoopDetectionAnalysis from vyper.venom.basicblock import IRBasicBlock, IRLabel, IRVariable from vyper.venom.context import IRContext from vyper.venom.function import IRFunction @@ -62,7 +62,7 @@ def test_loop_detection_analysis(depth, count): bb.append_instruction("ret") ac = IRAnalysesCache(fn) - analysis = ac.request_analysis(LoopDetectionAnalysis) + analysis = ac.request_analysis(NaturalLoopDetectionAnalysis) assert len(analysis.loops) == depth * count diff --git a/vyper/venom/analysis/loop_detection.py b/vyper/venom/analysis/loop_detection.py index fbf3610a86..d57b6d8949 100644 --- a/vyper/venom/analysis/loop_detection.py +++ b/vyper/venom/analysis/loop_detection.py @@ -4,7 +4,7 @@ from vyper.venom.basicblock import IRBasicBlock -class LoopDetectionAnalysis(IRAnalysis): +class NaturalLoopDetectionAnalysis(IRAnalysis): """ Detects loops and computes basic blocks and the block which is before the loop @@ -25,15 +25,17 @@ def analyze(self): entry = self.function.entry self._dfs_r(entry) - def _dfs_r(self, bb: IRBasicBlock, before: IRBasicBlock = None): + def _dfs_r(self, bb: IRBasicBlock, before: IRBasicBlock | None = None): if bb in self.visited: - assert before is not None, "Loop must have one basic block before it" + self.done.add(bb) + if before is None: + return loop = self._collect_path(before, bb) in_bb = bb.cfg_in.difference({before}) - assert len(in_bb) == 1, "Loop must have one input basic block" + if len(in_bb) != 1: + return input_bb = in_bb.first() self.loops[input_bb] = loop - self.done.add(bb) return self.visited.add(bb) @@ -43,7 +45,6 @@ def _dfs_r(self, bb: IRBasicBlock, before: IRBasicBlock = None): self._dfs_r(neighbour, bb) self.done.add(bb) - return def _collect_path(self, bb_from: IRBasicBlock, bb_to: IRBasicBlock) -> OrderedSet[IRBasicBlock]: loop: OrderedSet[IRBasicBlock] = OrderedSet() diff --git a/vyper/venom/passes/loop_invariant_hosting.py b/vyper/venom/passes/loop_invariant_hosting.py index d6ce99ec24..4f6c68d9a4 100644 --- a/vyper/venom/passes/loop_invariant_hosting.py +++ b/vyper/venom/passes/loop_invariant_hosting.py @@ -2,13 +2,13 @@ from vyper.venom.analysis.cfg import CFGAnalysis from vyper.venom.analysis.dfg import DFGAnalysis from vyper.venom.analysis.liveness import LivenessAnalysis -from vyper.venom.analysis.loop_detection import LoopDetectionAnalysis +from vyper.venom.analysis.loop_detection import NaturalLoopDetectionAnalysis from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRLabel, IRVariable from vyper.venom.function import IRFunction from vyper.venom.passes.base_pass import IRPass -def _ignore_instruction(instruction: IRInstruction) -> bool: +def _ignore_instructions(instruction: IRInstruction) -> bool: return ( instruction.is_volatile or instruction.is_bb_terminator @@ -18,7 +18,7 @@ def _ignore_instruction(instruction: IRInstruction) -> bool: ) -def _is_correct_store(instruction: IRInstruction) -> bool: +def _is_store(instruction: IRInstruction) -> bool: return instruction.opcode == "store" @@ -35,7 +35,7 @@ class LoopInvariantHoisting(IRPass): def run_pass(self): self.analyses_cache.request_analysis(CFGAnalysis) self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) - loops = self.analyses_cache.request_analysis(LoopDetectionAnalysis) + loops = self.analyses_cache.request_analysis(NaturalLoopDetectionAnalysis) self.loop_analysis = loops.loops invalidate_dependant = False while True: @@ -85,7 +85,7 @@ def _store_dependencies( for var in inst.get_input_variables(): source_inst = self.dfg.get_producing_instruction(var) assert isinstance(source_inst, IRInstruction) - if _is_correct_store(source_inst): + if _is_store(source_inst): for bb in self.loop_analysis[loop_idx]: if source_inst.parent == bb: result.append(source_inst) @@ -96,7 +96,7 @@ def _store_dependencies( def _can_hoist_instruction_ignore_stores( self, instruction: IRInstruction, loop: OrderedSet[IRBasicBlock] ) -> bool: - if _ignore_instruction(instruction) or _is_correct_store(instruction): + if _ignore_instructions(instruction) or _is_store(instruction): return False for bb in loop: if self._dependant_in_bb(instruction, bb): @@ -110,7 +110,7 @@ def _dependant_in_bb(self, instruction: IRInstruction, bb: IRBasicBlock): # ignores stores since all stores are independant # and can be always hoisted - if _is_correct_store(source_ins): + if _is_store(source_ins): continue if source_ins.parent == bb: From ecb272a4f299e523562ca16d8e3b2af0422f9224 Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 22 Jul 2024 17:20:16 +0200 Subject: [PATCH 12/17] fix[venom]: changes according to review --- vyper/venom/passes/loop_invariant_hosting.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/vyper/venom/passes/loop_invariant_hosting.py b/vyper/venom/passes/loop_invariant_hosting.py index 4f6c68d9a4..796edd7db4 100644 --- a/vyper/venom/passes/loop_invariant_hosting.py +++ b/vyper/venom/passes/loop_invariant_hosting.py @@ -8,13 +8,14 @@ from vyper.venom.passes.base_pass import IRPass -def _ignore_instructions(instruction: IRInstruction) -> bool: +def _ignore_instruction(instruction: IRInstruction) -> bool: return ( instruction.is_volatile or instruction.is_bb_terminator or instruction.opcode == "returndatasize" or instruction.opcode == "phi" or (instruction.opcode == "add" and isinstance(instruction.operands[1], IRLabel)) + or instruction.opcode == "store" ) @@ -96,7 +97,7 @@ def _store_dependencies( def _can_hoist_instruction_ignore_stores( self, instruction: IRInstruction, loop: OrderedSet[IRBasicBlock] ) -> bool: - if _ignore_instructions(instruction) or _is_store(instruction): + if _ignore_instruction(instruction): return False for bb in loop: if self._dependant_in_bb(instruction, bb): From 18c7610bbdbcc624b50c5cab8505af5ddae62f71 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 23 Jul 2024 12:28:27 +0200 Subject: [PATCH 13/17] fix[venom]: changes according to review --- vyper/venom/analysis/loop_detection.py | 26 +++++----- vyper/venom/passes/loop_invariant_hosting.py | 53 ++++++++++---------- 2 files changed, 41 insertions(+), 38 deletions(-) diff --git a/vyper/venom/analysis/loop_detection.py b/vyper/venom/analysis/loop_detection.py index d57b6d8949..416bcb17dc 100644 --- a/vyper/venom/analysis/loop_detection.py +++ b/vyper/venom/analysis/loop_detection.py @@ -30,7 +30,7 @@ def _dfs_r(self, bb: IRBasicBlock, before: IRBasicBlock | None = None): self.done.add(bb) if before is None: return - loop = self._collect_path(before, bb) + loop = self._collect_loop_bbs(before, bb) in_bb = bb.cfg_in.difference({before}) if len(in_bb) != 1: return @@ -46,25 +46,27 @@ def _dfs_r(self, bb: IRBasicBlock, before: IRBasicBlock | None = None): self.done.add(bb) - def _collect_path(self, bb_from: IRBasicBlock, bb_to: IRBasicBlock) -> OrderedSet[IRBasicBlock]: + def _collect_loop_bbs( + self, bb_from: IRBasicBlock, bb_to: IRBasicBlock + ) -> OrderedSet[IRBasicBlock]: loop: OrderedSet[IRBasicBlock] = OrderedSet() collect_visit: OrderedSet[IRBasicBlock] = OrderedSet() - self._collect_path_r(bb_from, bb_to, loop, collect_visit) + self._collect_loop_bbs_r(bb_from, bb_to, loop, collect_visit) return loop - def _collect_path_r( + def _collect_loop_bbs_r( self, - act_bb: IRBasicBlock, + curr_bb: IRBasicBlock, bb_to: IRBasicBlock, loop: OrderedSet[IRBasicBlock], - collect_visit: OrderedSet[IRBasicBlock], + visit: OrderedSet[IRBasicBlock], ): - if act_bb in collect_visit: + if curr_bb in visit: return - collect_visit.add(act_bb) - loop.add(act_bb) - if act_bb == bb_to: + visit.add(curr_bb) + loop.add(curr_bb) + if curr_bb == bb_to: return - for before in act_bb.cfg_in: - self._collect_path_r(before, bb_to, loop, collect_visit) + for before in curr_bb.cfg_in: + self._collect_loop_bbs_r(before, bb_to, loop, visit) diff --git a/vyper/venom/passes/loop_invariant_hosting.py b/vyper/venom/passes/loop_invariant_hosting.py index 796edd7db4..2e2ef5e2b2 100644 --- a/vyper/venom/passes/loop_invariant_hosting.py +++ b/vyper/venom/passes/loop_invariant_hosting.py @@ -8,19 +8,19 @@ from vyper.venom.passes.base_pass import IRPass -def _ignore_instruction(instruction: IRInstruction) -> bool: +def _ignore_instruction(inst: IRInstruction) -> bool: return ( - instruction.is_volatile - or instruction.is_bb_terminator - or instruction.opcode == "returndatasize" - or instruction.opcode == "phi" - or (instruction.opcode == "add" and isinstance(instruction.operands[1], IRLabel)) - or instruction.opcode == "store" + inst.is_volatile + or inst.is_bb_terminator + or inst.opcode == "returndatasize" + or inst.opcode == "phi" + or (inst.opcode == "add" and isinstance(inst.operands[1], IRLabel)) + or inst.opcode == "store" ) -def _is_store(instruction: IRInstruction) -> bool: - return instruction.opcode == "store" +def _is_store(inst: IRInstruction) -> bool: + return inst.opcode == "store" class LoopInvariantHoisting(IRPass): @@ -30,18 +30,18 @@ class LoopInvariantHoisting(IRPass): """ function: IRFunction - loop_analysis: dict[IRBasicBlock, OrderedSet[IRBasicBlock]] + loops: dict[IRBasicBlock, OrderedSet[IRBasicBlock]] dfg: DFGAnalysis def run_pass(self): self.analyses_cache.request_analysis(CFGAnalysis) self.dfg = self.analyses_cache.request_analysis(DFGAnalysis) loops = self.analyses_cache.request_analysis(NaturalLoopDetectionAnalysis) - self.loop_analysis = loops.loops - invalidate_dependant = False + self.loops = loops.loops + invalidate = False while True: change = False - for from_bb, loop in self.loop_analysis.items(): + for from_bb, loop in self.loops.items(): hoistable: list[IRInstruction] = self._get_hoistable_loop(from_bb, loop) if len(hoistable) == 0: continue @@ -49,10 +49,10 @@ def run_pass(self): self._hoist(from_bb, hoistable) if not change: break - invalidate_dependant = True + invalidate = True # only need to invalidate if you did some hoisting - if invalidate_dependant: + if invalidate: self.analyses_cache.invalidate_analysis(LivenessAnalysis) def _hoist(self, target_bb: IRBasicBlock, hoistable: list[IRInstruction]): @@ -71,10 +71,10 @@ def _get_hoistable_loop( def _get_hoistable_bb(self, bb: IRBasicBlock, loop_idx: IRBasicBlock) -> list[IRInstruction]: result: list[IRInstruction] = [] - for instruction in bb.instructions: - if self._can_hoist_instruction_ignore_stores(instruction, self.loop_analysis[loop_idx]): - result.extend(self._store_dependencies(instruction, loop_idx)) - result.append(instruction) + for inst in bb.instructions: + if self._can_hoist_instruction_ignore_stores(inst, self.loops[loop_idx]): + result.extend(self._store_dependencies(inst, loop_idx)) + result.append(inst) return result @@ -87,7 +87,7 @@ def _store_dependencies( source_inst = self.dfg.get_producing_instruction(var) assert isinstance(source_inst, IRInstruction) if _is_store(source_inst): - for bb in self.loop_analysis[loop_idx]: + for bb in self.loops[loop_idx]: if source_inst.parent == bb: result.append(source_inst) return result @@ -95,19 +95,20 @@ def _store_dependencies( # since the stores are always hoistable this ignores # stores in analysis (their are hoisted if some instrution is dependent on them) def _can_hoist_instruction_ignore_stores( - self, instruction: IRInstruction, loop: OrderedSet[IRBasicBlock] + self, inst: IRInstruction, loop: OrderedSet[IRBasicBlock] ) -> bool: - if _ignore_instruction(instruction): + if _ignore_instruction(inst): return False for bb in loop: - if self._dependant_in_bb(instruction, bb): + if self._dependent_in_bb(inst, bb): return False return True - def _dependant_in_bb(self, instruction: IRInstruction, bb: IRBasicBlock): - for in_var in instruction.get_input_variables(): + def _dependent_in_bb(self, inst: IRInstruction, bb: IRBasicBlock): + for in_var in inst.get_input_variables(): assert isinstance(in_var, IRVariable) - source_ins = self.dfg._dfg_outputs[in_var] + source_ins = self.dfg.get_producing_instruction(in_var) + assert isinstance(source_ins, IRInstruction) # ignores stores since all stores are independant # and can be always hoisted From 5583cc50cce680aa1cdc8ea3f316139c20930ffa Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 23 Jul 2024 13:55:44 +0200 Subject: [PATCH 14/17] fix[venom]: review changes (_is_store condition negation to reduce nesting) --- vyper/venom/passes/loop_invariant_hosting.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/vyper/venom/passes/loop_invariant_hosting.py b/vyper/venom/passes/loop_invariant_hosting.py index 2e2ef5e2b2..a391f2f1cd 100644 --- a/vyper/venom/passes/loop_invariant_hosting.py +++ b/vyper/venom/passes/loop_invariant_hosting.py @@ -86,10 +86,11 @@ def _store_dependencies( for var in inst.get_input_variables(): source_inst = self.dfg.get_producing_instruction(var) assert isinstance(source_inst, IRInstruction) - if _is_store(source_inst): - for bb in self.loops[loop_idx]: - if source_inst.parent == bb: - result.append(source_inst) + if not _is_store(source_inst): + continue + for bb in self.loops[loop_idx]: + if source_inst.parent == bb: + result.append(source_inst) return result # since the stores are always hoistable this ignores From bdb2896c568aaaf891d86af5711326da6a461199 Mon Sep 17 00:00:00 2001 From: Harry Kalogirou Date: Mon, 29 Jul 2024 20:56:43 +0300 Subject: [PATCH 15/17] clean up loop detection --- vyper/venom/analysis/loop_detection.py | 94 ++++++++++++-------------- 1 file changed, 45 insertions(+), 49 deletions(-) diff --git a/vyper/venom/analysis/loop_detection.py b/vyper/venom/analysis/loop_detection.py index 416bcb17dc..acae766cb8 100644 --- a/vyper/venom/analysis/loop_detection.py +++ b/vyper/venom/analysis/loop_detection.py @@ -10,63 +10,59 @@ class NaturalLoopDetectionAnalysis(IRAnalysis): and the block which is before the loop """ - # key = start of the loop (last bb not in the loop) - # value = all the block that loop contains + # key = loop header + # value = all the blocks that the loop contains loops: dict[IRBasicBlock, OrderedSet[IRBasicBlock]] - done: OrderedSet[IRBasicBlock] - visited: OrderedSet[IRBasicBlock] - def analyze(self): self.analyses_cache.request_analysis(CFGAnalysis) - self.loops: dict[IRBasicBlock, OrderedSet[IRBasicBlock]] = dict() - self.done = OrderedSet() - self.visited = OrderedSet() - entry = self.function.entry - self._dfs_r(entry) + self.loops = self._find_natural_loops(self.function.entry) + + # Could possibly reuse the dominator tree algorithm to find the back edges + # if it is already cached it will be faster. Still might need to separate the + # varius extra information that the dominator analysis provides + # (like frontiers and immediate dominators) + def _find_back_edges(self, entry: IRBasicBlock) -> list[tuple[IRBasicBlock, IRBasicBlock]]: + back_edges = [] + visited = OrderedSet() + stack = [] + + def dfs(bb: IRBasicBlock): + visited.add(bb) + stack.append(bb) + + for succ in bb.cfg_out: + if succ not in visited: + dfs(succ) + elif succ in stack: + back_edges.append((bb, succ)) + + stack.pop() - def _dfs_r(self, bb: IRBasicBlock, before: IRBasicBlock | None = None): - if bb in self.visited: - self.done.add(bb) - if before is None: - return - loop = self._collect_loop_bbs(before, bb) - in_bb = bb.cfg_in.difference({before}) - if len(in_bb) != 1: - return - input_bb = in_bb.first() - self.loops[input_bb] = loop - return + dfs(entry) - self.visited.add(bb) + return back_edges + + def _find_natural_loops(self, entry: IRBasicBlock) -> dict[IRBasicBlock, OrderedSet[IRBasicBlock]]: + back_edges = self._find_back_edges(entry) + natural_loops = {} - for neighbour in bb.cfg_out: - if neighbour not in self.done: - self._dfs_r(neighbour, bb) + for u, v in back_edges: + # back edge: u -> v + loop = OrderedSet() + stack = [u] - self.done.add(bb) + while stack: + bb = stack.pop() + if bb in loop: + continue + loop.add(bb) + for pred in bb.cfg_in: + if pred != v: + stack.append(pred) - def _collect_loop_bbs( - self, bb_from: IRBasicBlock, bb_to: IRBasicBlock - ) -> OrderedSet[IRBasicBlock]: - loop: OrderedSet[IRBasicBlock] = OrderedSet() - collect_visit: OrderedSet[IRBasicBlock] = OrderedSet() - self._collect_loop_bbs_r(bb_from, bb_to, loop, collect_visit) - return loop + loop.add(v) + natural_loops[v.cfg_in.first()] = loop - def _collect_loop_bbs_r( - self, - curr_bb: IRBasicBlock, - bb_to: IRBasicBlock, - loop: OrderedSet[IRBasicBlock], - visit: OrderedSet[IRBasicBlock], - ): - if curr_bb in visit: - return - visit.add(curr_bb) - loop.add(curr_bb) - if curr_bb == bb_to: - return + return natural_loops - for before in curr_bb.cfg_in: - self._collect_loop_bbs_r(before, bb_to, loop, visit) From 715b128d2ece1fc4f9549a258bc68aee27a39c9b Mon Sep 17 00:00:00 2001 From: Hodan Date: Mon, 12 Aug 2024 10:15:21 +0200 Subject: [PATCH 16/17] fix[venom]: improved tests to increase coverage --- .../unit/compiler/venom/test_loop_invariant_hoisting.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/unit/compiler/venom/test_loop_invariant_hoisting.py b/tests/unit/compiler/venom/test_loop_invariant_hoisting.py index 95dbe2ffaf..27a499fcae 100644 --- a/tests/unit/compiler/venom/test_loop_invariant_hoisting.py +++ b/tests/unit/compiler/venom/test_loop_invariant_hoisting.py @@ -43,10 +43,12 @@ def _simple_body(fn, loop_id, depth): def _hoistable_body(fn, loop_id, depth): assert isinstance(fn, IRFunction) bb = fn.get_basic_block() + store_var = IRVariable(f"store_var{loop_id}{depth}") add_var_a = IRVariable(f"add_var_a{loop_id}{depth}") - bb.append_instruction("add", 1, 2, ret=add_var_a) + bb.append_instruction("store", 1, ret=store_var) + bb.append_instruction("add", 1, store_var, ret=add_var_a) add_var_b = IRVariable(f"add_var_b{loop_id}{depth}") - bb.append_instruction("add", add_var_a, 2, ret=add_var_b) + bb.append_instruction("add", store_var, add_var_a, ret=add_var_b) @pytest.mark.parametrize("depth", range(1, 4)) @@ -113,9 +115,10 @@ def test_loop_invariant_hoisting_dependant(depth, count): for bb in filter(lambda bb: bb.label.name.startswith("exit_top"), fn.get_basic_blocks()): assignments.extend(map(lambda x: x.value, bb.get_assignments())) - assert len(assignments) == depth * count * 3 + assert len(assignments) == depth * count * 4 for loop_id in range(count): for d in range(1, depth + 1): + assert f"%store_var{loop_id}{d}" in assignments, repr(fn) assert f"%add_var_a{loop_id}{d}" in assignments, repr(fn) assert f"%add_var_b{loop_id}{d}" in assignments, repr(fn) assert f"%cond_var{loop_id}{d}" in assignments, repr(fn) From 8edce11099fa2f3280b9476ba847c28e6f252eb6 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 13 Aug 2024 14:46:35 +0200 Subject: [PATCH 17/17] fix[venom]: error when the store is not constant --- vyper/venom/passes/loop_invariant_hosting.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/vyper/venom/passes/loop_invariant_hosting.py b/vyper/venom/passes/loop_invariant_hosting.py index a391f2f1cd..f3296a1f8a 100644 --- a/vyper/venom/passes/loop_invariant_hosting.py +++ b/vyper/venom/passes/loop_invariant_hosting.py @@ -3,7 +3,7 @@ from vyper.venom.analysis.dfg import DFGAnalysis from vyper.venom.analysis.liveness import LivenessAnalysis from vyper.venom.analysis.loop_detection import NaturalLoopDetectionAnalysis -from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRLabel, IRVariable +from vyper.venom.basicblock import IRBasicBlock, IRInstruction, IRLabel, IRVariable, IRLiteral from vyper.venom.function import IRFunction from vyper.venom.passes.base_pass import IRPass @@ -19,8 +19,11 @@ def _ignore_instruction(inst: IRInstruction) -> bool: ) -def _is_store(inst: IRInstruction) -> bool: - return inst.opcode == "store" +# must check if it has as operand as literal because +# there are cases when the store just moves value +# from one variable to another +def _is_correct_store(inst: IRInstruction) -> bool: + return inst.opcode == "store" and isinstance(inst.operands[0], IRLiteral) class LoopInvariantHoisting(IRPass): @@ -86,7 +89,7 @@ def _store_dependencies( for var in inst.get_input_variables(): source_inst = self.dfg.get_producing_instruction(var) assert isinstance(source_inst, IRInstruction) - if not _is_store(source_inst): + if not _is_correct_store(source_inst): continue for bb in self.loops[loop_idx]: if source_inst.parent == bb: @@ -113,7 +116,7 @@ def _dependent_in_bb(self, inst: IRInstruction, bb: IRBasicBlock): # ignores stores since all stores are independant # and can be always hoisted - if _is_store(source_ins): + if _is_correct_store(source_ins): continue if source_ins.parent == bb: