From 2191fc3dc6f9ac378d208d9c194c0ca8bfaa8679 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sun, 1 Oct 2023 17:34:20 -0400 Subject: [PATCH 01/11] fix: block mload merging when src and dst overlap --- vyper/ir/optimizer.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/vyper/ir/optimizer.py b/vyper/ir/optimizer.py index 08c2168381..69a70780b0 100644 --- a/vyper/ir/optimizer.py +++ b/vyper/ir/optimizer.py @@ -662,10 +662,10 @@ def _rewrite_mstore_dload(argz): def _merge_mload(argz): if not version_check(begin="cancun"): return False - return _merge_load(argz, "mload", "mcopy") + return _merge_load(argz, "mload", "mcopy", allow_overlap=False) -def _merge_load(argz, _LOAD, _COPY): +def _merge_load(argz, _LOAD, _COPY, allow_overlap=True): # look for sequential operations copying from X to Y # and merge them into a single copy operation changed = False @@ -689,6 +689,11 @@ def _merge_load(argz, _LOAD, _COPY): initial_dst_offset = dst_offset initial_src_offset = src_offset idx = i + + if not allow_overlap and initial_dst_offset <= initial_src_offset <= dst_offset: + # dst and src overlap, block the optimization + break + if ( initial_dst_offset + total_length == dst_offset and initial_src_offset + total_length == src_offset From f01eb070259e6d78160a766db91686922cb07584 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sun, 1 Oct 2023 17:50:09 -0400 Subject: [PATCH 02/11] use continue instead of break --- vyper/ir/optimizer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vyper/ir/optimizer.py b/vyper/ir/optimizer.py index 69a70780b0..0ca6ff31ff 100644 --- a/vyper/ir/optimizer.py +++ b/vyper/ir/optimizer.py @@ -691,8 +691,8 @@ def _merge_load(argz, _LOAD, _COPY, allow_overlap=True): idx = i if not allow_overlap and initial_dst_offset <= initial_src_offset <= dst_offset: - # dst and src overlap, block the optimization - break + # dst and src overlap, discontinue the optimization + continue if ( initial_dst_offset + total_length == dst_offset From 00150f7c213b3ed2f2a6cedcefd81b0eb3e13ec5 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sun, 1 Oct 2023 18:14:27 -0400 Subject: [PATCH 03/11] fix the condition --- vyper/ir/optimizer.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vyper/ir/optimizer.py b/vyper/ir/optimizer.py index 0ca6ff31ff..a168fc037f 100644 --- a/vyper/ir/optimizer.py +++ b/vyper/ir/optimizer.py @@ -690,13 +690,13 @@ def _merge_load(argz, _LOAD, _COPY, allow_overlap=True): initial_src_offset = src_offset idx = i - if not allow_overlap and initial_dst_offset <= initial_src_offset <= dst_offset: - # dst and src overlap, discontinue the optimization - continue + # dst and src overlap, discontinue the optimization + has_overlap = initial_src_offset <= initial_dst_offset <= src_offset if ( initial_dst_offset + total_length == dst_offset and initial_src_offset + total_length == src_offset + and (allow_overlap or not has_overlap) ): mstore_nodes.append(ir_node) total_length += 32 From a126502e15087a61160e2e33d896361dd35c096d Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sun, 1 Oct 2023 20:17:12 -0400 Subject: [PATCH 04/11] fix the condition again --- vyper/ir/optimizer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/ir/optimizer.py b/vyper/ir/optimizer.py index a168fc037f..cbaabfd734 100644 --- a/vyper/ir/optimizer.py +++ b/vyper/ir/optimizer.py @@ -691,7 +691,7 @@ def _merge_load(argz, _LOAD, _COPY, allow_overlap=True): idx = i # dst and src overlap, discontinue the optimization - has_overlap = initial_src_offset <= initial_dst_offset <= src_offset + has_overlap = initial_src_offset <= initial_dst_offset <= src_offset + 31 if ( initial_dst_offset + total_length == dst_offset From 5580e98ecb0d952912f8c3eac7da0d017cc50192 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 2 Oct 2023 14:31:25 -0400 Subject: [PATCH 05/11] add test for mload merge overlap --- tests/parser/features/test_assignment.py | 60 ++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/parser/features/test_assignment.py b/tests/parser/features/test_assignment.py index e550f60541..35b008a8ba 100644 --- a/tests/parser/features/test_assignment.py +++ b/tests/parser/features/test_assignment.py @@ -442,3 +442,63 @@ def bug(p: Point) -> Point: """ c = get_contract(code) assert c.bug((1, 2)) == (2, 1) + + +mload_merge_codes = [ + ( + """ +@external +def foo() -> uint256[4]: + # copy "backwards" + xs: uint256[4] = [1, 2, 3, 4] + +# dst < src + xs[0] = xs[1] + xs[1] = xs[2] + xs[2] = xs[3] + + return xs + """, + [2, 3, 4, 4], + ), + ( + """ +@external +def foo() -> uint256[4]: + # copy "forwards" + xs: uint256[4] = [1, 2, 3, 4] + +# src < dst + xs[1] = xs[0] + xs[2] = xs[1] + xs[3] = xs[2] + + return xs + """, + [1, 1, 1, 1], + ), + ( + """ +@external +def foo() -> uint256[5]: + # partial "forward" copy + xs: uint256[5] = [1, 2, 3, 4, 5] + +# src < dst + xs[2] = xs[0] + xs[3] = xs[1] + xs[4] = xs[2] + + return xs + """, + [1, 2, 1, 2, 1], + ), +] + + +# functional test that mload merging does not occur when source and dest +# buffers overlap. (note: mload merging only applies after cancun) +@pytest.mark.parametrize("code,expected_result", mload_merge_codes) +def test_mcopy_overlap(get_contract, code, expected_result): + c = get_contract(code) + assert c.foo() == expected_result From 066f212e7c4253aadba703daeb76a77cb9db2984 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 2 Oct 2023 17:13:39 -0400 Subject: [PATCH 06/11] fix the condition to allow full overlap to become mcopy --- vyper/ir/optimizer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/ir/optimizer.py b/vyper/ir/optimizer.py index cbaabfd734..8df4bbac2d 100644 --- a/vyper/ir/optimizer.py +++ b/vyper/ir/optimizer.py @@ -691,7 +691,7 @@ def _merge_load(argz, _LOAD, _COPY, allow_overlap=True): idx = i # dst and src overlap, discontinue the optimization - has_overlap = initial_src_offset <= initial_dst_offset <= src_offset + 31 + has_overlap = initial_src_offset < initial_dst_offset < src_offset + 32 if ( initial_dst_offset + total_length == dst_offset From 668ba8456297df77f788fdc81931c008e0fac47b Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 2 Oct 2023 17:15:17 -0400 Subject: [PATCH 07/11] add more unit tests for merge_mload --- tests/compiler/ir/test_optimize_ir.py | 57 +++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tests/compiler/ir/test_optimize_ir.py b/tests/compiler/ir/test_optimize_ir.py index 1466166501..1a7a884871 100644 --- a/tests/compiler/ir/test_optimize_ir.py +++ b/tests/compiler/ir/test_optimize_ir.py @@ -1,9 +1,13 @@ import pytest from vyper.codegen.ir_node import IRnode +from vyper.evm.opcodes import EVM_VERSIONS, anchor_evm_version from vyper.exceptions import StaticAssertionException from vyper.ir import optimizer +POST_CANCUN = {k: v for k, v in EVM_VERSIONS.items() if v >= EVM_VERSIONS["cancun"]} + + optimize_list = [ (["eq", 1, 2], [0]), (["lt", 1, 2], [1]), @@ -272,3 +276,56 @@ def test_operator_set_values(): assert optimizer.COMPARISON_OPS == {"lt", "gt", "le", "ge", "slt", "sgt", "sle", "sge"} assert optimizer.STRICT_COMPARISON_OPS == {"lt", "gt", "slt", "sgt"} assert optimizer.UNSTRICT_COMPARISON_OPS == {"le", "ge", "sle", "sge"} + + +mload_merge_list = [ + # no overlap between src and dst buffers, must become mcopy + ( + ["seq", ["mstore", 32, ["mload", 128]], ["mstore", 64, ["mload", 160]]], + ["mcopy", 32, 128, 64], + ), + # full overlap (i.e. a no-op mcopy) + (["seq", ["mstore", 32, ["mload", 32]], ["mstore", 64, ["mload", 64]]], ["mcopy", 32, 32, 64]), + # copy "forwards", must NOT become mcopy + (["seq", ["mstore", 64, ["mload", 32]], ["mstore", 96, ["mload", 64]]], None), + # copy with overlap "backwards", OK to become mcopy + (["seq", ["mstore", 32, ["mload", 64]], ["mstore", 64, ["mload", 96]]], ["mcopy", 32, 64, 64]), + # copy 3 words with partial overlap "forwards", partially becomes mcopy + # (2 words are mcopied and 1 word is mload/mstored + ( + [ + "seq", + ["mstore", 96, ["mload", 32]], + ["mstore", 128, ["mload", 64]], + ["mstore", 160, ["mload", 96]], + ], + ["seq", ["mcopy", 96, 32, 64], ["mstore", 160, ["mload", 96]]], + ), + # copy 4 words with partial overlap "forwards", becomes 2 mcopies of 2 words each + ( + [ + "seq", + ["mstore", 96, ["mload", 32]], + ["mstore", 128, ["mload", 64]], + ["mstore", 160, ["mload", 96]], + ["mstore", 192, ["mload", 128]], + ], + ["seq", ["mcopy", 96, 32, 64], ["mcopy", 160, 96, 64]], + ), + # check overlap by one byte, should NOT become mcopy + (["seq", ["mstore", 63, ["mload", 0]], ["mstore", 95, ["mload", 32]]], None), +] + + +@pytest.mark.parametrize("ir", mload_merge_list) +@pytest.mark.parametrize("evm_version", list(POST_CANCUN.keys())) +def test_mload_merge(ir, evm_version): + with anchor_evm_version(evm_version): + optimized = optimizer.optimize(IRnode.from_list(ir[0])) + if ir[1] is None: + # no-op, assert optimizer does nothing + expected = IRnode.from_list(ir[0]) + else: + expected = IRnode.from_list(ir[1]) + + assert optimized == expected From d739d09f9de80509eb311f13a91f00181bf7715c Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 2 Oct 2023 17:24:35 -0400 Subject: [PATCH 08/11] add more mload merge tests --- tests/compiler/ir/test_optimize_ir.py | 30 ++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/tests/compiler/ir/test_optimize_ir.py b/tests/compiler/ir/test_optimize_ir.py index 1a7a884871..d55340b2ba 100644 --- a/tests/compiler/ir/test_optimize_ir.py +++ b/tests/compiler/ir/test_optimize_ir.py @@ -279,17 +279,24 @@ def test_operator_set_values(): mload_merge_list = [ - # no overlap between src and dst buffers, must become mcopy + # copy "backward" with no overlap between src and dst buffers, + # OK to become mcopy ( ["seq", ["mstore", 32, ["mload", 128]], ["mstore", 64, ["mload", 160]]], ["mcopy", 32, 128, 64], ), - # full overlap (i.e. a no-op mcopy) - (["seq", ["mstore", 32, ["mload", 32]], ["mstore", 64, ["mload", 64]]], ["mcopy", 32, 32, 64]), - # copy "forwards", must NOT become mcopy - (["seq", ["mstore", 64, ["mload", 32]], ["mstore", 96, ["mload", 64]]], None), # copy with overlap "backwards", OK to become mcopy (["seq", ["mstore", 32, ["mload", 64]], ["mstore", 64, ["mload", 96]]], ["mcopy", 32, 64, 64]), + # "stationary" overlap (i.e. a no-op mcopy), OK to become mcopy + (["seq", ["mstore", 32, ["mload", 32]], ["mstore", 64, ["mload", 64]]], ["mcopy", 32, 32, 64]), + # copy "forward" with no overlap, OK to become mcopy + (["seq", ["mstore", 64, ["mload", 0]], ["mstore", 96, ["mload", 32]]], ["mcopy", 64, 0, 64]), + # copy "forwards" with overlap by one word, must NOT become mcopy + (["seq", ["mstore", 64, ["mload", 32]], ["mstore", 96, ["mload", 64]]], None), + # check "forward" overlap by one byte, must NOT become mcopy + (["seq", ["mstore", 64, ["mload", 1]], ["mstore", 96, ["mload", 33]]], None), + # check "forward" overlap by one byte again, must NOT become mcopy + (["seq", ["mstore", 63, ["mload", 0]], ["mstore", 95, ["mload", 32]]], None), # copy 3 words with partial overlap "forwards", partially becomes mcopy # (2 words are mcopied and 1 word is mload/mstored ( @@ -312,8 +319,17 @@ def test_operator_set_values(): ], ["seq", ["mcopy", 96, 32, 64], ["mcopy", 160, 96, 64]], ), - # check overlap by one byte, should NOT become mcopy - (["seq", ["mstore", 63, ["mload", 0]], ["mstore", 95, ["mload", 32]]], None), + # copy 4 words with 1 byte of overlap, must NOT become mcopy + ( + [ + "seq", + ["mstore", 96, ["mload", 33]], + ["mstore", 128, ["mload", 65]], + ["mstore", 160, ["mload", 97]], + ["mstore", 192, ["mload", 129]], + ], + None, + ), ] From a0ef351f4c9acc266ff4b2cbaaea92e3270494db Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 3 Oct 2023 18:40:52 -0400 Subject: [PATCH 09/11] add more mload merging tests --------- Co-authored-by: Robert Chen --- tests/compiler/ir/test_optimize_ir.py | 53 +++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tests/compiler/ir/test_optimize_ir.py b/tests/compiler/ir/test_optimize_ir.py index d55340b2ba..ab325b7a72 100644 --- a/tests/compiler/ir/test_optimize_ir.py +++ b/tests/compiler/ir/test_optimize_ir.py @@ -330,6 +330,59 @@ def test_operator_set_values(): ], None, ), + # Ensure only sequential mstore + mload sequences are optimized + ( + [ + "seq", + ["mstore", 0, ["mload", 32]], + ["shr", 224, ["calldataload", 0]], + ["mstore", 32, ["mload", 64]] + ], + None + ), + # not-word aligned optimizations (not overlap) + ( + [ + "seq", + ["mstore", 0, ["mload", 1]], + ["mstore", 32, ["mload", 33]] + ], + ["mcopy", 0, 1, 64] + ), + # not-word aligned optimizations (overlap) + ( + [ + "seq", + ["mstore", 1, ["mload", 0]], + ["mstore", 33, ["mload", 32]] + ], + None + ), + # not-word aligned optimizations (overlap and not-overlap) + ( + [ + "seq", + ["mstore", 0, ["mload", 1]], + ["mstore", 32, ["mload", 33]], + ["mstore", 1, ["mload", 0]], + ["mstore", 33, ["mload", 32]] + ], + [ + "seq", + ["mcopy", 0, 1, 64], + ["mstore", 1, ["mload", 0]], + ["mstore", 33, ["mload", 32]] + ] + ), + # overflow test + ( + [ + "seq", + ["mstore", 115792089237316195423570985008687907853269984665640564039457584007913129639872, ["mload", 0]], # 2**256-1-31-32 + ["mstore", 115792089237316195423570985008687907853269984665640564039457584007913129639904, ["mload", 32]] # 2**256-1-31 + ], + ["mcopy", 115792089237316195423570985008687907853269984665640564039457584007913129639872, 0, 64] + ) ] From cb7117843bfcfd88ea95c30fd01ed7119d859163 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 3 Oct 2023 18:43:18 -0400 Subject: [PATCH 10/11] fix lint --- tests/compiler/ir/test_optimize_ir.py | 52 ++++++++++++--------------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/tests/compiler/ir/test_optimize_ir.py b/tests/compiler/ir/test_optimize_ir.py index ab325b7a72..2057070a20 100644 --- a/tests/compiler/ir/test_optimize_ir.py +++ b/tests/compiler/ir/test_optimize_ir.py @@ -336,28 +336,14 @@ def test_operator_set_values(): "seq", ["mstore", 0, ["mload", 32]], ["shr", 224, ["calldataload", 0]], - ["mstore", 32, ["mload", 64]] + ["mstore", 32, ["mload", 64]], ], - None + None, ), # not-word aligned optimizations (not overlap) - ( - [ - "seq", - ["mstore", 0, ["mload", 1]], - ["mstore", 32, ["mload", 33]] - ], - ["mcopy", 0, 1, 64] - ), + (["seq", ["mstore", 0, ["mload", 1]], ["mstore", 32, ["mload", 33]]], ["mcopy", 0, 1, 64]), # not-word aligned optimizations (overlap) - ( - [ - "seq", - ["mstore", 1, ["mload", 0]], - ["mstore", 33, ["mload", 32]] - ], - None - ), + (["seq", ["mstore", 1, ["mload", 0]], ["mstore", 33, ["mload", 32]]], None), # not-word aligned optimizations (overlap and not-overlap) ( [ @@ -365,24 +351,32 @@ def test_operator_set_values(): ["mstore", 0, ["mload", 1]], ["mstore", 32, ["mload", 33]], ["mstore", 1, ["mload", 0]], - ["mstore", 33, ["mload", 32]] + ["mstore", 33, ["mload", 32]], ], - [ - "seq", - ["mcopy", 0, 1, 64], - ["mstore", 1, ["mload", 0]], - ["mstore", 33, ["mload", 32]] - ] + ["seq", ["mcopy", 0, 1, 64], ["mstore", 1, ["mload", 0]], ["mstore", 33, ["mload", 32]]], ), # overflow test ( [ "seq", - ["mstore", 115792089237316195423570985008687907853269984665640564039457584007913129639872, ["mload", 0]], # 2**256-1-31-32 - ["mstore", 115792089237316195423570985008687907853269984665640564039457584007913129639904, ["mload", 32]] # 2**256-1-31 + [ + "mstore", + 115792089237316195423570985008687907853269984665640564039457584007913129639872, + ["mload", 0], + ], # 2**256-1-31-32 + [ + "mstore", + 115792089237316195423570985008687907853269984665640564039457584007913129639904, + ["mload", 32], + ], # 2**256-1-31 + ], + [ + "mcopy", + 115792089237316195423570985008687907853269984665640564039457584007913129639872, + 0, + 64, ], - ["mcopy", 115792089237316195423570985008687907853269984665640564039457584007913129639872, 0, 64] - ) + ), ] From d996a1e8fe83463042460d875828278d80cbc43b Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 3 Oct 2023 18:44:37 -0400 Subject: [PATCH 11/11] clarify a couple test cases --- tests/compiler/ir/test_optimize_ir.py | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/tests/compiler/ir/test_optimize_ir.py b/tests/compiler/ir/test_optimize_ir.py index 2057070a20..cb46ba238d 100644 --- a/tests/compiler/ir/test_optimize_ir.py +++ b/tests/compiler/ir/test_optimize_ir.py @@ -335,7 +335,7 @@ def test_operator_set_values(): [ "seq", ["mstore", 0, ["mload", 32]], - ["shr", 224, ["calldataload", 0]], + ["sstore", 0, ["calldataload", 4]], ["mstore", 32, ["mload", 64]], ], None, @@ -359,23 +359,10 @@ def test_operator_set_values(): ( [ "seq", - [ - "mstore", - 115792089237316195423570985008687907853269984665640564039457584007913129639872, - ["mload", 0], - ], # 2**256-1-31-32 - [ - "mstore", - 115792089237316195423570985008687907853269984665640564039457584007913129639904, - ["mload", 32], - ], # 2**256-1-31 - ], - [ - "mcopy", - 115792089237316195423570985008687907853269984665640564039457584007913129639872, - 0, - 64, + ["mstore", 2**256 - 1 - 31 - 32, ["mload", 0]], + ["mstore", 2**256 - 1 - 31, ["mload", 32]], ], + ["mcopy", 2**256 - 1 - 31 - 32, 0, 64], ), ]