From 48a5da46b7c553d29181880e8ff6d51da62b2679 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 20 Sep 2024 10:20:09 -0400 Subject: [PATCH] feat[venom]: only stack_reorder before join points (#4247) fix a venom performance bug, where the stack would be reordered before all jump instructions. for "joining" jump instructions (where the target basic block can have multiple cfg inputs), the stack reorder is needed, since we need the invariant that the stack layout needs to be the same no matter which basic block we jump to the target basic block from. however, before jumping into a block with only a single `cfg_in` (which, after cfg normalization, is equivalent to "splitting" jump instructions `jnz` and `djmp`), the stack reorder is unneeded, since stack reordering happens anyways in the target basic block, and no invariant on the incoming stack layout is required. this commit changes the behavior so that stack reordering only occurs before these "joining" jump instructions. on branch-heavy code, this can improve codesize by as much as 25%, with corresponding gas improvement, especially in the presence of loops. --- .../codegen/features/test_constructor.py | 4 +++ vyper/venom/venom_to_assembly.py | 29 +++++++++++-------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/tests/functional/codegen/features/test_constructor.py b/tests/functional/codegen/features/test_constructor.py index 6cc7007bb2..3b86fe3460 100644 --- a/tests/functional/codegen/features/test_constructor.py +++ b/tests/functional/codegen/features/test_constructor.py @@ -1,4 +1,7 @@ +import pytest + from tests.evm_backends.base_env import _compile +from vyper.exceptions import StackTooDeep from vyper.utils import method_id @@ -166,6 +169,7 @@ def get_foo() -> uint256: assert c.get_foo() == 39 +@pytest.mark.venom_xfail(raises=StackTooDeep, reason="stack scheduler regression") def test_nested_dynamic_array_constructor_arg_2(env, get_contract): code = """ foo: int128 diff --git a/vyper/venom/venom_to_assembly.py b/vyper/venom/venom_to_assembly.py index eb8c4e69ec..41a76319d7 100644 --- a/vyper/venom/venom_to_assembly.py +++ b/vyper/venom/venom_to_assembly.py @@ -392,18 +392,23 @@ def _generate_evm_for_instruction( # Step 2: Emit instruction's input operands self._emit_input_operands(assembly, inst, operands, stack) - # Step 3: Reorder stack - if opcode in ["jnz", "djmp", "jmp"]: - # prepare stack for jump into another basic block - assert inst.parent and isinstance(inst.parent.cfg_out, OrderedSet) - b = next(iter(inst.parent.cfg_out)) - target_stack = self.liveness_analysis.input_vars_from(inst.parent, b) - # TODO optimize stack reordering at entry and exit from basic blocks - # NOTE: stack in general can contain multiple copies of the same variable, - # however we are safe in the case of jmp/djmp/jnz as it's not going to - # have multiples. - target_stack_list = list(target_stack) - self._stack_reorder(assembly, stack, target_stack_list) + # Step 3: Reorder stack before join points + if opcode == "jmp": + # prepare stack for jump into a join point + # we only need to reorder stack before join points, which after + # cfg normalization, join points can only be led into by + # jmp instructions. + assert isinstance(inst.parent.cfg_out, OrderedSet) + assert len(inst.parent.cfg_out) == 1 + next_bb = inst.parent.cfg_out.first() + + # guaranteed by cfg normalization+simplification + assert len(next_bb.cfg_in) > 1 + + target_stack = self.liveness_analysis.input_vars_from(inst.parent, next_bb) + # NOTE: in general the stack can contain multiple copies of + # the same variable, however, before a jump that is not possible + self._stack_reorder(assembly, stack, list(target_stack)) if opcode in COMMUTATIVE_INSTRUCTIONS: cost_no_swap = self._stack_reorder([], stack, operands, dry_run=True)