Skip to content

Commit

Permalink
feat[venom]: only stack_reorder before join points (#4247)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
charles-cooper authored Sep 20, 2024
1 parent d76d750 commit 48a5da4
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 12 deletions.
4 changes: 4 additions & 0 deletions tests/functional/codegen/features/test_constructor.py
Original file line number Diff line number Diff line change
@@ -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


Expand Down Expand Up @@ -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
Expand Down
29 changes: 17 additions & 12 deletions vyper/venom/venom_to_assembly.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 48a5da4

Please sign in to comment.