From 1fbc109386a7628bdf7a82e68b11548c5c5f7c4e Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 27 Aug 2024 19:51:52 +0200 Subject: [PATCH 01/22] fix[venom]: Invalid jump error --- vyper/venom/passes/remove_unused_variables.py | 2 +- vyper/venom/venom_to_assembly.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index be9c1ed535..f1740e4c05 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -31,7 +31,7 @@ def run_pass(self): def _process_instruction(self, inst): if inst.output is None: return - if inst.is_volatile or inst.is_bb_terminator: + if (inst.opcode != "mload" and inst.is_volatile) or inst.is_bb_terminator: return uses = self.dfg.get_uses(inst.output) if len(uses) > 0: diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 51fac10134..80ec2e38fd 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -537,7 +537,9 @@ def _generate_evm_for_instruction( # Step 6: Emit instructions output operands (if any) if inst.output is not None: - if "call" in inst.opcode and inst.output not in next_liveness: + if ( + "call" in inst.opcode or inst.opcode in ["mload"] + ) and inst.output not in next_liveness: self.pop(assembly, stack) elif inst.output in next_liveness: # peek at next_liveness to find the next scheduled item, From 0c57b4eb1e26cb18e6ba9a49f1627ae379f76423 Mon Sep 17 00:00:00 2001 From: plodeada Date: Wed, 28 Aug 2024 13:11:33 +0200 Subject: [PATCH 02/22] fix/better cleanup --- vyper/venom/venom_to_assembly.py | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 80ec2e38fd..ada8d9fd3b 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -147,6 +147,14 @@ def generate_evm(self, no_optimize: bool = False) -> list[str]: asm: list[Any] = [] top_asm = asm + def bb_cleanup_needed(bb : IRBasicBlock) -> bool: + for inst in bb.instructions: + # not sure if the exit is stricly necessery + # but I added it just to be sure + if inst.opcode in ["ret", "exit"]: + return True + return False + for ctx in self.ctxs: for fn in ctx.functions.values(): ac = IRAnalysesCache(fn) @@ -157,7 +165,9 @@ def generate_evm(self, no_optimize: bool = False) -> list[str]: assert fn.normalized, "Non-normalized CFG!" - self._generate_evm_for_basicblock_r(asm, fn.entry, StackModel()) + cleanup_needed = any(map(lambda x : bb_cleanup_needed(x), fn.get_basic_blocks())) + + self._generate_evm_for_basicblock_r(asm, fn.entry, StackModel(), cleanup_needed) # TODO make this property on IRFunction asm.extend(["_sym__ctor_exit", "JUMPDEST"]) @@ -273,7 +283,7 @@ def _emit_input_operands( emitted_ops.add(op) def _generate_evm_for_basicblock_r( - self, asm: list, basicblock: IRBasicBlock, stack: StackModel + self, asm: list, basicblock: IRBasicBlock, stack: StackModel, cleanup_needed : bool ) -> None: if basicblock in self.visited_basicblocks: return @@ -289,17 +299,17 @@ def _generate_evm_for_basicblock_r( main_insts = [inst for inst in basicblock.instructions if inst.opcode != "param"] for inst in param_insts: - asm.extend(self._generate_evm_for_instruction(inst, stack)) + asm.extend(self._generate_evm_for_instruction(inst, stack, False)) self._clean_unused_params(asm, basicblock, stack) for i, inst in enumerate(main_insts): next_liveness = main_insts[i + 1].liveness if i + 1 < len(main_insts) else OrderedSet() - asm.extend(self._generate_evm_for_instruction(inst, stack, next_liveness)) + asm.extend(self._generate_evm_for_instruction(inst, stack, cleanup_needed, next_liveness)) for bb in basicblock.reachable: - self._generate_evm_for_basicblock_r(asm, bb, stack.copy()) + self._generate_evm_for_basicblock_r(asm, bb, stack.copy(), cleanup_needed) def _clean_unused_params(self, asm: list, bb: IRBasicBlock, stack: StackModel) -> None: for i, inst in enumerate(bb.instructions): @@ -345,7 +355,7 @@ def clean_stack_from_cfg_in( self.pop(asm, stack) def _generate_evm_for_instruction( - self, inst: IRInstruction, stack: StackModel, next_liveness: OrderedSet = None + self, inst: IRInstruction, stack: StackModel, cleanup_needed : bool, next_liveness: OrderedSet = None ) -> list[str]: assembly: list[str | int] = [] next_liveness = next_liveness or OrderedSet() @@ -537,9 +547,7 @@ def _generate_evm_for_instruction( # Step 6: Emit instructions output operands (if any) if inst.output is not None: - if ( - "call" in inst.opcode or inst.opcode in ["mload"] - ) and inst.output not in next_liveness: + if cleanup_needed and inst.output not in next_liveness: self.pop(assembly, stack) elif inst.output in next_liveness: # peek at next_liveness to find the next scheduled item, From da617c7326e9dc0c0f6a63295bf31fbb46fa585d Mon Sep 17 00:00:00 2001 From: plodeada Date: Wed, 28 Aug 2024 13:20:40 +0200 Subject: [PATCH 03/22] fix/better cleanup (lint) --- vyper/venom/venom_to_assembly.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index ada8d9fd3b..118c31d028 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -147,7 +147,7 @@ def generate_evm(self, no_optimize: bool = False) -> list[str]: asm: list[Any] = [] top_asm = asm - def bb_cleanup_needed(bb : IRBasicBlock) -> bool: + def bb_cleanup_needed(bb: IRBasicBlock) -> bool: for inst in bb.instructions: # not sure if the exit is stricly necessery # but I added it just to be sure @@ -165,7 +165,7 @@ def bb_cleanup_needed(bb : IRBasicBlock) -> bool: assert fn.normalized, "Non-normalized CFG!" - cleanup_needed = any(map(lambda x : bb_cleanup_needed(x), fn.get_basic_blocks())) + cleanup_needed = any(map(lambda x: bb_cleanup_needed(x), fn.get_basic_blocks())) self._generate_evm_for_basicblock_r(asm, fn.entry, StackModel(), cleanup_needed) @@ -283,7 +283,7 @@ def _emit_input_operands( emitted_ops.add(op) def _generate_evm_for_basicblock_r( - self, asm: list, basicblock: IRBasicBlock, stack: StackModel, cleanup_needed : bool + self, asm: list, basicblock: IRBasicBlock, stack: StackModel, cleanup_needed: bool ) -> None: if basicblock in self.visited_basicblocks: return @@ -306,7 +306,9 @@ def _generate_evm_for_basicblock_r( for i, inst in enumerate(main_insts): next_liveness = main_insts[i + 1].liveness if i + 1 < len(main_insts) else OrderedSet() - asm.extend(self._generate_evm_for_instruction(inst, stack, cleanup_needed, next_liveness)) + asm.extend( + self._generate_evm_for_instruction(inst, stack, cleanup_needed, next_liveness) + ) for bb in basicblock.reachable: self._generate_evm_for_basicblock_r(asm, bb, stack.copy(), cleanup_needed) @@ -355,7 +357,11 @@ def clean_stack_from_cfg_in( self.pop(asm, stack) def _generate_evm_for_instruction( - self, inst: IRInstruction, stack: StackModel, cleanup_needed : bool, next_liveness: OrderedSet = None + self, + inst: IRInstruction, + stack: StackModel, + cleanup_needed: bool, + next_liveness: OrderedSet = None, ) -> list[str]: assembly: list[str | int] = [] next_liveness = next_liveness or OrderedSet() From 74817f322aed67c341e2ea12b8e9a2ae7db31ef9 Mon Sep 17 00:00:00 2001 From: plodeada Date: Wed, 28 Aug 2024 14:18:28 +0200 Subject: [PATCH 04/22] fix/cleanup test --- tests/unit/compiler/venom/test_stack_cleanup.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 tests/unit/compiler/venom/test_stack_cleanup.py diff --git a/tests/unit/compiler/venom/test_stack_cleanup.py b/tests/unit/compiler/venom/test_stack_cleanup.py new file mode 100644 index 0000000000..864f11b4d9 --- /dev/null +++ b/tests/unit/compiler/venom/test_stack_cleanup.py @@ -0,0 +1,15 @@ +from vyper.venom.context import IRContext +from vyper.venom import generate_assembly_experimental +from vyper.compiler.settings import OptimizationLevel + + +def test_cleanup_stack(): + ctx = IRContext() + fn = ctx.create_function("test") + bb = fn.get_basic_block() + op = bb.append_instruction("store", 10) + bb.append_instruction("add", op, op) + bb.append_instruction("ret") + + asm = generate_assembly_experimental(ctx, optimize=OptimizationLevel.GAS) + assert asm == ["PUSH1", 10, "DUP1", "ADD", "POP", "JUMP"] From 7c269cf03fbcf167b12d172a9e569da15464f112 Mon Sep 17 00:00:00 2001 From: plodeada Date: Wed, 28 Aug 2024 14:32:18 +0200 Subject: [PATCH 05/22] fix/cleanup test (lint) --- tests/unit/compiler/venom/test_stack_cleanup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/compiler/venom/test_stack_cleanup.py b/tests/unit/compiler/venom/test_stack_cleanup.py index 864f11b4d9..0842d16173 100644 --- a/tests/unit/compiler/venom/test_stack_cleanup.py +++ b/tests/unit/compiler/venom/test_stack_cleanup.py @@ -1,6 +1,6 @@ -from vyper.venom.context import IRContext -from vyper.venom import generate_assembly_experimental from vyper.compiler.settings import OptimizationLevel +from vyper.venom import generate_assembly_experimental +from vyper.venom.context import IRContext def test_cleanup_stack(): From 37314035bea5d7fe19a6f7a48327e18a98ac9b5b Mon Sep 17 00:00:00 2001 From: plodeada Date: Wed, 28 Aug 2024 15:02:33 +0200 Subject: [PATCH 06/22] fix/correction remove_unused_variables pass --- vyper/venom/passes/remove_unused_variables.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/venom/passes/remove_unused_variables.py b/vyper/venom/passes/remove_unused_variables.py index f1740e4c05..be9c1ed535 100644 --- a/vyper/venom/passes/remove_unused_variables.py +++ b/vyper/venom/passes/remove_unused_variables.py @@ -31,7 +31,7 @@ def run_pass(self): def _process_instruction(self, inst): if inst.output is None: return - if (inst.opcode != "mload" and inst.is_volatile) or inst.is_bb_terminator: + if inst.is_volatile or inst.is_bb_terminator: return uses = self.dfg.get_uses(inst.output) if len(uses) > 0: From 8c7f8a7955826e1e76a000e5dfe44a01e55266a8 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 3 Sep 2024 10:32:22 +0200 Subject: [PATCH 07/22] param check added --- vyper/venom/venom_to_assembly.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 118c31d028..76e46add09 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -553,7 +553,7 @@ def _generate_evm_for_instruction( # Step 6: Emit instructions output operands (if any) if inst.output is not None: - if cleanup_needed and inst.output not in next_liveness: + if cleanup_needed and opcode != "param" and inst.output not in next_liveness: self.pop(assembly, stack) elif inst.output in next_liveness: # peek at next_liveness to find the next scheduled item, From da779ab0164f2b4d480bc7c30f71a0e7487d0e2d Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 3 Sep 2024 14:30:10 +0200 Subject: [PATCH 08/22] fix[venom]: small improvement --- tests/unit/compiler/venom/test_stack_cleanup.py | 3 ++- vyper/venom/venom_to_assembly.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unit/compiler/venom/test_stack_cleanup.py b/tests/unit/compiler/venom/test_stack_cleanup.py index 0842d16173..6015cf1c41 100644 --- a/tests/unit/compiler/venom/test_stack_cleanup.py +++ b/tests/unit/compiler/venom/test_stack_cleanup.py @@ -7,9 +7,10 @@ def test_cleanup_stack(): ctx = IRContext() fn = ctx.create_function("test") bb = fn.get_basic_block() + ret_val = bb.append_instruction("param") op = bb.append_instruction("store", 10) bb.append_instruction("add", op, op) - bb.append_instruction("ret") + bb.append_instruction("ret", ret_val) asm = generate_assembly_experimental(ctx, optimize=OptimizationLevel.GAS) assert asm == ["PUSH1", 10, "DUP1", "ADD", "POP", "JUMP"] diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 76e46add09..726dca9eba 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -299,7 +299,7 @@ def _generate_evm_for_basicblock_r( main_insts = [inst for inst in basicblock.instructions if inst.opcode != "param"] for inst in param_insts: - asm.extend(self._generate_evm_for_instruction(inst, stack, False)) + asm.extend(self._generate_evm_for_instruction(inst, stack, cleanup_needed)) self._clean_unused_params(asm, basicblock, stack) From d7a42326b596bd5b8ed6c66a992f23904ea26b6c Mon Sep 17 00:00:00 2001 From: HodanPlodky <36966616+HodanPlodky@users.noreply.github.com> Date: Tue, 10 Sep 2024 08:13:03 +0000 Subject: [PATCH 09/22] Update vyper/venom/venom_to_assembly.py - replace the map with for comprehensions Co-authored-by: Charles Cooper --- vyper/venom/venom_to_assembly.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index c486f492f6..4db6250b6e 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -165,7 +165,7 @@ def bb_cleanup_needed(bb: IRBasicBlock) -> bool: assert fn.normalized, "Non-normalized CFG!" - cleanup_needed = any(map(lambda x: bb_cleanup_needed(x), fn.get_basic_blocks())) + cleanup_needed = any((bb_cleanup_needed(bb) for bb in fn.get_basic_blocks())) self._generate_evm_for_basicblock_r(asm, fn.entry, StackModel(), cleanup_needed) From ee567ada195e7825c12948344db520a6f9bdddc2 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 10 Sep 2024 13:57:57 +0200 Subject: [PATCH 10/22] clean_unused_params cleanup --- vyper/venom/venom_to_assembly.py | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 726dca9eba..ee093c35f0 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -165,7 +165,7 @@ def bb_cleanup_needed(bb: IRBasicBlock) -> bool: assert fn.normalized, "Non-normalized CFG!" - cleanup_needed = any(map(lambda x: bb_cleanup_needed(x), fn.get_basic_blocks())) + cleanup_needed = any((bb_cleanup_needed(bb) for bb in fn.get_basic_blocks())) self._generate_evm_for_basicblock_r(asm, fn.entry, StackModel(), cleanup_needed) @@ -298,10 +298,13 @@ def _generate_evm_for_basicblock_r( param_insts = [inst for inst in basicblock.instructions if inst.opcode == "param"] main_insts = [inst for inst in basicblock.instructions if inst.opcode != "param"] - for inst in param_insts: - asm.extend(self._generate_evm_for_instruction(inst, stack, cleanup_needed)) - - self._clean_unused_params(asm, basicblock, stack) + for i, inst in enumerate(param_insts): + next_liveness = ( + param_insts[i + 1].liveness if i + 1 < len(param_insts) else main_insts[0].liveness + ) + asm.extend( + self._generate_evm_for_instruction(inst, stack, cleanup_needed, next_liveness) + ) for i, inst in enumerate(main_insts): next_liveness = main_insts[i + 1].liveness if i + 1 < len(main_insts) else OrderedSet() @@ -313,18 +316,6 @@ def _generate_evm_for_basicblock_r( for bb in basicblock.reachable: self._generate_evm_for_basicblock_r(asm, bb, stack.copy(), cleanup_needed) - def _clean_unused_params(self, asm: list, bb: IRBasicBlock, stack: StackModel) -> None: - for i, inst in enumerate(bb.instructions): - if inst.opcode != "param": - break - if inst.is_volatile and i + 1 < len(bb.instructions): - liveness = bb.instructions[i + 1].liveness - if inst.output is not None and inst.output not in liveness: - depth = stack.get_depth(inst.output) - if depth != 0: - self.swap(asm, stack, depth) - self.pop(asm, stack) - # pop values from stack at entry to bb # note this produces the same result(!) no matter which basic block # we enter from in the CFG. @@ -553,7 +544,10 @@ def _generate_evm_for_instruction( # Step 6: Emit instructions output operands (if any) if inst.output is not None: - if cleanup_needed and opcode != "param" and inst.output not in next_liveness: + if cleanup_needed and inst.output not in next_liveness: + depth = stack.get_depth(inst.output) + if depth != 0: + self.swap(assembly, stack, depth) self.pop(assembly, stack) elif inst.output in next_liveness: # peek at next_liveness to find the next scheduled item, From 977e147554f5edd3cc20a803cbb50ef117aaf6a0 Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 10 Sep 2024 15:25:50 +0200 Subject: [PATCH 11/22] changes according to review - for merge --- vyper/venom/venom_to_assembly.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 1e4cd63554..1c824abbb5 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -295,19 +295,12 @@ def _generate_evm_for_basicblock_r( self.clean_stack_from_cfg_in(asm, basicblock, stack) - param_insts = [inst for inst in basicblock.instructions if inst.opcode == "param"] - main_insts = [inst for inst in basicblock.instructions if inst.opcode != "param"] + param_insts = (inst for inst in basicblock.instructions if inst.opcode == "param") + main_insts = (inst for inst in basicblock.instructions if inst.opcode != "param") + all_inst = [x for one_iter in (param_insts, main_insts) for x in one_iter] - for i, inst in enumerate(param_insts): - next_liveness = ( - param_insts[i + 1].liveness if i + 1 < len(param_insts) else main_insts[0].liveness - ) - asm.extend( - self._generate_evm_for_instruction(inst, stack, cleanup_needed, next_liveness) - ) - - for i, inst in enumerate(main_insts): - next_liveness = main_insts[i + 1].liveness if i + 1 < len(main_insts) else OrderedSet() + for i, inst in enumerate(all_inst): + next_liveness = all_inst[i + 1].liveness if i + 1 < len(all_inst) else OrderedSet() asm.extend( self._generate_evm_for_instruction(inst, stack, cleanup_needed, next_liveness) From 2add3d7b5164d652303d6817c4174fbff015707f Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 10 Sep 2024 15:47:23 +0200 Subject: [PATCH 12/22] changes according to review - renamed all_inst to all_insts --- vyper/venom/venom_to_assembly.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 1c824abbb5..def05ed84e 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -297,10 +297,10 @@ def _generate_evm_for_basicblock_r( param_insts = (inst for inst in basicblock.instructions if inst.opcode == "param") main_insts = (inst for inst in basicblock.instructions if inst.opcode != "param") - all_inst = [x for one_iter in (param_insts, main_insts) for x in one_iter] + all_insts = [x for one_iter in (param_insts, main_insts) for x in one_iter] - for i, inst in enumerate(all_inst): - next_liveness = all_inst[i + 1].liveness if i + 1 < len(all_inst) else OrderedSet() + for i, inst in enumerate(all_insts): + next_liveness = all_insts[i + 1].liveness if i + 1 < len(all_insts) else OrderedSet() asm.extend( self._generate_evm_for_instruction(inst, stack, cleanup_needed, next_liveness) From b4c8b7ac21d0e2c2e740503bb7b8db0fc4e47afd Mon Sep 17 00:00:00 2001 From: Hodan Date: Tue, 10 Sep 2024 16:11:50 +0200 Subject: [PATCH 13/22] changes according to review - simplify the all_insts creation --- vyper/venom/venom_to_assembly.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index def05ed84e..60be69008d 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -295,9 +295,7 @@ def _generate_evm_for_basicblock_r( self.clean_stack_from_cfg_in(asm, basicblock, stack) - param_insts = (inst for inst in basicblock.instructions if inst.opcode == "param") - main_insts = (inst for inst in basicblock.instructions if inst.opcode != "param") - all_insts = [x for one_iter in (param_insts, main_insts) for x in one_iter] + all_insts = sorted(basicblock.instructions, key=lambda x: x.opcode != "param") for i, inst in enumerate(all_insts): next_liveness = all_insts[i + 1].liveness if i + 1 < len(all_insts) else OrderedSet() From 326adfd4c81f8748c885a8ba693f9d08f30020ef Mon Sep 17 00:00:00 2001 From: Hodan Date: Wed, 11 Sep 2024 21:22:34 +0200 Subject: [PATCH 14/22] assert that the depth is always zero --- vyper/venom/venom_to_assembly.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 60be69008d..f55ed13dfc 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -543,8 +543,7 @@ def _generate_evm_for_instruction( if inst.output is not None: if cleanup_needed and inst.output not in next_liveness: depth = stack.get_depth(inst.output) - if depth != 0: - self.swap(assembly, stack, depth) + assert depth == 0, "Depth cannot be 0" self.pop(assembly, stack) elif inst.output in next_liveness: # peek at next_liveness to find the next scheduled item, From 84a4f7d2c52632f5d7acde5c4ecb4227a8981e2d Mon Sep 17 00:00:00 2001 From: HodanPlodky <36966616+HodanPlodky@users.noreply.github.com> Date: Thu, 12 Sep 2024 07:43:11 +0000 Subject: [PATCH 15/22] Update vyper/venom/venom_to_assembly.py - Incorrect assert message fix Co-authored-by: Charles Cooper --- vyper/venom/venom_to_assembly.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index f55ed13dfc..59e44b0de1 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -543,7 +543,7 @@ def _generate_evm_for_instruction( if inst.output is not None: if cleanup_needed and inst.output not in next_liveness: depth = stack.get_depth(inst.output) - assert depth == 0, "Depth cannot be 0" + assert depth == 0, "Depth must be 0" self.pop(assembly, stack) elif inst.output in next_liveness: # peek at next_liveness to find the next scheduled item, From 9776544b536c313d9c69bc09770f79a33c2c48f4 Mon Sep 17 00:00:00 2001 From: Hodan Date: Thu, 12 Sep 2024 17:45:27 +0200 Subject: [PATCH 16/22] moved the cleanup check into the IRFunction class --- vyper/venom/function.py | 8 ++++++++ vyper/venom/venom_to_assembly.py | 14 +++----------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/vyper/venom/function.py b/vyper/venom/function.py index fb0dabc99a..128904a967 100644 --- a/vyper/venom/function.py +++ b/vyper/venom/function.py @@ -179,6 +179,14 @@ def pop_source(self): assert len(self._error_msg_stack) > 0, "Empty error stack" self._error_msg_stack.pop() + def is_cleanup_needed(self) -> bool: + def _bb_cleanup_needed(bb: IRBasicBlock) -> bool: + if bb.is_terminated: + return bb.instructions[-1].opcode == "ret" + return False + + return any(_bb_cleanup_needed(bb) for bb in self.get_basic_blocks()) + @property def ast_source(self) -> Optional[IRnode]: return self._ast_source_stack[-1] if len(self._ast_source_stack) > 0 else None diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 59e44b0de1..ccc54f1537 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -147,14 +147,6 @@ def generate_evm(self, no_optimize: bool = False) -> list[str]: asm: list[Any] = [] top_asm = asm - def bb_cleanup_needed(bb: IRBasicBlock) -> bool: - for inst in bb.instructions: - # not sure if the exit is stricly necessery - # but I added it just to be sure - if inst.opcode in ["ret", "exit"]: - return True - return False - for ctx in self.ctxs: for fn in ctx.functions.values(): ac = IRAnalysesCache(fn) @@ -165,9 +157,9 @@ def bb_cleanup_needed(bb: IRBasicBlock) -> bool: assert fn.normalized, "Non-normalized CFG!" - cleanup_needed = any((bb_cleanup_needed(bb) for bb in fn.get_basic_blocks())) - - self._generate_evm_for_basicblock_r(asm, fn.entry, StackModel(), cleanup_needed) + self._generate_evm_for_basicblock_r( + asm, fn.entry, StackModel(), fn.is_cleanup_needed() + ) # TODO make this property on IRFunction asm.extend(["_sym__ctor_exit", "JUMPDEST"]) From 7c0f33ae27dc00cbab37b19448ee8455fc981c4e Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 12 Sep 2024 18:07:35 -0400 Subject: [PATCH 17/22] small refactor --- vyper/venom/function.py | 8 -------- vyper/venom/venom_to_assembly.py | 17 +++++++++++++---- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/vyper/venom/function.py b/vyper/venom/function.py index 128904a967..fb0dabc99a 100644 --- a/vyper/venom/function.py +++ b/vyper/venom/function.py @@ -179,14 +179,6 @@ def pop_source(self): assert len(self._error_msg_stack) > 0, "Empty error stack" self._error_msg_stack.pop() - def is_cleanup_needed(self) -> bool: - def _bb_cleanup_needed(bb: IRBasicBlock) -> bool: - if bb.is_terminated: - return bb.instructions[-1].opcode == "ret" - return False - - return any(_bb_cleanup_needed(bb) for bb in self.get_basic_blocks()) - @property def ast_source(self) -> Optional[IRnode]: return self._ast_source_stack[-1] if len(self._ast_source_stack) > 0 else None diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index ccc54f1537..c9d8f0867f 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -1,5 +1,5 @@ from collections import Counter -from typing import Any +from typing import TYPE_CHECKING, Any from vyper.exceptions import CompilerPanic, StackTooDeep from vyper.ir.compile_ir import ( @@ -26,6 +26,9 @@ from vyper.venom.passes.normalization import NormalizationPass from vyper.venom.stack_model import StackModel +if TYPE_CHECKING: + from vyper.venom.function import IRFunction + # instructions which map one-to-one from venom to EVM _ONE_TO_ONE_INSTRUCTIONS = frozenset( [ @@ -118,6 +121,13 @@ def apply_line_numbers(inst: IRInstruction, asm) -> list[str]: return ret # type: ignore +def _is_cleanup_needed(fn: IRFunction) -> bool: + for bb in fn.get_basic_blocks(): + if bb.is_terminated and bb.instructions[-1].opcode == "ret": + return True + return False + + # TODO: "assembly" gets into the recursion due to how the original # IR was structured recursively in regards with the deploy instruction. # There, recursing into the deploy instruction was by design, and @@ -157,9 +167,8 @@ def generate_evm(self, no_optimize: bool = False) -> list[str]: assert fn.normalized, "Non-normalized CFG!" - self._generate_evm_for_basicblock_r( - asm, fn.entry, StackModel(), fn.is_cleanup_needed() - ) + cleanup_needed = _is_cleanup_needed(fn) + self._generate_evm_for_basicblock_r(asm, fn.entry, StackModel(), cleanup_needed) # TODO make this property on IRFunction asm.extend(["_sym__ctor_exit", "JUMPDEST"]) From ff3444c8c8fd802ab70ef8e6e5622edbc1f4b5b1 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 12 Sep 2024 18:10:23 -0400 Subject: [PATCH 18/22] fix import --- vyper/venom/venom_to_assembly.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index c9d8f0867f..aeb482afb4 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -1,5 +1,5 @@ from collections import Counter -from typing import TYPE_CHECKING, Any +from typing import Any from vyper.exceptions import CompilerPanic, StackTooDeep from vyper.ir.compile_ir import ( @@ -23,12 +23,10 @@ IRVariable, ) from vyper.venom.context import IRContext +from vyper.venom.function import IRFunction from vyper.venom.passes.normalization import NormalizationPass from vyper.venom.stack_model import StackModel -if TYPE_CHECKING: - from vyper.venom.function import IRFunction - # instructions which map one-to-one from venom to EVM _ONE_TO_ONE_INSTRUCTIONS = frozenset( [ From cb35c54800fa396ddf0d58ec2945f158f7f9ccf4 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 12 Sep 2024 18:53:36 -0400 Subject: [PATCH 19/22] always pop --- vyper/venom/venom_to_assembly.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index aeb482afb4..25e4a6a45f 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -540,13 +540,12 @@ def _generate_evm_for_instruction( # Step 6: Emit instructions output operands (if any) if inst.output is not None: - if cleanup_needed and inst.output not in next_liveness: - depth = stack.get_depth(inst.output) - assert depth == 0, "Depth must be 0" + if inst.output not in next_liveness: self.pop(assembly, stack) - elif inst.output in next_liveness: + else: # peek at next_liveness to find the next scheduled item, # and optimistically swap with it + # TODO: implement OrderedSet.last() next_scheduled = list(next_liveness)[-1] self.swap_op(assembly, stack, next_scheduled) From bdf20b0e59a216f33599d1fd3f5351572538cb5d Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 13 Sep 2024 09:56:32 -0400 Subject: [PATCH 20/22] fix instructions length check --- vyper/venom/venom_to_assembly.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 25e4a6a45f..4d924d9043 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -121,7 +121,7 @@ def apply_line_numbers(inst: IRInstruction, asm) -> list[str]: def _is_cleanup_needed(fn: IRFunction) -> bool: for bb in fn.get_basic_blocks(): - if bb.is_terminated and bb.instructions[-1].opcode == "ret": + if len(bb.instructions) > 0 and bb.instructions[-1].opcode == "ret": return True return False From 8b8dda73636874a2b09b6790d175a26faf472c51 Mon Sep 17 00:00:00 2001 From: Hodan Date: Fri, 13 Sep 2024 17:57:27 +0200 Subject: [PATCH 21/22] adjusted the test_duplicate_operands test --- tests/unit/compiler/venom/test_duplicate_operands.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/compiler/venom/test_duplicate_operands.py b/tests/unit/compiler/venom/test_duplicate_operands.py index 44c4ed0404..fbff0835d2 100644 --- a/tests/unit/compiler/venom/test_duplicate_operands.py +++ b/tests/unit/compiler/venom/test_duplicate_operands.py @@ -13,7 +13,7 @@ def test_duplicate_operands(): %3 = mul %1, %2 stop - Should compile to: [PUSH1, 10, DUP1, DUP1, DUP1, ADD, MUL, STOP] + Should compile to: [PUSH1, 10, DUP1, DUP1, DUP1, ADD, MUL, POP, STOP] """ ctx = IRContext() fn = ctx.create_function("test") @@ -24,4 +24,4 @@ def test_duplicate_operands(): bb.append_instruction("stop") asm = generate_assembly_experimental(ctx, optimize=OptimizationLevel.GAS) - assert asm == ["PUSH1", 10, "DUP1", "DUP1", "ADD", "MUL", "STOP"] + assert asm == ["PUSH1", 10, "DUP1", "DUP1", "ADD", "MUL", "POP", "STOP"] From 3536dc04755b3d6532444106bc51eb6705491ad2 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 16 Sep 2024 09:22:45 -0400 Subject: [PATCH 22/22] remove cleanup_needed --- vyper/venom/venom_to_assembly.py | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index 4d924d9043..c087d29bff 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -23,7 +23,6 @@ IRVariable, ) from vyper.venom.context import IRContext -from vyper.venom.function import IRFunction from vyper.venom.passes.normalization import NormalizationPass from vyper.venom.stack_model import StackModel @@ -119,13 +118,6 @@ def apply_line_numbers(inst: IRInstruction, asm) -> list[str]: return ret # type: ignore -def _is_cleanup_needed(fn: IRFunction) -> bool: - for bb in fn.get_basic_blocks(): - if len(bb.instructions) > 0 and bb.instructions[-1].opcode == "ret": - return True - return False - - # TODO: "assembly" gets into the recursion due to how the original # IR was structured recursively in regards with the deploy instruction. # There, recursing into the deploy instruction was by design, and @@ -165,8 +157,7 @@ def generate_evm(self, no_optimize: bool = False) -> list[str]: assert fn.normalized, "Non-normalized CFG!" - cleanup_needed = _is_cleanup_needed(fn) - self._generate_evm_for_basicblock_r(asm, fn.entry, StackModel(), cleanup_needed) + self._generate_evm_for_basicblock_r(asm, fn.entry, StackModel()) # TODO make this property on IRFunction asm.extend(["_sym__ctor_exit", "JUMPDEST"]) @@ -282,7 +273,7 @@ def _emit_input_operands( emitted_ops.add(op) def _generate_evm_for_basicblock_r( - self, asm: list, basicblock: IRBasicBlock, stack: StackModel, cleanup_needed: bool + self, asm: list, basicblock: IRBasicBlock, stack: StackModel ) -> None: if basicblock in self.visited_basicblocks: return @@ -299,12 +290,10 @@ def _generate_evm_for_basicblock_r( for i, inst in enumerate(all_insts): next_liveness = all_insts[i + 1].liveness if i + 1 < len(all_insts) else OrderedSet() - asm.extend( - self._generate_evm_for_instruction(inst, stack, cleanup_needed, next_liveness) - ) + asm.extend(self._generate_evm_for_instruction(inst, stack, next_liveness)) for bb in basicblock.reachable: - self._generate_evm_for_basicblock_r(asm, bb, stack.copy(), cleanup_needed) + self._generate_evm_for_basicblock_r(asm, bb, stack.copy()) # pop values from stack at entry to bb # note this produces the same result(!) no matter which basic block @@ -338,11 +327,7 @@ def clean_stack_from_cfg_in( self.pop(asm, stack) def _generate_evm_for_instruction( - self, - inst: IRInstruction, - stack: StackModel, - cleanup_needed: bool, - next_liveness: OrderedSet = None, + self, inst: IRInstruction, stack: StackModel, next_liveness: OrderedSet = None ) -> list[str]: assembly: list[str | int] = [] next_liveness = next_liveness or OrderedSet()