Skip to content

Commit

Permalink
fix[venom]: fix eval of exp in sccp (#4009)
Browse files Browse the repository at this point in the history
fix `exp` evaluation in sccp. the current implementation is incorrect,
which was found by enabling the `venom + -opt-none` pipeline in the CI.
`evm_pow` is correct, as it preserves the correct edge cases when
base/exponent are 0 or 1 (all combinations).

this commit also enables the `venom + -opt-none` and
`venom + -opt-codesize` jobs in the CI, ensuring higher coverage of the
venom pipeline going forward.
  • Loading branch information
charles-cooper authored May 8, 2024
1 parent 35996f1 commit f213655
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 18 deletions.
15 changes: 14 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,20 @@ jobs:
evm-version: shanghai
evm-backend: revm
experimental-codegen: true
# TODO: test experimental_codegen + -Ocodesize

- python-version: ["3.11", "311"]
opt-mode: codesize
debug: false
evm-version: shanghai
evm-backend: revm
experimental-codegen: true

- python-version: ["3.11", "311"]
opt-mode: none
debug: false
evm-version: shanghai
evm-backend: revm
experimental-codegen: true

# run across other python versions. we don't really need to run all
# modes across all python versions - one is enough
Expand Down
9 changes: 7 additions & 2 deletions tests/unit/compiler/test_source_map.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,19 @@ def foo(a: uint256) -> int128:
"""


def test_jump_map(optimize):
def test_jump_map(optimize, experimental_codegen):
source_map = compile_code(TEST_CODE, output_formats=["source_map"])["source_map"]
pos_map = source_map["pc_pos_map"]
jump_map = source_map["pc_jump_map"]

expected_jumps = 1
if optimize == OptimizationLevel.NONE:
expected_jumps = 3 # some jumps get optimized out when optimizer is on
# some jumps which don't get optimized out when optimizer is off
# (slightly different behavior depending if venom pipeline is enabled):
if not experimental_codegen:
expected_jumps = 3
else:
expected_jumps = 2

assert len([v for v in jump_map.values() if v == "o"]) == expected_jumps
assert len([v for v in jump_map.values() if v == "i"]) == 2
Expand Down
1 change: 1 addition & 0 deletions vyper/ast/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1104,6 +1104,7 @@ def _op(self, left, right):
# r > ln(2 ** 256) / ln(l)
if right > math.log(decimal.Decimal(2**257)) / math.log(decimal.Decimal(left)):
raise InvalidLiteral("Out of bounds", self)

return int(left**right)


Expand Down
29 changes: 14 additions & 15 deletions vyper/venom/passes/sccp/eval.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
import operator
from typing import Callable

from vyper.utils import SizeLimits, evm_div, evm_mod, signed_to_unsigned, unsigned_to_signed
from vyper.utils import (
SizeLimits,
evm_div,
evm_mod,
evm_pow,
signed_to_unsigned,
unsigned_to_signed,
)
from vyper.venom.basicblock import IROperand


Expand Down Expand Up @@ -30,9 +37,11 @@ def wrapper(ops: list[IROperand]) -> int:

def _wrap_binop(operation):
def wrapper(ops: list[IROperand]) -> int:
first = ops[1].value
second = ops[0].value
return (int(operation(first, second))) & SizeLimits.MAX_UINT256
first = _signed_to_unsigned(ops[1].value)
second = _signed_to_unsigned(ops[0].value)
ret = operation(first, second)
assert isinstance(ret, int)
return ret & SizeLimits.MAX_UINT256

return wrapper

Expand Down Expand Up @@ -90,16 +99,6 @@ def _evm_not(ops: list[IROperand]) -> int:
return SizeLimits.MAX_UINT256 ^ value


def _evm_exp(ops: list[IROperand]) -> int:
base = ops[1].value
exponent = ops[0].value

if base == 0:
return 0

return pow(base, exponent, SizeLimits.CEILING_UINT256)


ARITHMETIC_OPS: dict[str, Callable[[list[IROperand]], int]] = {
"add": _wrap_binop(operator.add),
"sub": _wrap_binop(operator.sub),
Expand All @@ -108,7 +107,7 @@ def _evm_exp(ops: list[IROperand]) -> int:
"sdiv": _wrap_signed_binop(evm_div),
"mod": _wrap_binop(evm_mod),
"smod": _wrap_signed_binop(evm_mod),
"exp": _evm_exp,
"exp": _wrap_binop(evm_pow),
"eq": _wrap_binop(operator.eq),
"ne": _wrap_binop(operator.ne),
"lt": _wrap_binop(operator.lt),
Expand Down

0 comments on commit f213655

Please sign in to comment.