From 8dbefe2457da7d66b3ad9fc67572a45221e05714 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sun, 5 May 2024 10:05:29 -0400 Subject: [PATCH 01/12] fix[venom]: fix branch eliminator cases in sccp in sccp, when the operand of `jnz` or `assert` is already an IRLiteral (this is most easily seen by disabling the IRnode branch eliminator), the compiler will panic. --- tests/unit/compiler/venom/test_sccp.py | 72 +++++++++++++++++++++++++- vyper/venom/passes/sccp/sccp.py | 20 +++++-- 2 files changed, 87 insertions(+), 5 deletions(-) diff --git a/tests/unit/compiler/venom/test_sccp.py b/tests/unit/compiler/venom/test_sccp.py index 37a8bc9000..e65839136e 100644 --- a/tests/unit/compiler/venom/test_sccp.py +++ b/tests/unit/compiler/venom/test_sccp.py @@ -1,5 +1,8 @@ +import pytest + +from vyper.exceptions import StaticAssertionException from vyper.venom.analysis.analysis import IRAnalysesCache -from vyper.venom.basicblock import IRBasicBlock, IRLabel, IRVariable +from vyper.venom.basicblock import IRBasicBlock, IRLabel, IRLiteral, IRVariable from vyper.venom.context import IRContext from vyper.venom.passes.make_ssa import MakeSSA from vyper.venom.passes.sccp import SCCP @@ -28,6 +31,73 @@ def test_simple_case(): assert sccp.lattice[IRVariable("%4")].value == 96 +def test_branch_eliminator_simple(): + ctx = IRContext() + fn = ctx.create_function("_global") + + bb1 = fn.get_basic_block() + + br1 = IRBasicBlock(IRLabel("then"), fn) + br1.append_instruction("stop") + br2 = IRBasicBlock(IRLabel("else"), fn) + br2.append_instruction("jmp", IRLabel("foo")) + + fn.append_basic_block(br1) + fn.append_basic_block(br2) + + bb1.append_instruction("jnz", IRLiteral(1), br1.label, br2.label) + + bb2 = IRBasicBlock(IRLabel("foo"), fn) + bb2.append_instruction("jnz", IRLiteral(0), br1.label, br2.label) + fn.append_basic_block(bb2) + + ac = IRAnalysesCache(fn) + MakeSSA(ac, fn).run_pass() + sccp = SCCP(ac, fn) + sccp.run_pass() + + assert bb1.instructions[-1].opcode == "jmp" + assert bb1.instructions[-1].operands == [br1.label] + assert bb2.instructions[-1].opcode == "jmp" + assert bb2.instructions[-1].operands == [br2.label] + + +def test_assert_elimination(): + ctx = IRContext() + fn = ctx.create_function("_global") + + bb = fn.get_basic_block() + + bb.append_instruction("assert", IRLiteral(1)) + bb.append_instruction("assert_unreachable", IRLiteral(1)) + bb.append_instruction("stop") + + ac = IRAnalysesCache(fn) + MakeSSA(ac, fn).run_pass() + sccp = SCCP(ac, fn) + sccp.run_pass() + + for inst in bb.instructions[:-1]: + assert inst.opcode == "nop" + + +@pytest.mark.parametrize("asserter", ("assert", "assert_unreachable")) +def test_assert_false(asserter): + ctx = IRContext() + fn = ctx.create_function("_global") + + bb = fn.get_basic_block() + + bb.append_instruction(asserter, IRLiteral(0)) + bb.append_instruction("stop") + + ac = IRAnalysesCache(fn) + MakeSSA(ac, fn).run_pass() + sccp = SCCP(ac, fn) + with pytest.raises(StaticAssertionException): + sccp.run_pass() + + def test_cont_jump_case(): ctx = IRContext() fn = ctx.create_function("_global") diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index 7f3fc7e03e..47b276ad29 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -153,6 +153,7 @@ def _visit_phi(self, inst: IRInstruction): in_vars.append(self.lattice[var]) value = reduce(_meet, in_vars, LatticeEnum.TOP) # type: ignore assert inst.output in self.lattice, "Got undefined var for phi" + if value != self.lattice[inst.output]: self.lattice[inst.output] = value self._add_ssa_work_items(inst) @@ -169,7 +170,8 @@ def _visit_expr(self, inst: IRInstruction): target = self.fn.get_basic_block(inst.operands[0].value) self.work_list.append(FlowWorkItem(inst.parent, target)) elif opcode == "jnz": - lat = self.lattice[inst.operands[0]] + lat = _get_lattice(self.lattice, inst.operands[0]) + assert lat != LatticeEnum.TOP, f"Got undefined var at jmp at {inst.parent}" if lat == LatticeEnum.BOTTOM: for out_bb in inst.parent.cfg_out: @@ -276,7 +278,8 @@ def _replace_constants(self, inst: IRInstruction, lattice: Lattice): case of jumps and asserts as needed. """ if inst.opcode == "jnz": - lat = lattice[inst.operands[0]] + lat = _get_lattice(lattice, inst.operands[0]) + if isinstance(lat, IRLiteral): if lat.value == 0: target = inst.operands[2] @@ -285,8 +288,10 @@ def _replace_constants(self, inst: IRInstruction, lattice: Lattice): inst.opcode = "jmp" inst.operands = [target] self.cfg_dirty = True - elif inst.opcode == "assert": - lat = lattice[inst.operands[0]] + + elif inst.opcode in ("assert", "assert_unreachable"): + lat = _get_lattice(lattice, inst.operands[0]) + if isinstance(lat, IRLiteral): if lat.value > 0: inst.opcode = "nop" @@ -334,3 +339,10 @@ def _meet(x: LatticeItem, y: LatticeItem) -> LatticeItem: if y == LatticeEnum.TOP or x == y: return x return LatticeEnum.BOTTOM + + +def _get_lattice(lattice: Lattice, x: IROperand) -> LatticeItem: + if isinstance(x, IRLiteral): + return x + + return lattice[x] From df435321d970ddeea7b038c48955669f63a28571 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 7 May 2024 08:47:04 -0400 Subject: [PATCH 02/12] fix get_lattice for djmp refactor: rename _get_lattice to _from_lattice, change it to a method --- vyper/venom/passes/sccp/sccp.py | 40 +++++++++++++++------------------ 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index 47b276ad29..bc8b068bf6 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -143,6 +143,16 @@ def _handle_SSA_work_item(self, work_item: SSAWorkListItem): elif len(self.cfg_in_exec[work_item.inst.parent]) > 0: self._visit_expr(work_item.inst) + def _from_lattice(self, op: IROperand): + if isinstance(op, IRLiteral): + return op + + if isinstance(op, IRLabel): + return LatticeEnum.BOTTOM + + assert isinstance(op, IRVariable) # IRLabel would be an error + return self.lattice[op] + def _visit_phi(self, inst: IRInstruction): assert inst.opcode == "phi", "Can't visit non phi instruction" in_vars: list[LatticeItem] = [] @@ -170,7 +180,7 @@ def _visit_expr(self, inst: IRInstruction): target = self.fn.get_basic_block(inst.operands[0].value) self.work_list.append(FlowWorkItem(inst.parent, target)) elif opcode == "jnz": - lat = _get_lattice(self.lattice, inst.operands[0]) + lat = self._from_lattice(inst.operands[0]) assert lat != LatticeEnum.TOP, f"Got undefined var at jmp at {inst.parent}" if lat == LatticeEnum.BOTTOM: @@ -184,7 +194,7 @@ def _visit_expr(self, inst: IRInstruction): target = self.fn.get_basic_block(inst.operands[2].name) self.work_list.append(FlowWorkItem(inst.parent, target)) elif opcode == "djmp": - lat = self.lattice[inst.operands[0]] + lat = self._from_lattice(inst.operands[0]) assert lat != LatticeEnum.TOP, f"Got undefined var at jmp at {inst.parent}" if lat == LatticeEnum.BOTTOM: for op in inst.operands[1:]: @@ -213,14 +223,7 @@ def _eval(self, inst) -> LatticeItem: """ opcode = inst.opcode - ops = [] - for op in inst.operands: - if isinstance(op, IRVariable): - ops.append(self.lattice[op]) - elif isinstance(op, IRLabel): - return LatticeEnum.BOTTOM - else: - ops.append(op) + ops = [self._from_lattice(op) for op in inst.operands] ret = None if LatticeEnum.BOTTOM in ops: @@ -269,16 +272,16 @@ def _propagate_constants(self): """ for bb in self.dom.dfs_walk: for inst in bb.instructions: - self._replace_constants(inst, self.lattice) + self._replace_constants(inst) - def _replace_constants(self, inst: IRInstruction, lattice: Lattice): + def _replace_constants(self, inst: IRInstruction): """ This method replaces constant values in the instruction with their actual values. It also updates the instruction opcode in case of jumps and asserts as needed. """ if inst.opcode == "jnz": - lat = _get_lattice(lattice, inst.operands[0]) + lat = self._from_lattice(inst.operands[0]) if isinstance(lat, IRLiteral): if lat.value == 0: @@ -290,7 +293,7 @@ def _replace_constants(self, inst: IRInstruction, lattice: Lattice): self.cfg_dirty = True elif inst.opcode in ("assert", "assert_unreachable"): - lat = _get_lattice(lattice, inst.operands[0]) + lat = self._from_lattice(inst.operands[0]) if isinstance(lat, IRLiteral): if lat.value > 0: @@ -308,7 +311,7 @@ def _replace_constants(self, inst: IRInstruction, lattice: Lattice): for i, op in enumerate(inst.operands): if isinstance(op, IRVariable): - lat = lattice[op] + lat = self.lattice[op] if isinstance(lat, IRLiteral): inst.operands[i] = lat @@ -339,10 +342,3 @@ def _meet(x: LatticeItem, y: LatticeItem) -> LatticeItem: if y == LatticeEnum.TOP or x == y: return x return LatticeEnum.BOTTOM - - -def _get_lattice(lattice: Lattice, x: IROperand) -> LatticeItem: - if isinstance(x, IRLiteral): - return x - - return lattice[x] From 8010d7f4e17c4d77ebe48b7547c3e08ec1578956 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 7 May 2024 09:03:25 -0400 Subject: [PATCH 03/12] additionally test venom with -Ocodesize and -Ogas --- .github/workflows/test.yml | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5f02444af1..3162b7ae6f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -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 From 2aecae37037e3e3537f7dbad8983162234a52e7e Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 7 May 2024 09:18:44 -0400 Subject: [PATCH 04/12] fix jump map --- tests/unit/compiler/test_source_map.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/unit/compiler/test_source_map.py b/tests/unit/compiler/test_source_map.py index e4d5e69770..d99b546403 100644 --- a/tests/unit/compiler/test_source_map.py +++ b/tests/unit/compiler/test_source_map.py @@ -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 From 4c0319970b32a27c3dc406aca96a06e72b50c99d Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 7 May 2024 15:17:21 -0400 Subject: [PATCH 05/12] fix exp eval in sccp --- vyper/ast/nodes.py | 1 + vyper/venom/passes/sccp/eval.py | 29 ++++++++++++++--------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/vyper/ast/nodes.py b/vyper/ast/nodes.py index 9818078b21..cce60561a8 100644 --- a/vyper/ast/nodes.py +++ b/vyper/ast/nodes.py @@ -1114,6 +1114,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) diff --git a/vyper/venom/passes/sccp/eval.py b/vyper/venom/passes/sccp/eval.py index 8acca039c0..cd4243e998 100644 --- a/vyper/venom/passes/sccp/eval.py +++ b/vyper/venom/passes/sccp/eval.py @@ -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 @@ -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 @@ -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), @@ -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), From e39076ad3d24f0a286967da449ddb5c7b7ca4ddf Mon Sep 17 00:00:00 2001 From: Harry Kalogirou Date: Wed, 8 May 2024 12:12:10 +0300 Subject: [PATCH 06/12] refactor: sccp lattice manipulation * changed the index type of lattice to `IRVariable` (it should only have variables) * add getter/setter methods that also check types to cache future regretions * add `_eval_lattice_with_op()` to evaluate the lattice with an operant * revert `_eval()` changes * unify the handling of `store` --- vyper/venom/passes/sccp/sccp.py | 51 ++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index bc8b068bf6..2778aa5e18 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -39,7 +39,7 @@ class FlowWorkItem: WorkListItem = Union[FlowWorkItem, SSAWorkListItem] LatticeItem = Union[LatticeEnum, IRLiteral] -Lattice = dict[IROperand, LatticeItem] +Lattice = dict[IRVariable, LatticeItem] class SCCP(IRPass): @@ -143,15 +143,21 @@ def _handle_SSA_work_item(self, work_item: SSAWorkListItem): elif len(self.cfg_in_exec[work_item.inst.parent]) > 0: self._visit_expr(work_item.inst) - def _from_lattice(self, op: IROperand): + def _get_lattice(self, op: IROperand) -> LatticeItem: + assert isinstance(op, IRVariable), "Can't get lattice for non-variable" + lat = self.lattice[op] + assert lat is not None, f"Got undefined var {op}" + return lat + + def _set_lattice(self, op: IROperand, value: LatticeItem): + assert isinstance(op, IRVariable), "Can't set lattice for non-variable" + self.lattice[op] = value + + def _eval_lattice_with_op(self, op: IROperand) -> IRLiteral | LatticeEnum: if isinstance(op, IRLiteral): return op - if isinstance(op, IRLabel): - return LatticeEnum.BOTTOM - - assert isinstance(op, IRVariable) # IRLabel would be an error - return self.lattice[op] + return self._get_lattice(op) def _visit_phi(self, inst: IRInstruction): assert inst.opcode == "phi", "Can't visit non phi instruction" @@ -160,27 +166,25 @@ def _visit_phi(self, inst: IRInstruction): bb = self.fn.get_basic_block(bb_label.name) if bb not in self.cfg_in_exec[inst.parent]: continue - in_vars.append(self.lattice[var]) + in_vars.append(self._get_lattice(var)) value = reduce(_meet, in_vars, LatticeEnum.TOP) # type: ignore assert inst.output in self.lattice, "Got undefined var for phi" - if value != self.lattice[inst.output]: - self.lattice[inst.output] = value + if value != self._get_lattice(inst.output): + self._set_lattice(inst.output, value) self._add_ssa_work_items(inst) def _visit_expr(self, inst: IRInstruction): opcode = inst.opcode if opcode in ["store", "alloca"]: - if isinstance(inst.operands[0], IRLiteral): - self.lattice[inst.output] = inst.operands[0] # type: ignore - else: - self.lattice[inst.output] = self.lattice[inst.operands[0]] # type: ignore + assert inst.output is not None, "Got store/alloca without output" + self._set_lattice(inst.output, self._eval_lattice_with_op(inst.operands[0])) self._add_ssa_work_items(inst) elif opcode == "jmp": target = self.fn.get_basic_block(inst.operands[0].value) self.work_list.append(FlowWorkItem(inst.parent, target)) elif opcode == "jnz": - lat = self._from_lattice(inst.operands[0]) + lat = self._eval_lattice_with_op(inst.operands[0]) assert lat != LatticeEnum.TOP, f"Got undefined var at jmp at {inst.parent}" if lat == LatticeEnum.BOTTOM: @@ -194,7 +198,7 @@ def _visit_expr(self, inst: IRInstruction): target = self.fn.get_basic_block(inst.operands[2].name) self.work_list.append(FlowWorkItem(inst.parent, target)) elif opcode == "djmp": - lat = self._from_lattice(inst.operands[0]) + lat = self._eval_lattice_with_op(inst.operands[0]) assert lat != LatticeEnum.TOP, f"Got undefined var at jmp at {inst.parent}" if lat == LatticeEnum.BOTTOM: for op in inst.operands[1:]: @@ -212,7 +216,7 @@ def _visit_expr(self, inst: IRInstruction): self._eval(inst) else: if inst.output is not None: - self.lattice[inst.output] = LatticeEnum.BOTTOM + self._set_lattice(inst.output, LatticeEnum.BOTTOM) def _eval(self, inst) -> LatticeItem: """ @@ -223,7 +227,14 @@ def _eval(self, inst) -> LatticeItem: """ opcode = inst.opcode - ops = [self._from_lattice(op) for op in inst.operands] + ops = [] + for op in inst.operands: + if isinstance(op, IRVariable): + ops.append(self.lattice[op]) + elif isinstance(op, IRLabel): + return LatticeEnum.BOTTOM + else: + ops.append(op) ret = None if LatticeEnum.BOTTOM in ops: @@ -281,7 +292,7 @@ def _replace_constants(self, inst: IRInstruction): case of jumps and asserts as needed. """ if inst.opcode == "jnz": - lat = self._from_lattice(inst.operands[0]) + lat = self._eval_lattice_with_op(inst.operands[0]) if isinstance(lat, IRLiteral): if lat.value == 0: @@ -293,7 +304,7 @@ def _replace_constants(self, inst: IRInstruction): self.cfg_dirty = True elif inst.opcode in ("assert", "assert_unreachable"): - lat = self._from_lattice(inst.operands[0]) + lat = self._eval_lattice_with_op(inst.operands[0]) if isinstance(lat, IRLiteral): if lat.value > 0: From ee6fc9868af95ab93cc4f460bf7dfe435757b5d2 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 8 May 2024 08:49:00 -0400 Subject: [PATCH 07/12] Revert "additionally test venom with -Ocodesize and -Ogas" This reverts commit 8010d7f4e17c4d77ebe48b7547c3e08ec1578956. --- .github/workflows/test.yml | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3162b7ae6f..5f02444af1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -120,20 +120,7 @@ jobs: evm-version: shanghai evm-backend: revm experimental-codegen: true - - - 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 + # TODO: test experimental_codegen + -Ocodesize # run across other python versions. we don't really need to run all # modes across all python versions - one is enough From c7e435b3ade20e8ea8116d12106bc947428172f1 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 8 May 2024 08:49:11 -0400 Subject: [PATCH 08/12] Revert "fix jump map" This reverts commit 2aecae37037e3e3537f7dbad8983162234a52e7e. --- tests/unit/compiler/test_source_map.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/unit/compiler/test_source_map.py b/tests/unit/compiler/test_source_map.py index d99b546403..e4d5e69770 100644 --- a/tests/unit/compiler/test_source_map.py +++ b/tests/unit/compiler/test_source_map.py @@ -32,19 +32,14 @@ def foo(a: uint256) -> int128: """ -def test_jump_map(optimize, experimental_codegen): +def test_jump_map(optimize): 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: - # 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 + expected_jumps = 3 # some jumps get optimized out when optimizer is on 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 From 78718ae0cc365b5fa2e680c0ab09b6534984e221 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 8 May 2024 08:49:20 -0400 Subject: [PATCH 09/12] Revert "fix exp eval in sccp" This reverts commit 4c0319970b32a27c3dc406aca96a06e72b50c99d. --- vyper/ast/nodes.py | 1 - vyper/venom/passes/sccp/eval.py | 29 +++++++++++++++-------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/vyper/ast/nodes.py b/vyper/ast/nodes.py index b4042c75a7..ca101b61d5 100644 --- a/vyper/ast/nodes.py +++ b/vyper/ast/nodes.py @@ -1104,7 +1104,6 @@ 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) diff --git a/vyper/venom/passes/sccp/eval.py b/vyper/venom/passes/sccp/eval.py index cd4243e998..8acca039c0 100644 --- a/vyper/venom/passes/sccp/eval.py +++ b/vyper/venom/passes/sccp/eval.py @@ -1,14 +1,7 @@ import operator from typing import Callable -from vyper.utils import ( - SizeLimits, - evm_div, - evm_mod, - evm_pow, - signed_to_unsigned, - unsigned_to_signed, -) +from vyper.utils import SizeLimits, evm_div, evm_mod, signed_to_unsigned, unsigned_to_signed from vyper.venom.basicblock import IROperand @@ -37,11 +30,9 @@ def wrapper(ops: list[IROperand]) -> int: def _wrap_binop(operation): def wrapper(ops: list[IROperand]) -> int: - 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 + first = ops[1].value + second = ops[0].value + return (int(operation(first, second))) & SizeLimits.MAX_UINT256 return wrapper @@ -99,6 +90,16 @@ 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), @@ -107,7 +108,7 @@ def _evm_not(ops: list[IROperand]) -> int: "sdiv": _wrap_signed_binop(evm_div), "mod": _wrap_binop(evm_mod), "smod": _wrap_signed_binop(evm_mod), - "exp": _wrap_binop(evm_pow), + "exp": _evm_exp, "eq": _wrap_binop(operator.eq), "ne": _wrap_binop(operator.ne), "lt": _wrap_binop(operator.lt), From a3cc2adaa1277bd63b7f4a46f4d55744cfe34524 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 8 May 2024 09:38:00 -0400 Subject: [PATCH 10/12] rename to eval_from_lattice --- vyper/venom/passes/sccp/sccp.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index 2778aa5e18..df10b6a1bb 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -153,7 +153,7 @@ def _set_lattice(self, op: IROperand, value: LatticeItem): assert isinstance(op, IRVariable), "Can't set lattice for non-variable" self.lattice[op] = value - def _eval_lattice_with_op(self, op: IROperand) -> IRLiteral | LatticeEnum: + def _eval_from_lattice(self, op: IROperand) -> IRLiteral | LatticeEnum: if isinstance(op, IRLiteral): return op @@ -178,13 +178,13 @@ def _visit_expr(self, inst: IRInstruction): opcode = inst.opcode if opcode in ["store", "alloca"]: assert inst.output is not None, "Got store/alloca without output" - self._set_lattice(inst.output, self._eval_lattice_with_op(inst.operands[0])) + self._set_lattice(inst.output, self._eval_from_lattice(inst.operands[0])) self._add_ssa_work_items(inst) elif opcode == "jmp": target = self.fn.get_basic_block(inst.operands[0].value) self.work_list.append(FlowWorkItem(inst.parent, target)) elif opcode == "jnz": - lat = self._eval_lattice_with_op(inst.operands[0]) + lat = self._eval_from_lattice(inst.operands[0]) assert lat != LatticeEnum.TOP, f"Got undefined var at jmp at {inst.parent}" if lat == LatticeEnum.BOTTOM: @@ -198,7 +198,7 @@ def _visit_expr(self, inst: IRInstruction): target = self.fn.get_basic_block(inst.operands[2].name) self.work_list.append(FlowWorkItem(inst.parent, target)) elif opcode == "djmp": - lat = self._eval_lattice_with_op(inst.operands[0]) + lat = self._eval_from_lattice(inst.operands[0]) assert lat != LatticeEnum.TOP, f"Got undefined var at jmp at {inst.parent}" if lat == LatticeEnum.BOTTOM: for op in inst.operands[1:]: @@ -292,7 +292,7 @@ def _replace_constants(self, inst: IRInstruction): case of jumps and asserts as needed. """ if inst.opcode == "jnz": - lat = self._eval_lattice_with_op(inst.operands[0]) + lat = self._eval_from_lattice(inst.operands[0]) if isinstance(lat, IRLiteral): if lat.value == 0: @@ -304,7 +304,7 @@ def _replace_constants(self, inst: IRInstruction): self.cfg_dirty = True elif inst.opcode in ("assert", "assert_unreachable"): - lat = self._eval_lattice_with_op(inst.operands[0]) + lat = self._eval_from_lattice(inst.operands[0]) if isinstance(lat, IRLiteral): if lat.value > 0: From 4d60aa0b6d98b88cd5600a16e3bbd43ba8006357 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 8 May 2024 11:29:07 -0400 Subject: [PATCH 11/12] small style thing --- vyper/venom/passes/sccp/sccp.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index 7188f7e727..9de2a99ab1 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -178,7 +178,8 @@ def _visit_expr(self, inst: IRInstruction): opcode = inst.opcode if opcode in ["store", "alloca"]: assert inst.output is not None, "Got store/alloca without output" - self._set_lattice(inst.output, self._eval_from_lattice(inst.operands[0])) + out = self._eval_from_lattice(inst.operands[0]) + self._set_lattice(inst.output, out) self._add_ssa_work_items(inst) elif opcode == "jmp": target = self.fn.get_basic_block(inst.operands[0].value) From b02e098358e2d6846c11df9412dd0dcca067e71d Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 8 May 2024 10:35:50 -0400 Subject: [PATCH 12/12] rename get_lattice to lookup_from_lattice --- vyper/venom/passes/sccp/sccp.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/vyper/venom/passes/sccp/sccp.py b/vyper/venom/passes/sccp/sccp.py index 9de2a99ab1..ced1f711c5 100644 --- a/vyper/venom/passes/sccp/sccp.py +++ b/vyper/venom/passes/sccp/sccp.py @@ -143,7 +143,7 @@ def _handle_SSA_work_item(self, work_item: SSAWorkListItem): elif len(self.cfg_in_exec[work_item.inst.parent]) > 0: self._visit_expr(work_item.inst) - def _get_lattice(self, op: IROperand) -> LatticeItem: + def _lookup_from_lattice(self, op: IROperand) -> LatticeItem: assert isinstance(op, IRVariable), "Can't get lattice for non-variable" lat = self.lattice[op] assert lat is not None, f"Got undefined var {op}" @@ -157,7 +157,7 @@ def _eval_from_lattice(self, op: IROperand) -> IRLiteral | LatticeEnum: if isinstance(op, IRLiteral): return op - return self._get_lattice(op) + return self._lookup_from_lattice(op) def _visit_phi(self, inst: IRInstruction): assert inst.opcode == "phi", "Can't visit non phi instruction" @@ -166,11 +166,11 @@ def _visit_phi(self, inst: IRInstruction): bb = self.fn.get_basic_block(bb_label.name) if bb not in self.cfg_in_exec[inst.parent]: continue - in_vars.append(self._get_lattice(var)) + in_vars.append(self._lookup_from_lattice(var)) value = reduce(_meet, in_vars, LatticeEnum.TOP) # type: ignore assert inst.output in self.lattice, "Got undefined var for phi" - if value != self._get_lattice(inst.output): + if value != self._lookup_from_lattice(inst.output): self._set_lattice(inst.output, value) self._add_ssa_work_items(inst)