From 8b3095920ac1b40f5298175deb804311d9c54628 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Mon, 15 Mar 2021 19:29:41 +0100 Subject: [PATCH 1/2] Inline ordinary jumps to small blocks and jumps to terminating control flow. --- Changelog.md | 1 + libevmasm/Inliner.cpp | 45 +++++++++++++------ libevmasm/Inliner.h | 4 +- .../optimizer_BlockDeDuplicator/output | 14 +++--- .../output | 14 ++---- .../output | 16 +++---- test/libevmasm/Optimiser.cpp | 21 +++++++++ test/libsolidity/Assembly.cpp | 2 +- .../array/byte_array_transitional_2.sol | 2 +- .../copying/arrays_from_and_to_storage.sol | 2 +- .../copying/copy_byte_array_to_storage.sol | 2 +- .../array/push/byte_array_push_transition.sol | 2 +- .../constructor/arrays_in_constructors.sol | 2 +- .../semanticTests/externalContracts/snark.sol | 2 +- .../salted_create/salted_create.sol | 2 +- .../various/destructuring_assignment.sol | 2 +- .../viaYul/array_memory_index_access.sol | 2 +- 17 files changed, 79 insertions(+), 56 deletions(-) diff --git a/Changelog.md b/Changelog.md index 187452bc8..5684531e2 100644 --- a/Changelog.md +++ b/Changelog.md @@ -4,6 +4,7 @@ Language Features: * Possibility to use ``bytes.concat`` with variable number of ``bytes`` and ``bytesNN`` arguments which behaves as a restricted version of `abi.encodePacked` with a more descriptive name. Compiler Features: + * Low-Level Inliner: Inline ordinary jumps to small blocks and jumps to small blocks that terminate. Bugfixes: diff --git a/libevmasm/Inliner.cpp b/libevmasm/Inliner.cpp index 6a3972a82..1e910fa60 100644 --- a/libevmasm/Inliner.cpp +++ b/libevmasm/Inliner.cpp @@ -82,9 +82,7 @@ bool Inliner::isInlineCandidate(size_t _tag, ranges::span _i { assertThrow(_items.size() > 0, OptimizerException, ""); - // Only consider blocks that end in a JUMP for now. This can e.g. be extended to include transaction terminating - // instructions as well in the future. - if (_items.back() != Instruction::JUMP) + if (_items.back() != Instruction::JUMP && !SemanticInformation::terminatesControlFlow(_items.back())) return false; // Never inline tags that reference themselves. @@ -196,19 +194,38 @@ bool Inliner::shouldInlineFullFunctionBody(size_t _tag, ranges::span Inliner::shouldInline(size_t _tag, AssemblyItem const& _jump, InlinableBlock const& _block) const +optional Inliner::shouldInline(size_t _tag, AssemblyItem const& _jump, InlinableBlock const& _block) const { - AssemblyItem exitJump = _block.items.back(); - assertThrow(_jump == Instruction::JUMP && exitJump == Instruction::JUMP, OptimizerException, ""); + assertThrow(_jump == Instruction::JUMP, OptimizerException, ""); + AssemblyItem blockExit = _block.items.back(); if ( _jump.getJumpType() == AssemblyItem::JumpType::IntoFunction && - exitJump.getJumpType() == AssemblyItem::JumpType::OutOfFunction + blockExit == Instruction::JUMP && + blockExit.getJumpType() == AssemblyItem::JumpType::OutOfFunction && + shouldInlineFullFunctionBody(_tag, _block.items, _block.pushTagCount) ) - return - shouldInlineFullFunctionBody(_tag, _block.items, _block.pushTagCount) ? - make_optional(AssemblyItem::JumpType::Ordinary) : nullopt; + { + blockExit.setJumpType(AssemblyItem::JumpType::Ordinary); + return blockExit; + } + + // Inline small blocks, if the jump to it is ordinary or the blockExit is a terminating instruction. + if ( + _jump.getJumpType() == AssemblyItem::JumpType::Ordinary || + SemanticInformation::terminatesControlFlow(blockExit) + ) + { + static AssemblyItems const jumpPattern = { + AssemblyItem{PushTag}, + AssemblyItem{Instruction::JUMP}, + }; + if ( + GasMeter::dataGas(codeSize(_block.items), m_isCreation, m_evmVersion) <= + GasMeter::dataGas(codeSize(jumpPattern), m_isCreation, m_evmVersion) + ) + return blockExit; + } return nullopt; } @@ -232,10 +249,10 @@ void Inliner::optimise() { if (optional tag = getLocalTag(item)) if (auto* inlinableBlock = util::valueOrNullptr(inlinableBlocks, *tag)) - if (auto exitJumpType = shouldInline(*tag, nextItem, *inlinableBlock)) + if (auto exitItem = shouldInline(*tag, nextItem, *inlinableBlock)) { - newItems += inlinableBlock->items; - newItems.back().setJumpType(*exitJumpType); + newItems += inlinableBlock->items | ranges::views::drop_last(1); + newItems.emplace_back(move(*exitItem)); // We are removing one push tag to the block we inline. --inlinableBlock->pushTagCount; diff --git a/libevmasm/Inliner.h b/libevmasm/Inliner.h index 651ff8510..ed68dc4ce 100644 --- a/libevmasm/Inliner.h +++ b/libevmasm/Inliner.h @@ -61,8 +61,8 @@ private: uint64_t pushTagCount = 0; }; - /// @returns the exit jump type for the block to be inlined, if a particular jump to it should be inlined, otherwise nullopt. - std::optional shouldInline(size_t _tag, AssemblyItem const& _jump, InlinableBlock const& _block) const; + /// @returns the exit item for the block to be inlined, if a particular jump to it should be inlined, otherwise nullopt. + std::optional shouldInline(size_t _tag, AssemblyItem const& _jump, InlinableBlock const& _block) const; /// @returns true, if the full function at tag @a _tag with body @a _block that is referenced @a _pushTagCount times /// should be inlined, false otherwise. @a _block should start at the first instruction after the function entry tag /// up to and including the return jump. diff --git a/test/cmdlineTests/optimizer_BlockDeDuplicator/output b/test/cmdlineTests/optimizer_BlockDeDuplicator/output index b68e11e1a..ddf17bdae 100644 --- a/test/cmdlineTests/optimizer_BlockDeDuplicator/output +++ b/test/cmdlineTests/optimizer_BlockDeDuplicator/output @@ -10,7 +10,7 @@ EVM assembly: not(sub(shl(0x40, 0x01), 0x01)) and /* "optimizer_BlockDeDuplicator/input.sol":201:206 fun_x */ - or(tag_0_7, shl(0x20, tag_4)) + or(tag_0_7, shl(0x20, tag_2)) sub(shl(0x40, 0x01), 0x01) /* "optimizer_BlockDeDuplicator/input.sol":179:210 function() r = true ? fun_x : f */ and @@ -29,8 +29,8 @@ EVM assembly: tag_5: pop jump(tag_6) - /* "optimizer_BlockDeDuplicator/input.sol":77:103 function fun_x() public {} */ -tag_4: + /* "optimizer_BlockDeDuplicator/input.sol":138:174 function f() public { true ? 1 : 3;} */ +tag_2: jump // out /* "optimizer_BlockDeDuplicator/input.sol":60:213 contract C {... */ tag_6: @@ -66,12 +66,12 @@ sub_0: assembly { dup1 0x2e1fb2bc eq - tag_4 + tag_3 jumpi dup1 0x4753a67d eq - tag_4 + tag_3 jumpi tag_2: 0x00 @@ -80,10 +80,6 @@ sub_0: assembly { /* "optimizer_BlockDeDuplicator/input.sol":138:174 function f() public { true ? 1 : 3;} */ tag_3: stop - /* "optimizer_BlockDeDuplicator/input.sol":108:133 function fun_() public {} */ - tag_4: - jump(tag_3) - /* "optimizer_BlockDeDuplicator/input.sol":138:174 function f() public { true ? 1 : 3;} */ tag_7: jump // out diff --git a/test/cmdlineTests/optimizer_inliner_dynamic_reference/output b/test/cmdlineTests/optimizer_inliner_dynamic_reference/output index 8e986b308..d101f61f1 100644 --- a/test/cmdlineTests/optimizer_inliner_dynamic_reference/output +++ b/test/cmdlineTests/optimizer_inliner_dynamic_reference/output @@ -58,9 +58,9 @@ sub_0: assembly { revert /* "optimizer_inliner_dynamic_reference/input.sol":160:215 function a() public pure returns (uint) { return f(); } */ tag_3: - tag_6 - tag_7 - jump // in + /* "optimizer_inliner_dynamic_reference/input.sol":361:362 6 */ + 0x06 + /* "optimizer_inliner_dynamic_reference/input.sol":160:215 function a() public pure returns (uint) { return f(); } */ tag_6: mload(0x40) /* "#utility.yul":160:185 */ @@ -100,12 +100,6 @@ sub_0: assembly { sstore /* "optimizer_inliner_dynamic_reference/input.sol":125:155 function g() public { x = f; } */ stop - /* "optimizer_inliner_dynamic_reference/input.sol":160:215 function a() public pure returns (uint) { return f(); } */ - tag_7: - /* "optimizer_inliner_dynamic_reference/input.sol":194:198 uint */ - 0x00 - /* "optimizer_inliner_dynamic_reference/input.sol":361:362 6 */ - 0x06 /* "optimizer_inliner_dynamic_reference/input.sol":209:212 f() */ tag_16: /* "optimizer_inliner_dynamic_reference/input.sol":202:212 return f() */ @@ -147,8 +141,8 @@ sub_0: assembly { tag_17: /* "optimizer_inliner_dynamic_reference/input.sol":361:362 6 */ 0x06 - /* "optimizer_inliner_dynamic_reference/input.sol":310:365 function f() internal pure returns (uint) { return 6; } */ swap1 + /* "optimizer_inliner_dynamic_reference/input.sol":310:365 function f() internal pure returns (uint) { return 6; } */ jump // out tag_20: mstore(0x00, shl(0xe0, 0x4e487b71)) diff --git a/test/cmdlineTests/optimizer_inliner_dynamic_reference_constructor/output b/test/cmdlineTests/optimizer_inliner_dynamic_reference_constructor/output index e9f43bdab..6a06f5d73 100644 --- a/test/cmdlineTests/optimizer_inliner_dynamic_reference_constructor/output +++ b/test/cmdlineTests/optimizer_inliner_dynamic_reference_constructor/output @@ -35,8 +35,8 @@ tag_1: tag_4: /* "optimizer_inliner_dynamic_reference_constructor/input.sol":355:356 6 */ 0x06 - /* "optimizer_inliner_dynamic_reference_constructor/input.sol":304:359 function f() internal pure returns (uint) { return 6; } */ swap1 + /* "optimizer_inliner_dynamic_reference_constructor/input.sol":304:359 function f() internal pure returns (uint) { return 6; } */ jump // out /* "optimizer_inliner_dynamic_reference_constructor/input.sol":60:361 contract C {... */ tag_5: @@ -80,9 +80,9 @@ sub_0: assembly { revert /* "optimizer_inliner_dynamic_reference_constructor/input.sol":154:209 function a() public pure returns (uint) { return f(); } */ tag_3: - tag_5 - tag_6 - jump // in + /* "optimizer_inliner_dynamic_reference_constructor/input.sol":355:356 6 */ + 0x06 + /* "optimizer_inliner_dynamic_reference_constructor/input.sol":154:209 function a() public pure returns (uint) { return f(); } */ tag_5: mload(0x40) /* "#utility.yul":160:185 */ @@ -105,12 +105,6 @@ sub_0: assembly { tag_5 tag_10 jump // in - /* "optimizer_inliner_dynamic_reference_constructor/input.sol":154:209 function a() public pure returns (uint) { return f(); } */ - tag_6: - /* "optimizer_inliner_dynamic_reference_constructor/input.sol":188:192 uint */ - 0x00 - /* "optimizer_inliner_dynamic_reference_constructor/input.sol":355:356 6 */ - 0x06 /* "optimizer_inliner_dynamic_reference_constructor/input.sol":203:206 f() */ tag_14: /* "optimizer_inliner_dynamic_reference_constructor/input.sol":196:206 return f() */ @@ -152,8 +146,8 @@ sub_0: assembly { tag_12: /* "optimizer_inliner_dynamic_reference_constructor/input.sol":355:356 6 */ 0x06 - /* "optimizer_inliner_dynamic_reference_constructor/input.sol":304:359 function f() internal pure returns (uint) { return 6; } */ swap1 + /* "optimizer_inliner_dynamic_reference_constructor/input.sol":304:359 function f() internal pure returns (uint) { return 6; } */ jump // out tag_17: mstore(0x00, shl(0xe0, 0x4e487b71)) diff --git a/test/libevmasm/Optimiser.cpp b/test/libevmasm/Optimiser.cpp index f6519646d..1899a9363 100644 --- a/test/libevmasm/Optimiser.cpp +++ b/test/libevmasm/Optimiser.cpp @@ -1637,6 +1637,27 @@ BOOST_AUTO_TEST_CASE(inliner_cse_break) ); } +BOOST_AUTO_TEST_CASE(inliner_stop) +{ + AssemblyItem jumpTo{Instruction::JUMP}; + AssemblyItems items{ + AssemblyItem(PushTag, 1), + Instruction::JUMP, + AssemblyItem(Tag, 1), + Instruction::STOP + }; + AssemblyItems expectation{ + Instruction::STOP, + AssemblyItem(Tag, 1), + Instruction::STOP + }; + Inliner{items, {}, 200, false, {}}.optimise(); + BOOST_CHECK_EQUAL_COLLECTIONS( + items.begin(), items.end(), + expectation.begin(), expectation.end() + ); +} + BOOST_AUTO_TEST_SUITE_END() } // end namespaces diff --git a/test/libsolidity/Assembly.cpp b/test/libsolidity/Assembly.cpp index 7803a0d0b..75238c9cf 100644 --- a/test/libsolidity/Assembly.cpp +++ b/test/libsolidity/Assembly.cpp @@ -210,7 +210,7 @@ BOOST_AUTO_TEST_CASE(jump_type) jumpTypes += item.getJumpTypeAsString() + "\n"; if (solidity::test::CommonOptions::get().optimize) - BOOST_CHECK_EQUAL(jumpTypes, "[in]\n[out]\n[in]\n[out]\n"); + BOOST_CHECK_EQUAL(jumpTypes, "[in]\n[out]\n[out]\n[in]\n[out]\n"); else BOOST_CHECK_EQUAL(jumpTypes, "[in]\n[out]\n[in]\n[out]\n"); } diff --git a/test/libsolidity/semanticTests/array/byte_array_transitional_2.sol b/test/libsolidity/semanticTests/array/byte_array_transitional_2.sol index b9e7d6b8c..6d73090f0 100644 --- a/test/libsolidity/semanticTests/array/byte_array_transitional_2.sol +++ b/test/libsolidity/semanticTests/array/byte_array_transitional_2.sol @@ -21,4 +21,4 @@ contract c { // test() -> 0 // gas irOptimized: 312322 // gas legacy: 483915 -// gas legacyOptimized: 478673 +// gas legacyOptimized: 478672 diff --git a/test/libsolidity/semanticTests/array/copying/arrays_from_and_to_storage.sol b/test/libsolidity/semanticTests/array/copying/arrays_from_and_to_storage.sol index b0b4b00df..c1d7d4d6c 100644 --- a/test/libsolidity/semanticTests/array/copying/arrays_from_and_to_storage.sol +++ b/test/libsolidity/semanticTests/array/copying/arrays_from_and_to_storage.sol @@ -14,7 +14,7 @@ contract Test { // set(uint24[]): 0x20, 18, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18 -> 18 // gas irOptimized: 121109 // gas legacy: 125815 -// gas legacyOptimized: 123615 +// gas legacyOptimized: 123614 // data(uint256): 7 -> 8 // data(uint256): 15 -> 16 // data(uint256): 18 -> FAILURE diff --git a/test/libsolidity/semanticTests/array/copying/copy_byte_array_to_storage.sol b/test/libsolidity/semanticTests/array/copying/copy_byte_array_to_storage.sol index 7c98a96d5..fef894c27 100644 --- a/test/libsolidity/semanticTests/array/copying/copy_byte_array_to_storage.sol +++ b/test/libsolidity/semanticTests/array/copying/copy_byte_array_to_storage.sol @@ -50,4 +50,4 @@ contract C { // f() -> 0xff // gas irOptimized: 137415 // gas legacy: 137645 -// gas legacyOptimized: 134377 +// gas legacyOptimized: 134376 diff --git a/test/libsolidity/semanticTests/array/push/byte_array_push_transition.sol b/test/libsolidity/semanticTests/array/push/byte_array_push_transition.sol index bb4f87b6b..58aae2aed 100644 --- a/test/libsolidity/semanticTests/array/push/byte_array_push_transition.sol +++ b/test/libsolidity/semanticTests/array/push/byte_array_push_transition.sol @@ -19,4 +19,4 @@ contract c { // test() -> 0 // gas irOptimized: 397892 // gas legacy: 565428 -// gas legacyOptimized: 552525 +// gas legacyOptimized: 552524 diff --git a/test/libsolidity/semanticTests/constructor/arrays_in_constructors.sol b/test/libsolidity/semanticTests/constructor/arrays_in_constructors.sol index 86ffccea4..fdc97e3fb 100644 --- a/test/libsolidity/semanticTests/constructor/arrays_in_constructors.sol +++ b/test/libsolidity/semanticTests/constructor/arrays_in_constructors.sol @@ -28,4 +28,4 @@ contract Creator { // f(uint256,address[]): 7, 0x40, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 -> 7, 8 // gas irOptimized: 472714 // gas legacy: 570900 -// gas legacyOptimized: 436360 +// gas legacyOptimized: 435524 diff --git a/test/libsolidity/semanticTests/externalContracts/snark.sol b/test/libsolidity/semanticTests/externalContracts/snark.sol index afc248e67..b3409848f 100644 --- a/test/libsolidity/semanticTests/externalContracts/snark.sol +++ b/test/libsolidity/semanticTests/externalContracts/snark.sol @@ -298,4 +298,4 @@ contract Test { // verifyTx() -> true // gas irOptimized: 145824 // gas legacy: 130571 -// gas legacyOptimized: 100187 +// gas legacyOptimized: 100147 diff --git a/test/libsolidity/semanticTests/salted_create/salted_create.sol b/test/libsolidity/semanticTests/salted_create/salted_create.sol index 8d26c4120..9c0e4aa80 100644 --- a/test/libsolidity/semanticTests/salted_create/salted_create.sol +++ b/test/libsolidity/semanticTests/salted_create/salted_create.sol @@ -24,4 +24,4 @@ contract A { // same_salt() -> true // gas irOptimized: 98439083 // gas legacy: 98439116 -// gas legacyOptimized: 98438982 +// gas legacyOptimized: 98438970 diff --git a/test/libsolidity/semanticTests/various/destructuring_assignment.sol b/test/libsolidity/semanticTests/various/destructuring_assignment.sol index 0e8ae6edf..055a635f8 100644 --- a/test/libsolidity/semanticTests/various/destructuring_assignment.sol +++ b/test/libsolidity/semanticTests/various/destructuring_assignment.sol @@ -38,4 +38,4 @@ contract C { // f(bytes): 0x20, 0x5, "abcde" -> 0 // gas irOptimized: 248997 // gas legacy: 239258 -// gas legacyOptimized: 238578 +// gas legacyOptimized: 238577 diff --git a/test/libsolidity/semanticTests/viaYul/array_memory_index_access.sol b/test/libsolidity/semanticTests/viaYul/array_memory_index_access.sol index 26b07df09..95efccecc 100644 --- a/test/libsolidity/semanticTests/viaYul/array_memory_index_access.sol +++ b/test/libsolidity/semanticTests/viaYul/array_memory_index_access.sol @@ -30,7 +30,7 @@ contract C { // index(uint256): 0xFF -> true // gas irOptimized: 167533 // gas legacy: 248854 -// gas legacyOptimized: 152640 +// gas legacyOptimized: 152638 // accessIndex(uint256,int256): 10, 1 -> 2 // accessIndex(uint256,int256): 10, 0 -> 1 // accessIndex(uint256,int256): 10, 11 -> FAILURE, hex"4e487b71", 0x32 From 4fbf5a3f122656d234d642a03f719c6267e69564 Mon Sep 17 00:00:00 2001 From: hrkrshnn Date: Wed, 24 Mar 2021 13:48:52 +0100 Subject: [PATCH 2/2] Added few more tests for low level inliner. --- test/libevmasm/Optimiser.cpp | 88 +++++++++++++++++++++++++++++++++++- 1 file changed, 87 insertions(+), 1 deletion(-) diff --git a/test/libevmasm/Optimiser.cpp b/test/libevmasm/Optimiser.cpp index 1899a9363..e71862fcc 100644 --- a/test/libevmasm/Optimiser.cpp +++ b/test/libevmasm/Optimiser.cpp @@ -1639,7 +1639,6 @@ BOOST_AUTO_TEST_CASE(inliner_cse_break) BOOST_AUTO_TEST_CASE(inliner_stop) { - AssemblyItem jumpTo{Instruction::JUMP}; AssemblyItems items{ AssemblyItem(PushTag, 1), Instruction::JUMP, @@ -1658,6 +1657,93 @@ BOOST_AUTO_TEST_CASE(inliner_stop) ); } +BOOST_AUTO_TEST_CASE(inliner_stop_jumpi) +{ + // Because of `jumpi`, will not be inlined. + AssemblyItems items{ + u256(1), + AssemblyItem(PushTag, 1), + Instruction::JUMPI, + AssemblyItem(Tag, 1), + Instruction::STOP + }; + AssemblyItems expectation = items; + Inliner{items, {}, 200, false, {}}.optimise(); + BOOST_CHECK_EQUAL_COLLECTIONS( + items.begin(), items.end(), + expectation.begin(), expectation.end() + ); +} + +BOOST_AUTO_TEST_CASE(inliner_revert) +{ + AssemblyItems items{ + AssemblyItem(PushTag, 1), + Instruction::JUMP, + AssemblyItem(Tag, 1), + u256(0), + Instruction::DUP1, + Instruction::REVERT + }; + AssemblyItems expectation{ + u256(0), + Instruction::DUP1, + Instruction::REVERT, + AssemblyItem(Tag, 1), + u256(0), + Instruction::DUP1, + Instruction::REVERT + }; + + Inliner{items, {}, 200, false, {}}.optimise(); + BOOST_CHECK_EQUAL_COLLECTIONS( + items.begin(), items.end(), + expectation.begin(), expectation.end() + ); +} + +BOOST_AUTO_TEST_CASE(inliner_revert_increased_datagas) +{ + // Inlining this would increase data gas (5 bytes v/s 4 bytes), therefore, skipped. + AssemblyItems items{ + AssemblyItem(PushTag, 1), + Instruction::JUMP, + AssemblyItem(Tag, 1), + u256(0), + u256(0), + Instruction::REVERT + }; + + AssemblyItems expectation = items; + Inliner{items, {}, 200, false, {}}.optimise(); + BOOST_CHECK_EQUAL_COLLECTIONS( + items.begin(), items.end(), + expectation.begin(), expectation.end() + ); +} + +BOOST_AUTO_TEST_CASE(inliner_invalid) +{ + AssemblyItems items{ + AssemblyItem(PushTag, 1), + Instruction::JUMP, + AssemblyItem(Tag, 1), + Instruction::INVALID + }; + + AssemblyItems expectation = { + Instruction::INVALID, + AssemblyItem(Tag, 1), + Instruction::INVALID + }; + Inliner{items, {}, 200, false, {}}.optimise(); + BOOST_CHECK_EQUAL_COLLECTIONS( + items.begin(), items.end(), + expectation.begin(), expectation.end() + ); +} + + BOOST_AUTO_TEST_SUITE_END() } // end namespaces