Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove superfluous initial iteration of repeating parts of optimizer sequence #14854

Merged
merged 1 commit into from May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Changelog.md
Expand Up @@ -9,6 +9,7 @@ Compiler Features:

Bugfixes:
* Commandline Interface: Fix ICE when the optimizer is disabled and an empty/blank string is used for ``--yul-optimizations`` sequence.
* Optimizer: Fix optimizer executing each repeating part of the step sequence at least twice, even if the code size already became stable after the first iteration.
* SMTChecker: Fix false positive when comparing hashes of same array or string literals.
* SMTChecker: Fix internal error on mapping access caused by too strong requirements on sort compatibility of the index and mapping domain.
* SMTChecker: Fix internal error when using an empty tuple in a conditional operator.
Expand Down
4 changes: 3 additions & 1 deletion libyul/optimiser/Suite.cpp
Expand Up @@ -462,7 +462,9 @@ void OptimiserSuite::runSequence(std::string_view _stepAbbreviations, Block& _as
subsequences.push_back({subsequence, true});
}

size_t codeSize = 0;
// NOTE: If _repeatUntilStable is false, the value will not be used so do not calculate it.
size_t codeSize = (_repeatUntilStable ? CodeSize::codeSizeIncludingFunctions(_ast) : 0);
cameel marked this conversation as resolved.
Show resolved Hide resolved

for (size_t round = 0; round < MaxRounds; ++round)
{
for (auto const& [subsequence, repeat]: subsequences)
Expand Down
13 changes: 11 additions & 2 deletions test/cmdlineTests/optimizer_user_yul/output
Expand Up @@ -42,6 +42,7 @@ tag_5:
/* "optimizer_user_yul/input.sol":263:269 a := 2 */
swap1
pop
0x00
/* "optimizer_user_yul/input.sol":369:370 3 */
0x03
/* "optimizer_user_yul/input.sol":366:367 2 */
Expand All @@ -58,10 +59,18 @@ tag_6:
/* "optimizer_user_yul/input.sol":384:392 sload(5) */
dup1
/* "optimizer_user_yul/input.sol":376:509 for { } sload(5) { } {... */
iszero
tag_6
tag_8
jumpi
/* "optimizer_user_yul/input.sol":495:504 exp(x, y) */
0x01
/* "optimizer_user_yul/input.sol":490:504 z := exp(x, y) */
swap2
pop
/* "optimizer_user_yul/input.sol":376:509 for { } sload(5) { } {... */
jump(tag_6)
tag_8:
/* "optimizer_user_yul/input.sol":380:383 { } */
pop
pop
/* "optimizer_user_yul/input.sol":340:513 {... */
pop
Expand Down
Expand Up @@ -52,7 +52,7 @@ contract C {
}
// ----
// from_storage() -> 0x20, 2, 0x40, 0xa0, 2, 10, 11, 3, 12, 13, 14
// gas irOptimized: 150004
// gas irOptimized: 150022
// gas legacy: 150745
// gas legacyOptimized: 148678
// from_storage_ptr() -> 0x20, 2, 0x40, 0xa0, 2, 10, 11, 3, 12, 13, 14
Expand Down
Expand Up @@ -37,7 +37,7 @@ contract C {
}
// ----
// from_storage() -> 0x20, 2, 0x40, 0xa0, 2, 10, 11, 3, 12, 13, 14
// gas irOptimized: 147871
// gas irOptimized: 147889
// gas legacy: 148896
// gas legacyOptimized: 146901
// from_storage_ptr() -> 0x20, 2, 0x40, 0xa0, 2, 10, 11, 3, 12, 13, 14
Expand Down
Expand Up @@ -14,4 +14,4 @@ contract C {
// compileViaYul: true
// ----
// from_calldata(uint8[][]): 0x20, 2, 0x40, 0xa0, 2, 10, 11, 3, 12, 13, 14 -> 0x20, 2, 0x40, 0xa0, 2, 10, 11, 3, 12, 13, 14
// gas irOptimized: 139683
// gas irOptimized: 139637
Expand Up @@ -31,8 +31,8 @@ contract C {
// compileViaYul: true
// ----
// test1((uint8[],uint8[2])[][][]): 0x20, 1, 0x20, 2, 0x40, 0x0140, 1, 0x20, 0x60, 3, 7, 2, 1, 2, 2, 0x40, 0x0100, 0x60, 17, 19, 2, 11, 13, 0x60, 31, 37, 2, 23, 29 -> 0x20, 2, 0x40, 0x0140, 1, 0x20, 0x60, 3, 7, 2, 1, 2, 2, 0x40, 0x0100, 0x60, 17, 19, 2, 11, 13, 0x60, 31, 37, 2, 23, 29
// gas irOptimized: 327874
// gas irOptimized: 327883
// test2((uint8[],uint8[2])[][1][]): 0x20, 2, 0x40, 0x0160, 0x20, 1, 0x20, 0x60, 17, 19, 2, 11, 13, 0x20, 1, 0x20, 0x60, 31, 37, 2, 23, 29 -> 0x20, 0x20, 1, 0x20, 0x60, 17, 19, 2, 11, 13
// gas irOptimized: 140879
// gas irOptimized: 140882
// test3((uint8[],uint8[2])[1][][2]): 0x20, 0x40, 0x60, 0, 2, 0x40, 288, 0x20, 0x60, 3, 7, 2, 1, 2, 0x20, 0x60, 17, 19, 2, 11, 13 -> 0x20, 2, 0x40, 288, 0x20, 0x60, 3, 7, 2, 1, 2, 0x20, 0x60, 17, 19, 2, 11, 13
// gas irOptimized: 188535
// gas irOptimized: 188541
Expand Up @@ -31,8 +31,8 @@ contract C {
// compileViaYul: true
// ----
// test1((uint8[],uint8[2])[][][]): 0x20, 1, 0x20, 2, 0x40, 0x0140, 1, 0x20, 0x60, 3, 7, 2, 1, 2, 2, 0x40, 0x0100, 0x60, 17, 19, 2, 11, 13, 0x60, 31, 37, 2, 23, 29 -> 0x20, 2, 0x40, 0x0140, 1, 0x20, 0x60, 3, 7, 2, 1, 2, 2, 0x40, 0x0100, 0x60, 17, 19, 2, 11, 13, 0x60, 31, 37, 2, 23, 29
// gas irOptimized: 332660
// gas irOptimized: 332687
// test2((uint8[],uint8[2])[][1][]): 0x20, 2, 0x40, 0x0160, 0x20, 1, 0x20, 0x60, 17, 19, 2, 11, 13, 0x20, 1, 0x20, 0x60, 31, 37, 2, 23, 29 -> 0x20, 0x20, 1, 0x20, 0x60, 17, 19, 2, 11, 13
// gas irOptimized: 145150
// gas irOptimized: 145159
// test3((uint8[],uint8[2])[1][][2]): 0x20, 0x40, 0x60, 0, 2, 0x40, 288, 0x20, 0x60, 3, 7, 2, 1, 2, 0x20, 0x60, 17, 19, 2, 11, 13 -> 0x20, 2, 0x40, 288, 0x20, 0x60, 3, 7, 2, 1, 2, 0x20, 0x60, 17, 19, 2, 11, 13
// gas irOptimized: 192598
// gas irOptimized: 192616
Expand Up @@ -29,8 +29,8 @@ contract C {
// compileViaYul: true
// ----
// test1((uint8[],uint8[2])[][]): 0x20, 2, 0x40, 0x0140, 1, 0x20, 0x60, 3, 7, 2, 1, 2, 2, 0x40, 0x0100, 0x60, 17, 19, 2, 11, 13, 0x60, 31, 37, 2, 23, 29 -> 0x20, 2, 0x40, 0x0140, 1, 0x20, 0x60, 3, 7, 2, 1, 2, 2, 0x40, 0x0100, 0x60, 17, 19, 2, 11, 13, 0x60, 31, 37, 2, 23, 29
// gas irOptimized: 304747
// gas irOptimized: 304756
// test2((uint8[],uint8[2])[][1]): 0x20, 0x20, 1, 0x20, 0x60, 17, 19, 2, 11, 13 -> 0x20, 0x20, 1, 0x20, 0x60, 17, 19, 2, 11, 13
// gas irOptimized: 116464
// gas irOptimized: 116467
// test3((uint8[],uint8[2])[1][]): 0x20, 2, 0x40, 0x0120, 0x20, 0x60, 3, 7, 2, 1, 2, 0x20, 0x60, 17, 19, 2, 11, 13 -> 0x20, 2, 0x40, 0x0120, 0x20, 0x60, 3, 7, 2, 1, 2, 0x20, 0x60, 17, 19, 2, 11, 13
// gas irOptimized: 188024
// gas irOptimized: 188030
Expand Up @@ -29,8 +29,8 @@ contract C {
// compileViaYul: true
// ----
// test1((uint8[],uint8[2])[][]): 0x20, 2, 0x40, 0x0140, 1, 0x20, 0x60, 3, 7, 2, 1, 2, 2, 0x40, 0x0100, 0x60, 17, 19, 2, 11, 13, 0x60, 31, 37, 2, 23, 29 -> 0x20, 2, 0x40, 0x0140, 1, 0x20, 0x60, 3, 7, 2, 1, 2, 2, 0x40, 0x0100, 0x60, 17, 19, 2, 11, 13, 0x60, 31, 37, 2, 23, 29
// gas irOptimized: 309074
// gas irOptimized: 309101
// test2((uint8[],uint8[2])[][1]): 0x20, 0x20, 1, 0x20, 0x60, 17, 19, 2, 11, 13 -> 0x20, 0x20, 1, 0x20, 0x60, 17, 19, 2, 11, 13
// gas irOptimized: 118256
// gas irOptimized: 118265
// test3((uint8[],uint8[2])[1][]): 0x20, 2, 0x40, 0x0120, 0x20, 0x60, 3, 7, 2, 1, 2, 0x20, 0x60, 17, 19, 2, 11, 13 -> 0x20, 2, 0x40, 0x0120, 0x20, 0x60, 3, 7, 2, 1, 2, 0x20, 0x60, 17, 19, 2, 11, 13
// gas irOptimized: 190993
// gas irOptimized: 191011
Expand Up @@ -13,6 +13,6 @@ contract C {
}
// ----
// f() -> 2, 3, 4
// gas irOptimized: 109108
// gas irOptimized: 109104
// gas legacy: 122235
// gas legacyOptimized: 118411
Expand Up @@ -176,8 +176,8 @@ contract DepositContract is IDepositContract, ERC165 {
}
// ----
// constructor()
// gas irOptimized: 809248
// gas irOptimized code: 558000
// gas irOptimized: 809628
// gas irOptimized code: 563200
// gas legacy: 919945
// gas legacy code: 1437600
// gas legacyOptimized: 848464
Expand All @@ -187,27 +187,27 @@ contract DepositContract is IDepositContract, ERC165 {
// supportsInterface(bytes4): 0x01ffc9a700000000000000000000000000000000000000000000000000000000 -> true # ERC-165 id #
// supportsInterface(bytes4): 0x8564090700000000000000000000000000000000000000000000000000000000 -> true # the deposit interface id #
// get_deposit_root() -> 0xd70a234731285c6804c2a4f56711ddb8c82c99740f207854891028af34e27e5e
// gas irOptimized: 109009
// gas irOptimized: 108921
// gas legacy: 142735
// gas legacyOptimized: 117558
// get_deposit_count() -> 0x20, 8, 0 # TODO: check balance and logs after each deposit #
// deposit(bytes,bytes,bytes,bytes32), 32 ether: 0 -> FAILURE # Empty input #
// get_deposit_root() -> 0xd70a234731285c6804c2a4f56711ddb8c82c99740f207854891028af34e27e5e
// gas irOptimized: 109009
// gas irOptimized: 108921
// gas legacy: 142735
// gas legacyOptimized: 117558
// get_deposit_count() -> 0x20, 8, 0
// deposit(bytes,bytes,bytes,bytes32), 1 ether: 0x80, 0xe0, 0x120, 0xaa4a8d0b7d9077248630f1a4701ae9764e42271d7f22b7838778411857fd349e, 0x30, 0x933ad9491b62059dd065b560d256d8957a8c402cc6e8d8ee7290ae11e8f73292, 0x67a8811c397529dac52ae1342ba58c9500000000000000000000000000000000, 0x20, 0x00f50428677c60f997aadeab24aabf7fceaef491c96a52b463ae91f95611cf71, 0x60, 0xa29d01cc8c6296a8150e515b5995390ef841dc18948aa3e79be6d7c1851b4cbb, 0x5d6ff49fa70b9c782399506a22a85193151b9b691245cebafd2063012443c132, 0x4b6c36debaedefb7b2d71b0503ffdc00150aaffd42e63358238ec888901738b8 -> # txhash: 0x7085c586686d666e8bb6e9477a0f0b09565b2060a11f1c4209d3a52295033832 #
// ~ emit DepositEvent(bytes,bytes,bytes,bytes,bytes): 0xa0, 0x0100, 0x0140, 0x0180, 0x0200, 0x30, 0x933ad9491b62059dd065b560d256d8957a8c402cc6e8d8ee7290ae11e8f73292, 0x67a8811c397529dac52ae1342ba58c9500000000000000000000000000000000, 0x20, 0xf50428677c60f997aadeab24aabf7fceaef491c96a52b463ae91f95611cf71, 0x08, 0xca9a3b00000000000000000000000000000000000000000000000000000000, 0x60, 0xa29d01cc8c6296a8150e515b5995390ef841dc18948aa3e79be6d7c1851b4cbb, 0x5d6ff49fa70b9c782399506a22a85193151b9b691245cebafd2063012443c132, 0x4b6c36debaedefb7b2d71b0503ffdc00150aaffd42e63358238ec888901738b8, 0x08, 0x00
// get_deposit_root() -> 0x2089653123d9c721215120b6db6738ba273bbc5228ac093b1f983badcdc8a438
// gas irOptimized: 108994
// gas irOptimized: 108914
// gas legacy: 142744
// gas legacyOptimized: 117570
// get_deposit_count() -> 0x20, 8, 0x0100000000000000000000000000000000000000000000000000000000000000
// deposit(bytes,bytes,bytes,bytes32), 32 ether: 0x80, 0xe0, 0x120, 0xdbd986dc85ceb382708cf90a3500f500f0a393c5ece76963ac3ed72eccd2c301, 0x30, 0xb2ce0f79f90e7b3a113ca5783c65756f96c4b4673c2b5c1eb4efc22280259441, 0x06d601211e8866dc5b50dc48a244dd7c00000000000000000000000000000000, 0x20, 0x00344b6c73f71b11c56aba0d01b7d8ad83559f209d0a4101a515f6ad54c89771, 0x60, 0x945caaf82d18e78c033927d51f452ebcd76524497b91d7a11219cb3db6a1d369, 0x7595fc095ce489e46b2ef129591f2f6d079be4faaf345a02c5eb133c072e7c56, 0x0c6c3617eee66b4b878165c502357d49485326bc6b31bc96873f308c8f19c09d -> # txhash: 0x404d8e109822ce448e68f45216c12cb051b784d068fbe98317ab8e50c58304ac #
// ~ emit DepositEvent(bytes,bytes,bytes,bytes,bytes): 0xa0, 0x0100, 0x0140, 0x0180, 0x0200, 0x30, 0xb2ce0f79f90e7b3a113ca5783c65756f96c4b4673c2b5c1eb4efc22280259441, 0x06d601211e8866dc5b50dc48a244dd7c00000000000000000000000000000000, 0x20, 0x344b6c73f71b11c56aba0d01b7d8ad83559f209d0a4101a515f6ad54c89771, 0x08, 0x40597307000000000000000000000000000000000000000000000000000000, 0x60, 0x945caaf82d18e78c033927d51f452ebcd76524497b91d7a11219cb3db6a1d369, 0x7595fc095ce489e46b2ef129591f2f6d079be4faaf345a02c5eb133c072e7c56, 0x0c6c3617eee66b4b878165c502357d49485326bc6b31bc96873f308c8f19c09d, 0x08, 0x0100000000000000000000000000000000000000000000000000000000000000
// get_deposit_root() -> 0x40255975859377d912c53aa853245ebd939bdd2b33a28e084babdcc1ed8238ee
// gas irOptimized: 108994
// gas irOptimized: 108914
// gas legacy: 142744
// gas legacyOptimized: 117570
// get_deposit_count() -> 0x20, 8, 0x0200000000000000000000000000000000000000000000000000000000000000
4 changes: 2 additions & 2 deletions test/libsolidity/semanticTests/externalContracts/snark.sol
Expand Up @@ -294,11 +294,11 @@ contract Test {
// f() -> true
// g() -> true
// pair() -> true
// gas irOptimized: 269697
// gas irOptimized: 269701
// gas legacy: 275206
// gas legacyOptimized: 266925
// verifyTx() -> true
// ~ emit Verified(string): 0x20, 0x16, "Successfully verified."
// gas irOptimized: 782210
// gas irOptimized: 782238
// gas legacy: 801868
// gas legacyOptimized: 770942
Expand Up @@ -38,8 +38,8 @@ contract test {
}
// ----
// constructor(), 20 wei ->
// gas irOptimized: 120226
// gas irOptimized code: 132400
// gas irOptimized: 120912
// gas irOptimized code: 140000
// gas legacy: 130568
// gas legacy code: 261000
// gas legacyOptimized: 121069
Expand Down
Expand Up @@ -37,8 +37,8 @@ contract test {
}
// ----
// constructor(), 20 wei ->
// gas irOptimized: 120226
// gas irOptimized code: 132400
// gas irOptimized: 120912
// gas irOptimized code: 140000
// gas legacy: 130568
// gas legacy code: 261000
// gas legacyOptimized: 121069
Expand Down
Expand Up @@ -22,6 +22,6 @@ contract Test {
// ----
// library: Lib
// f() -> 4, 0x11
// gas irOptimized: 111560
// gas irOptimized: 111629
// gas legacy: 132935
// gas legacyOptimized: 118023
Expand Up @@ -44,7 +44,7 @@ contract C {
}
// ----
// to_state() -> 0x20, 0x60, 0xa0, 7, 3, 0x666F6F0000000000000000000000000000000000000000000000000000000000, 2, 13, 14
// gas irOptimized: 121511
// gas irOptimized: 121499
// gas legacy: 123117
// gas legacyOptimized: 121655
// to_storage() -> 0x20, 0x60, 0xa0, 7, 3, 0x666F6F0000000000000000000000000000000000000000000000000000000000, 2, 13, 14
Expand Down
Expand Up @@ -52,14 +52,14 @@ contract C {
}
// ----
// from_memory() -> 0x20, 0x60, 0xa0, 0x15, 3, 0x666F6F0000000000000000000000000000000000000000000000000000000000, 2, 13, 14
// gas irOptimized: 122952
// gas irOptimized: 122923
// gas legacy: 130136
// gas legacyOptimized: 128650
// from_state() -> 0x20, 0x60, 0xa0, 21, 3, 0x666F6F0000000000000000000000000000000000000000000000000000000000, 2, 13, 14
// gas irOptimized: 121622
// gas irOptimized: 121616
// gas legacy: 123191
// gas legacyOptimized: 121760
// from_calldata((bytes,uint16[],uint16)): 0x20, 0x60, 0xa0, 21, 3, 0x666F6F0000000000000000000000000000000000000000000000000000000000, 2, 13, 14 -> 0x20, 0x60, 0xa0, 0x15, 3, 0x666F6F0000000000000000000000000000000000000000000000000000000000, 2, 13, 14
// gas irOptimized: 115043
// gas irOptimized: 115037
// gas legacy: 122425
// gas legacyOptimized: 120696
8 changes: 4 additions & 4 deletions test/libsolidity/semanticTests/structs/copy_to_mapping.sol
Expand Up @@ -45,18 +45,18 @@ contract C {
}
// ----
// from_state() -> 0x20, 0x60, 0xa0, 21, 3, 0x666F6F0000000000000000000000000000000000000000000000000000000000, 2, 13, 14
// gas irOptimized: 121599
// gas irOptimized: 121593
// gas legacy: 123053
// gas legacyOptimized: 121700
// from_storage() -> 0x20, 0x60, 0xa0, 21, 3, 0x666F6F0000000000000000000000000000000000000000000000000000000000, 2, 13, 14
// gas irOptimized: 121644
// gas irOptimized: 121638
// gas legacy: 123102
// gas legacyOptimized: 121752
// from_memory() -> 0x20, 0x60, 0xa0, 21, 3, 0x666F6F0000000000000000000000000000000000000000000000000000000000, 2, 13, 14
// gas irOptimized: 122860
// gas irOptimized: 122831
// gas legacy: 129997
// gas legacyOptimized: 128646
// from_calldata((bytes,uint16[],uint16)): 0x20, 0x60, 0xa0, 21, 3, 0x666F6F0000000000000000000000000000000000000000000000000000000000, 2, 13, 14 -> 0x20, 0x60, 0xa0, 21, 3, 0x666f6f0000000000000000000000000000000000000000000000000000000000, 2, 13, 14
// gas irOptimized: 114958
// gas irOptimized: 114952
// gas legacy: 118210
// gas legacyOptimized: 115324
Expand Up @@ -29,6 +29,6 @@ contract C {
}
// ----
// f() -> 0x20, 7, 8, 9, 0xa0, 13, 2, 0x40, 0xa0, 2, 3, 4, 2, 3, 4
// gas irOptimized: 197082
// gas irOptimized: 197116
// gas legacy: 199891
// gas legacyOptimized: 196817
Expand Up @@ -49,11 +49,11 @@ contract C {
}
// ----
// test_f() -> true
// gas irOptimized: 122078
// gas irOptimized: 122094
// gas legacy: 125322
// gas legacyOptimized: 122709
// test_g() -> true
// gas irOptimized: 106215
// gas irOptimized: 106271
// gas legacy: 111120
// gas legacyOptimized: 106964
// addresses(uint256): 0 -> 0x18
Expand Down
Expand Up @@ -19,6 +19,6 @@ contract C {
}
// ----
// g() -> 2, 6
// gas irOptimized: 178263
// gas irOptimized: 178277
// gas legacy: 180657
// gas legacyOptimized: 179156
Expand Up @@ -20,18 +20,46 @@
// {
// {
// f()
// f()
// f()
// f_72()
// f_73()
// sstore(0, 1)
// }
// function f()
// {
// let a := 1
// let b := 10
// let a_1 := calldataload(0)
// let _1 := iszero(a_1)
// for { } iszero(b) { b := add(b, not(0)) }
// {
// a := a_1
// mstore(a_1, 0)
// if _1 { leave }
// }
// }
// function f_72()
// {
// let a := 2
// let b := 10
// let a_1 := calldataload(0)
// let _1 := iszero(a_1)
// for { } iszero(b) { b := add(b, not(0)) }
// {
// a := a_1
// mstore(a_1, 0)
// if _1 { leave }
// }
// }
// function f_73()
// {
// let a := 3
// let b := 10
// let a := calldataload(0)
// let _1 := iszero(a)
// let a_1 := calldataload(0)
// let _1 := iszero(a_1)
// for { } iszero(b) { b := add(b, not(0)) }
// {
// mstore(a, 0)
// a := a_1
// mstore(a_1, 0)
// if _1 { leave }
// }
// }
Comment on lines -23 to 65
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the changes show up not only as gas differences. Two tests also got updated output.

In this case it looks like a clear downgrade. The optimizer is definitely leaving some possible optimizations on the table. Still, not sure how common this is in practice. I'm going to play with this when tweaking the optimizer sequence.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, the optimizer is mainly being rather stupid here - it specializes all three calls and then trusts that since the arguments are unused, the result will collapse to identical functions that can be deduplicated... the simpler choice would have been to not specialize in the first place.
For cases like this it's not worth preserving optimizations that work out by chance. Even if someone thought it funny to include it as test case :-).

Expand Down