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

Use MCOPY when copying byte arrays #14834

Merged
merged 2 commits into from
Feb 19, 2024
Merged

Conversation

nikola-matic
Copy link
Collaborator

Part of #14741.

@nikola-matic nikola-matic added this to the 0.8.25 milestone Feb 6, 2024
@nikola-matic nikola-matic force-pushed the use_mcopy_in_code_generation branch 2 times, most recently from 72a61bb to 6644a74 Compare February 7, 2024 07:19
@nikola-matic nikola-matic marked this pull request as ready for review February 7, 2024 07:21
@nikola-matic nikola-matic self-assigned this Feb 7, 2024
Copy link
Member

@ekpyron ekpyron left a comment

Choose a reason for hiding this comment

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

Looks good in general - some minor comments, though

libsolidity/codegen/YulUtilFunctions.cpp Outdated Show resolved Hide resolved
libsolidity/codegen/YulUtilFunctions.cpp Outdated Show resolved Hide resolved
Copy link
Member

@cameel cameel left a comment

Choose a reason for hiding this comment

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

The things that the PR touches seem fine. Still, I'm not yet convinced that it shouldn't be touching more. E.g. are there no gas updates at all, which seems odd.

Perhaps we should have a heavier semantic test with functions that return strings. Or use abi.encode() with strings. I don't think that returning bytes is that rare in real contracts, so I can't believe we're seeing zero gains here. Have you checked external test benchmarks?

@@ -0,0 +1 @@
--evm-version cancun --no-cbor-metadata --via-ir --optimize --ir --debug-info none
Copy link
Member

Choose a reason for hiding this comment

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

There's not much point in using --optimize if you don't want to see the optimized output :)

Suggested change
--evm-version cancun --no-cbor-metadata --via-ir --optimize --ir --debug-info none
--evm-version cancun --no-cbor-metadata --via-ir --optimize --ir-optimized --debug-info none

Same in the other test.

Comment on lines +7 to +8
bytes memory ret = "aaaaa";
return ret;
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this bit is even necessary to trigger the copyToMemoryFunction() code path. It's just the fact that the function is external and returns bytes memory, which means there's ABI encoding needed. The body can be left empty to make the output slightly shorter.

Copy link
Member

@cameel cameel Feb 9, 2024

Choose a reason for hiding this comment

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

By the way, I see two code paths that use copyToMemoryFunction() and here's a snippet that would trigger the other one (decoding a byte array stored in memory):

contract C {
    function foo() external pure {
        abi.decode("abcd", (bytes));
    }
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've added this test case in a separate commit so you can take a look. There's no mcopy in there (or rather much of anything) since the whole thing likely gets optimized away as unused (not the mcopy op by itself, rather the memory_to_memory_copy_.... function), which is why I pushed it without the --optimize flag. I could capture the return value from the abi.decode(...) call and return it, but that would essentially just be a duplicate of the other test, which would make it redundant.

@cameel
Copy link
Member

cameel commented Feb 9, 2024

Ah wait. Lack of gas cost updates comes from the fact that we show gas only for the default EVM (i.e. still shanghai). Still, we should have a look at gas differences in some way before we merge this.

@cameel
Copy link
Member

cameel commented Feb 9, 2024

I went over all uses of mstore in the codegen and the function handled in this PR seems to be the only one in the IR pipeline that can benefit from mcopy. But I found two places in the legacy codegen that seem to be very low-hanging fruit and I think they're worth covering too:

  1. memoryCopy32()

    for { let i := 0 } lt(i, len) { i := add(i, 32) } {
    mstore(add(dst, i), mload(add(src, i)))

  2. memoryCopy()

    // copy 32 bytes at once
    for
    {}
    iszero(lt(len, 32))
    {
    dst := add(dst, 32)
    src := add(src, 32)
    len := sub(len, 32)
    }
    { mstore(dst, mload(src)) }

These two functions are used as a part of legacy ABI encoding and decoding. They're probably also the only places in the legacy codegen where mcopy is viable. I was not paying as much attention to it as to IR but at least I haven't seen Yul snippets that are simple mload/mstore loops.

@cameel
Copy link
Member

cameel commented Feb 11, 2024

I created a PR that will allow you to compare cancun costs: #14844.

You can first run isoltest with --enforce-gas-cost --accept-updates --evm-version cancun on develop, commit them, rebase a copy of your branch on that and run it again and look at the diff.

You can even get gas_diff_stats.py to work on it if you replace hard-coded origin/develop with the commit that contains cancun costs on develop. The script should really be modified to accept it as an option.

@nikola-matic
Copy link
Collaborator Author

I created a PR that will allow you to compare cancun costs: #14844.

You can first run isoltest with --enforce-gas-cost --accept-updates --evm-version cancun on develop, commit them, rebase a copy of your branch on that and run it again and look at the diff.

You can even get gas_diff_stats.py to work on it if you replace hard-coded origin/develop with the commit that contains cancun costs on develop. The script should really be modified to accept it as an option.

When you say run it on develop, I assume you mean - run it on top of your branch, and then subsequently rebase this branch on top of yours, and run it again to compare?

@cameel
Copy link
Member

cameel commented Feb 12, 2024

Yeah. It will be the way I wrote above when the PR gets merged :) For now you have to work on top of it or temporarily cherry-pick it into the branches you want to measure.

@ekpyron
Copy link
Member

ekpyron commented Feb 12, 2024

I went over all uses of mstore in the codegen and the function handled in this PR seems to be the only one in the IR pipeline that can benefit from mcopy. But I found two places in the legacy codegen that seem to be very low-hanging fruit and I think they're worth covering too:

1. [`memoryCopy32()`](https://github.com/ethereum/solidity/blob/v0.8.24/libsolidity/codegen/CompilerUtils.cpp#L665-L679)
   https://github.com/ethereum/solidity/blob/e11b9ed9f2c254bc894d844c0a64a0eb76bbb4fd/libsolidity/codegen/CompilerUtils.cpp#L671-L672

2. [`memoryCopy()`](https://github.com/ethereum/solidity/blob/v0.8.24/libsolidity/codegen/CompilerUtils.cpp#L681-L708)
   https://github.com/ethereum/solidity/blob/e11b9ed9f2c254bc894d844c0a64a0eb76bbb4fd/libsolidity/codegen/CompilerUtils.cpp#L687-L696

These two functions are used as a part of legacy ABI encoding and decoding. They're probably also the only places in the legacy codegen where mcopy is viable. I was not paying as much attention to it as to IR but at least I haven't seen Yul snippets that are simple mload/mstore loops.

Both of those are just called from ArrayUtils::copyArrayToMemory, though - which in turn is only called in three places, once in a convertType routine (but only with non-memory sources, so it doesn't apply) and once each for abiDecode and encodeToMemory - but in each of those cases, the beginning of the respective function bails out to ABIEncoderV2, so both only apply to ABIEncoderV1...

If what I just said is correct, then there's little reason for dealing with any of it...

@cameel
Copy link
Member

cameel commented Feb 12, 2024

You're right. I did not notice that it was only for v1.

So we could ignore it, but it's simple enough to rewrite with mcopy() that I'd probably do it anyway. v1 is still there and I think some optimizors will still occasionally disable it when they notice it's cheaper.

@nikola-matic
Copy link
Collaborator Author

You're right. I did not notice that it was only for v1.

So we could ignore it, but it's simple enough to rewrite with mcopy() that I'd probably do it anyway. v1 is still there and I think some optimizors will still occasionally disable it when they notice it's cheaper.

I do kinda agree with you Kamil, but since this is my first time touching code gen, I'd rather err on the side of caution, so I'll go for Daniel's suggestion - i.e. not to touch it.

@nikola-matic
Copy link
Collaborator Author

nikola-matic commented Feb 13, 2024

Gas diffs

Click for a table of gas differences
File name IR-optimized (%) Legacy-Optimized (%) Legacy (%)
byte_array_to_storage_cleanup.sol -1.41405 -1.03113 -0.790526
structs/copy_substructures_from_mapping.sol -0.0715472 -0.0886969 -0.0738588
structs/copy_to_mapping.sol -0.0722878 -0.0885511 -0.0735761
structs/copy_substructures_to_mapping.sol -0.0725246 -0.0872304 -0.0726016
structs/copy_from_mapping.sol -0.0716161 -0.088702 -0.0739423
inlineAssembly/transient_storage_low_level_calls.sol 0 0 0
inlineAssembly/tstore_hidden_staticcall.sol 0 0 0
inlineAssembly/transient_storage_selfdestruct.sol 0 0 0
various/create_calldata.sol -5.04472 -3.59451 -3.23046
various/skip_dynamic_types_for_structs.sol -0.118784 -0.121534 -0.100642
various/address_code.sol -4.11028 -3.23681 -2.32262
various/selfdestruct_post_cancun.sol -46.8306 -53.8018 -0.514521
various/selfdestruct_post_cancun_redeploy.sol 14217.7 13654.2 15328
various/selfdestruct_post_cancun_multiple_beneficiaries.sol 0 0 0
constructor/bytes_in_constructors_packer.sol -2.37766 -1.7475 -1.35926
constructor/bytes_in_constructors_unpacker.sol -3.44285 -2.16702 -2.17105
externalContracts/strings.sol -0.995043 -0.691855 -0.531754
externalContracts/base64.sol -1.54645 -0.95041 -0.764026
externalContracts/deposit_contract.sol -2.40797 -1.58466 -1.11672
externalContracts/FixedFeeRegistrar.sol -1.23554 -0.997635 -0.615488
functionCall/external_call_to_nonexisting_debugstrings.sol -1.43983 -0.998132 -0.694087
functionCall/external_call_to_nonexisting.sol -2.05828 -1.77251 -1.27219
abiEncoderV1/abi_encode_calldata_slice.sol -3.51323 -4.23426 -3.40039
salted_create/prediction_example.sol 0 0 -0.717873
abiEncoderV2/storage_array_encoding.sol -0.319978 -0.317854 -0.300438
abiEncoderV2/calldata_array.sol -2.40414 0 0
abiEncoderV2/abi_encode_calldata_slice.sol -3.51323 -4.23426 -3.40039
abiEncoderV2/abi_encode_v2.sol -0.81861 -0.822585 -0.785755
abiEncoderV2/calldata_overlapped_dynamic_arrays.sol -0.221243 -0.21411 -0.208068
abiencodedecode/abi_decode_simple_storage.sol -0.15406 -0.157297 -0.132578
array/pop/byte_array_pop_masking_long.sol -0.149055 -0.136061 -0.134258
array/copying/copy_byte_array_in_struct_to_storage.sol -0.230424 -0.221641 -0.22427
array/copying/bytes_storage_to_storage.sol -1.23159 -0.0907571 -0.0899201
array/copying/function_type_array_to_storage.sol -0.0900446 -0.0804961 -0.0821
array/copying/storage_memory_nested_bytes.sol -0.280667 -0.231121 -0.229494

@cameel
Copy link
Member

cameel commented Feb 13, 2024

Did you run against cancun (rather than shanghai) on develop? This is reporting 14000% increase for a selfdestruct test, which is wild.

@nikola-matic
Copy link
Collaborator Author

nikola-matic commented Feb 13, 2024

Did you run against cancun (rather than shanghai) on develop? This is reporting 14000% increase for a selfdestruct test, which is wild.

I merged your PR and ran nothing on develop - I just ran it on my own branch and committed the updates, and then ran gas_diff_stats.py.

This is the diff on just divelop for the test in question:

[nikola@archlinux solidity]$ git diff test/libsolidity/semanticTests/various/selfdestruct_post_cancun_redeploy.sol
diff --git a/test/libsolidity/semanticTests/various/selfdestruct_post_cancun_redeploy.sol b/test/libsolidity/semanticTests/various/selfdestruct_post_cancun_redeploy.sol
index 473571bf1..c7e99ae87 100644
--- a/test/libsolidity/semanticTests/various/selfdestruct_post_cancun_redeploy.sol
+++ b/test/libsolidity/semanticTests/various/selfdestruct_post_cancun_redeploy.sol
@@ -82,14 +82,15 @@ contract D {
 // EVMVersion: >=cancun
 // ----
 // constructor(), 1 ether ->
-// gas irOptimized: 562058
-// gas legacy: 633310
-// gas legacyOptimized: 590051
+// gas irOptimized: 430265
+// gas legacy: 690264
+// gas legacyOptimized: 412819
 // exists() -> false
 // test_deploy_and_terminate() ->
 // ~ emit Deployed(address,bytes32) from 0x137aa4dfc0911524504fcd4d98501f179bc13b4a: 0x7e6580007e709ac52945fae182c61131d42634e8, 0x1234000000000000000000000000000000000000000000000000000000000000
-// gas irOptimized: 118541
-// gas legacyOptimized: 118305
+// gas irOptimized: 117489
+// gas legacy: 118895
+// gas legacyOptimized: 117137
 // exists() -> false
 // deploy_create2() ->
 // ~ emit Deployed(address,bytes32) from 0x137aa4dfc0911524504fcd4d98501f179bc13b4a: 0x7e6580007e709ac52945fae182c61131d42634e8, 0x1234000000000000000000000000000000000000000000000000000000000000
@@ -99,3 +100,6 @@ contract D {
 // test_balance_after_selfdestruct() ->
 // exists() -> true
 // deploy_create2() -> FAILURE
+// gas irOptimized: 96903658
+// gas legacy: 96903665
+// gas legacyOptimized: 96903645

That bottom addition for contract D is kinda sus.

@cameel
Copy link
Member

cameel commented Feb 13, 2024

I merged your #14844 and ran nothing on develop

So that's the problem. The reason I said to run it on develop first was that tests that are skipped on non-default EVM version may have outdated or missing gas expectations since they're not enforced (i.e. never checked or updated by isoltest, just left as they are, even if they're wrong). That's why you need to run this on develop with --evm-version=cancun, commit your changes it locally and only then run it on your branch. Otherwise to the script it looks like it was your branch that added the whole cost.

@cameel
Copy link
Member

cameel commented Feb 13, 2024

Also, might be worth running it (both times) with lower enforcement limit. Currently it's 100k and that, times the number of gas changes in the file, is our margin of error, which is pretty high. I'd just set the limit to zero.

This way we'll also see some results for tests which are nowhere close to 100k, which would be interesting. That would be a bit less biased than the current situation where only very heavy tests are measured (well, still biased, but at least in a different way).

@nikola-matic
Copy link
Collaborator Author

I merged your #14844 and ran nothing on develop

So that's the problem. The reason I said to run it on develop first was that tests that are skipped on non-default EVM version may have outdated or missing gas expectations since they're not enforced (i.e. never checked or updated by isoltest, just left as they are, even if they're wrong). That's why you need to run this on develop with --evm-version=cancun, commit your changes it locally and only then run it on your branch. Otherwise to the script it looks like it was your branch that added the whole cost.

The results now are significantly more sane, however, I'll go through the steps I've performed just so you can make sure I've not made any mistakes again.


on develop

  1. ./test/tools/isoltest --no-smt --enforce-gas-cost --accept-updates --evm-version cancun -t semanticTests/*
  2. ./test/tools/isoltest --no-smt --optimize --enforce-gas-cost --accept-updates --evm-version cancun -t semanticTests/*
  3. git add .
  4. git commit -m "Gas costs"

on use_mcopy_in_code_generation (this branch)

  1. ./test/tools/isoltest --no-smt --enforce-gas-cost --accept-updates --evm-version cancun -t semanticTests/*
  2. ./test/tools/isoltest --no-smt --optimize --enforce-gas-cost --accept-updates --evm-version cancun -t semanticTests/*
  3. git add .
  4. git commit -m "Gas costs"

gas_diff_stats.py

  1. changed origin/develop to develop
  2. ran the script

This yields the following output:

gas costs

Click for a table of gas differences
File name IR-optimized (%) Legacy-Optimized (%) Legacy (%)
byte_array_to_storage_cleanup.sol -1.41405 -1.03113 -0.790526
structs/copy_substructures_from_mapping.sol -0.0715472 -0.0886969 -0.0738588
structs/copy_to_mapping.sol -0.0722878 -0.0885511 -0.0735761
structs/copy_substructures_to_mapping.sol -0.0725246 -0.0872304 -0.0726016
structs/copy_from_mapping.sol -0.0716161 -0.088702 -0.0739423
inlineAssembly/transient_storage_low_level_calls.sol 0 -1.01586e-06 -2.03171e-06
various/create_calldata.sol -5.04472 -3.59451 -3.23046
various/skip_dynamic_types_for_structs.sol -0.118784 -0.121534 -0.100642
various/address_code.sol -4.11028 -3.23681 -2.32262
various/selfdestruct_post_cancun_redeploy.sol -0.00541603 -0.00519328 -0.00607801
constructor/bytes_in_constructors_packer.sol -2.37766 -1.7475 -1.35926
constructor/bytes_in_constructors_unpacker.sol -3.44285 -2.16702 -2.17105
externalContracts/strings.sol -0.995043 -0.691855 -0.531754
externalContracts/base64.sol -1.54645 -0.95041 -0.764026
externalContracts/deposit_contract.sol -2.40797 -1.58466 -1.11672
externalContracts/FixedFeeRegistrar.sol -1.23554 -0.997635 -0.615488
functionCall/external_call_to_nonexisting_debugstrings.sol -1.43983 -0.998132 -0.694087
functionCall/external_call_to_nonexisting.sol -2.05828 -1.77251 -1.27219
abiEncoderV1/abi_encode_calldata_slice.sol -3.51323 -4.23426 -3.40039
salted_create/prediction_example.sol 0 0 -0.717873
abiEncoderV2/storage_array_encoding.sol -0.319978 -0.317854 -0.300438
abiEncoderV2/calldata_array.sol -2.40414 0 0
abiEncoderV2/abi_encode_calldata_slice.sol -3.51323 -4.23426 -3.40039
abiEncoderV2/abi_encode_v2.sol -0.81861 -0.822585 -0.785755
abiEncoderV2/calldata_overlapped_dynamic_arrays.sol -0.221243 -0.21411 -0.208068
abiencodedecode/abi_decode_simple_storage.sol -0.15406 -0.157297 -0.132578
array/pop/byte_array_pop_masking_long.sol -0.149055 -0.136061 -0.134258
array/copying/copy_byte_array_in_struct_to_storage.sol -0.230424 -0.221641 -0.22427
array/copying/bytes_storage_to_storage.sol -1.23159 -0.0907571 -0.0899201
array/copying/function_type_array_to_storage.sol -0.0900446 -0.0804961 -0.0821
array/copying/storage_memory_nested_bytes.sol -0.280667 -0.231121 -0.229494

@ekpyron @cameel please take a look.

@cameel
Copy link
Member

cameel commented Feb 14, 2024

./test/tools/isoltest --no-smt --enforce-gas-cost --accept-updates --evm-version cancun -t semanticTests/*

I'll take a closer look a bit later but for now just a note that I was hoping you'd use the lower enforcement limit I mentioned, i.e. --enforce-gas-cost-min-value 0.

@nikola-matic
Copy link
Collaborator Author

./test/tools/isoltest --no-smt --enforce-gas-cost --accept-updates --evm-version cancun -t semanticTests/*

I'll take a closer look a bit later but for now just a note that I was hoping you'd use the lower enforcement limit I mentioned, i.e. --enforce-gas-cost-min-value 0.

Ah, knew I was forgetting about something. I'll do another run with the zero limit.

@ekpyron
Copy link
Member

ekpyron commented Feb 14, 2024

There is some weird flukes in the data like the minimal changes in
inlineAssembly/transient_storage_low_level_calls.sol 0 -1.01586e-06 -2.03171e-06
(which I guess should be unaffected of this change)

Not sure if we should worry about that (potentially as some kind of non-determinism outside of this PR already on develop) or whether there's a harmless explanation :-). Not a concern for this PR itself, but might be worth double-checking how this behaves on develop itself.

@cameel
Copy link
Member

cameel commented Feb 14, 2024

There is some weird flukes in the data like the minimal changes in
inlineAssembly/transient_storage_low_level_calls.sol 0 -1.01586e-06 -2.03171e-06
(which I guess should be unaffected of this change)

It uses abi.encodeCall(), which does ABI encoding and returns bytes memory so it's plausible that it's affected. The infinitesimally small difference might be due to the contract deployment using new dwarfing all other costs.

@nikola-matic
Copy link
Collaborator Author

with --enforce-gas-costs-min-value 0

Click for a table of gas differences
File name IR-optimized (%) Legacy-Optimized (%) Legacy (%)
byte_array_to_storage_cleanup.sol -1.16843 -0.881466 -0.717236
deployedCodeExclusion/subassembly_deduplication.sol -0.0546573 -0.0589596 -0.0536553
strings/return_string.sol -0.413108 -0.362917 -0.371504
strings/unicode_string.sol -0.455316 -0.409017 -0.417795
strings/empty_storage_string.sol -0.287227 -0.249138 -0.285831
strings/unicode_escapes.sol -0.458445 -0.408367 -0.417374
strings/empty_string_input.sol -0.109278 -0.044847 -0.119119
strings/empty_string.sol -0.0888307 -0.0560826 -0.0876667
strings/constant_string_literal.sol -0.796318 -0.756865 -0.744315
strings/concat/string_concat_nested.sol -2.20293 -1.68521 -1.74009
strings/concat/string_concat_different_types.sol -0.836395 -0.796762 -0.774781
strings/concat/string_concat_2_args.sol -1.80918 -1.53777 -1.5835
strings/concat/string_concat_empty_argument_list.sol -0.0561535 -0.0560905 -0.0876101
strings/concat/string_concat_empty_strings.sol -1.1386 -0.608904 -0.613313
structs/copy_substructures_from_mapping.sol -0.133467 -0.165432 -0.136484
structs/copy_to_mapping.sol -0.0722878 -0.0885511 -0.0735761
structs/copy_substructures_to_mapping.sol -0.0725246 -0.0872304 -0.0726016
structs/msg_data_to_struct_member_copy.sol -1.11347 -1.07524 -1.02023
structs/copy_from_mapping.sol -0.141067 -0.174577 -0.14452
variables/public_state_overridding_dynamic_struct.sol -0.281131 -0.190523 -0.206973
variables/public_state_overridding_mapping_to_dynamic_struct.sol -0.185428 -0.117234 -0.137282
tryCatch/try_catch_library_call.sol -0.199187 -0.159609 -0.168954
tryCatch/create.sol -0.141068 -0.127503 -0.137615
tryCatch/malformed_error.sol -0.330544 -0.276968 -0.286092
tryCatch/structured.sol -0.487146 -0.386304 -0.379025
tryCatch/panic.sol -0.307139 -0.215934 -0.219736
tryCatch/lowLevel.sol -0.885192 -0.724382 -0.684615
tryCatch/nested.sol -0.49144 -0.373578 -0.362196
tryCatch/structuredAndLowLevel.sol -0.889868 -0.71647 -0.675928
tryCatch/invalid_error_encoding.sol -1.37241 -1.36185 -1.26822
viaYul/require.sol -0.151293 -0.229232 -0.247703
viaYul/mapping_string_key.sol -0.402432 -0.36781 -0.391583
viaYul/string_literals.sol -0.758752 -0.732348 -0.743732
viaYul/string_format.sol -0.628756 -0.583468 -0.581608
viaYul/conversion/explicit_string_bytes_calldata_cast.sol -0.469526 -0.391564 -0.398197
viaYul/storage/mappings.sol -0.198621 -0.188993 -0.204895
metaTypes/name_other_contract.sol -0.458185 -0.408315 -0.417304
inlineAssembly/transient_storage_low_level_calls.sol 0 -0.000253399 -0.000278625
inlineAssembly/mcopy_overlap.sol -1.11253 -1.09837 -1.06432
inlineAssembly/inline_assembly_memory_access.sol -0.413514 -0.388007 -0.38656
inlineAssembly/mcopy.sol -0.437937 -0.387119 -0.389388
inlineAssembly/mcopy_empty.sol -0.438538 -0.38729 -0.389588
inlineAssembly/calldata_assign.sol -0.484931 -0.39531 -0.401943
various/create_calldata.sol -4.36554 -3.06993 -2.84528
various/address_code_complex.sol -0.197196 -0.145864 -0.150282
various/memory_overwrite.sol -0.510346 -0.406429 -0.414938
various/string_tuples.sol -0.626607 -0.571809 -0.617103
various/skip_dynamic_types_for_structs.sol -0.118784 -0.121534 -0.100642
various/address_code.sol -3.63373 -2.80675 -2.11826
various/selfdestruct_post_cancun_redeploy.sol -0.00571341 -0.00548888 -0.00638616
constructor/bytes_in_constructors_packer.sol -2.37766 -1.7475 -1.35926
constructor/bytes_in_constructors_unpacker.sol -3.17081 -2.01976 -2.04324
externalContracts/strings.sol -0.970986 -0.688747 -0.534866
externalContracts/base64.sol -1.00358 -0.700856 -0.613361
externalContracts/deposit_contract.sol -2.30589 -1.54736 -1.12679
externalContracts/FixedFeeRegistrar.sol -0.724654 -0.647102 -0.495128
functionCall/call_attached_library_function_on_string.sol -0.273984 -0.240611 -0.241002
functionCall/external_call_to_nonexisting_debugstrings.sol -1.43983 -0.998132 -0.694087
functionCall/precompile_extcodesize_check.sol 0 -0.692339 -0.717619
functionCall/delegatecall_return_value.sol -0.458712 -0.980145 -0.775442
functionCall/external_call_to_nonexisting.sol -2.05828 -1.62639 -1.27219
abiEncoderV1/abi_encode_rational.sol -0.727863 -0.758592 -0.728915
abiEncoderV1/return_dynamic_types_cross_call_advanced.sol -1.97572 -2.0286 -1.66536
abiEncoderV1/abi_encode.sol -1.11893 -1.08284 -1.07619
abiEncoderV1/memory_dynamic_array_and_calldata_bytes.sol -1.99499 0 0
abiEncoderV1/return_dynamic_types_cross_call_out_of_range_2.sol -0.469638 -0.734903 -0.691586
abiEncoderV1/abi_encode_call.sol 0 -0.944013 -0.944364
abiEncoderV1/abi_encode_calldata_slice.sol -3.51323 -4.23426 -3.40039
abiEncoderV1/abi_encode_decode_simple.sol -1.43368 -1.41759 -1.15786
abiEncoderV1/return_dynamic_types_cross_call_simple.sol -2.15115 -2.18921 -2.05229
errors/using_structs.sol -0.501139 -0.400783 -0.397484
salted_create/prediction_example.sol -0.649626 -0.400587 -0.717873
builtinFunctions/ripemd160_packed.sol -1.3214 -1.25069 -1.2394
builtinFunctions/ripemd160.sol -0.434359 -0.36514 -0.388408
builtinFunctions/sha256_packed.sol -1.38002 -1.29893 -1.27587
builtinFunctions/sha256.sol -0.447509 -0.374127 -0.39488
fallback/fallback_override_multi.sol -0.115682 -0.055175 -0.0854855
fallback/fallback_return_data.sol -0.456766 -0.398226 -0.402601
fallback/fallback_override.sol -0.474215 -0.400237 -0.404948
fallback/fallback_override2.sol -0.115682 -0.055175 -0.0854855
fallback/fallback_argument_to_storage.sol -0.158742 -0.131224 -0.158301
fallback/fallback_argument.sol -0.0570907 -0.0273243 -0.0427514
revertStrings/library_non_view_call.sol -1.69805 -1.92966 -1.83744
abiEncoderV2/calldata_nested_array_reencode.sol -3.50139 -3.53359 -3.07779
abiEncoderV2/calldata_array_two_static.sol -1.66908 -1.6888 -1.60158
abiEncoderV2/calldata_array_dynamic_index_access.sol -3.0465 -3.09673 -3.02674
abiEncoderV2/calldata_with_garbage.sol -0.693857 -0.675655 -0.647104
abiEncoderV2/calldata_array_dynamic_static_dynamic.sol -6.85608 -7.1409 -6.87434
abiEncoderV2/storage_array_encoding.sol -0.319978 -0.317854 -0.300438
abiEncoderV2/calldata_struct_dynamic.sol -3.39227 -3.47383 -3.52378
abiEncoderV2/calldata_array.sol -2.25005 -2.73888 -2.78378
abiEncoderV2/calldata_array_static.sol -1.95188 -1.97374 -1.8699
abiEncoderV2/memory_dynamic_array_and_calldata_bytes.sol -1.99499 -1.95317 -1.78823
abiEncoderV2/calldata_array_static_index_access.sol -2.21366 -2.23046 -2.09288
abiEncoderV2/calldata_overlapped_nested_dynamic_arrays.sol -2.33658 -2.74567 -2.53933
abiEncoderV2/abi_encode_calldata_slice.sol -3.51323 -4.23426 -3.40039
abiEncoderV2/calldata_array_static_dynamic_static.sol -7.26106 -7.53278 -7.14058
abiEncoderV2/abi_encode_v2.sol -1.02577 -1.01263 -0.984996
abiEncoderV2/calldata_array_two_dynamic.sol -2.54476 -2.57747 -2.59252
abiEncoderV2/calldata_three_dimensional_dynamic_array_index_access.sol -0.7869 -0.749176 -0.718411
abiEncoderV2/abi_encode_empty_string_v2.sol -0.833143 -0.757196 -0.80254
abiEncoderV2/calldata_struct_simple.sol -0.937088 -0.928772 -0.767107
abiEncoderV2/calldata_array_struct_dynamic.sol -4.21325 -4.31783 -4.38088
abiEncoderV2/memory_dynamic_array_and_calldata_static_array.sol -1.22556 -1.35946 -1.25804
abiEncoderV2/abi_encode_rational_v2.sol -0.727863 -0.758592 -0.728915
abiEncoderV2/calldata_struct_array_reencode.sol -2.45991 -2.82774 -2.40971
abiEncoderV2/calldata_overlapped_dynamic_arrays.sol -0.675221 -0.657997 -0.630542
abiEncoderV2/calldata_array_multi_dynamic.sol -5.44171 -5.579 -5.48233
abiEncoderV2/calldata_array_dynamic.sol -2.58309 -2.6314 -2.59142
abiEncoderV2/cleanup/static_array.sol -2.25191 -2.29312 -2.17797
abiEncoderV2/cleanup/simple_struct.sol -2.26023 -2.29617 -2.18596
abiEncoderV2/cleanup/dynamic_array.sol -3.32001 -3.38507 -3.38795
abiEncoderV2/cleanup/reencoded_calldata_string.sol -2.84148 -2.87188 -2.75143
specialFunctions/abi_encode_with_signature_from_string.sol -0.996256 -1.5378 -1.34105
userDefinedValueType/abicodec.sol 0 -1.27942 -1.15484
libraries/internal_library_function_attached_to_string_accepting_storage.sol -0.398084 -0.356974 -0.385348
libraries/library_delegatecall_guard_view_staticcall.sol 0 -0.475968 -0.483608
libraries/internal_library_function_attached_to_literal.sol -1.51329 -1.16654 -1.20882
libraries/attached_public_library_function_returning_calldata.sol -1.80602 -1.7696 -1.49234
libraries/attached_public_library_function_accepting_calldata.sol.sol -0.729658 -0.646393 -0.628714
libraries/library_address_via_module.sol 0 -0.610931 -0.624049
libraries/library_address_homestead.sol 0 -0.883623 -0.883982
libraries/library_delegatecall_guard_pure.sol 0 -0.614532 -0.629976
libraries/library_delegatecall_guard_view_not_needed.sol 0 -0.614532 -0.629976
libraries/library_delegatecall_guard_view_needed.sol 0 -0.566835 -0.582517
libraries/library_address.sol 0 -0.610931 -0.624049
libraries/library_function_selectors_struct.sol 0 -0.587874 -0.597277
libraries/library_function_selectors.sol 0 -2.57463 -2.34715
abiencodedecode/abi_encode_call_is_consistent.sol -1.70502 -1.22077 -1.2046
abiencodedecode/abi_decode_simple_storage.sol -0.15406 -0.157297 -0.132578
abiencodedecode/abi_encode_with_selector.sol -1.04924 0 0
abiencodedecode/abi_decode_simple.sol -0.899428 -0.910096 -0.740259
abiencodedecode/abi_decode_calldata.sol -0.49544 -0.380771 -0.378993
abiencodedecode/abi_encode_call_special_args.sol -0.902484 -0.867348 -0.838922
abiencodedecode/abi_encode_call.sol -0.687467 -5.37257 -4.98143
abiencodedecode/abi_encode_with_signature.sol -1.11688 0 0
abiencodedecode/abi_encode_call_declaration.sol 0 -2.65994 -2.26515
abiencodedecode/abi_encode_call_uint_bytes.sol -3.49264 -3.55901 -3.35712
abiencodedecode/abi_encode_with_selectorv2.sol -1.63665 -1.6176 -1.5711
abiencodedecode/abi_encode_with_signaturev2.sol -1.60571 -1.95016 -1.90345
abiencodedecode/contract_array.sol -1.93805 -1.7227 -1.55698
abiencodedecode/contract_array_v2.sol -1.93805 -1.7227 -1.55698
array/invalid_encoding_for_storage_byte_array.sol -0.411938 -0.36748 -0.374425
array/string_allocation_bug.sol -0.629192 -0.558584 -0.59384
array/inline_array_strings_from_document.sol -0.428237 -0.398641 -0.40502
array/strings_in_struct.sol -0.379451 -0.340904 -0.370597
array/string_literal_assign_to_storage_bytes.sol -0.413681 -0.363044 -0.371633
array/inline_array_storage_to_memory_conversion_strings.sol -0.743494 -0.669517 -0.722394
array/array_memory_allocation/array_array_static.sol -0.30152 -0.24235 -0.25413
array/array_memory_allocation/array_2d_zeroed_memory_index_access.sol -0.361149 -0.3146 -0.328636
array/array_memory_allocation/array_static_zeroed_memory_index_access.sol -0.379109 -0.331881 -0.352795
array/array_memory_allocation/array_static_return_param_zeroed_memory_index_access.sol -0.309428 -0.321195 -0.32557
array/array_memory_allocation/array_zeroed_memory_index_access.sol -0.370831 -0.326296 -0.344153
array/pop/byte_array_pop_masking_long.sol -0.149055 -0.136061 -0.134258
array/pop/byte_array_pop_copy_long.sol -0.0832678 -0.0722066 -0.0746971
array/indexAccess/index_access.sol -0.510435 -0.388384 -0.39192
array/indexAccess/inline_array_index_access_strings.sol -0.413845 -0.363216 -0.371808
array/copying/calldata_bytes_array_to_memory.sol -0.509174 -0.381746 -0.385986
array/copying/copy_byte_array_in_struct_to_storage.sol -0.206027 -0.193602 -0.20049
array/copying/calldata_dyn_2d_bytes_to_memory.sol -0.882536 -0.682089 -0.694922
array/copying/calldata_2d_bytes_to_memory.sol -0.412774 -0.375747 -0.367736
array/copying/bytes_storage_to_storage.sol -1.17427 -0.0912084 -0.090989
array/copying/function_type_array_to_storage.sol -0.112385 -0.100251 -0.102023
array/copying/storage_memory_nested_bytes.sol -0.280667 -0.231121 -0.229494
array/concat/bytes_concat_empty_argument_list.sol -0.0561535 -0.0560905 -0.0876101
array/concat/bytes_concat_different_types.sol -0.93338 -0.841829 -0.83534
array/concat/bytes_concat_empty_strings.sol -1.1386 -0.608904 -0.613313
array/concat/bytes_concat_2_args.sol -1.80918 -1.53777 -1.5835
array/concat/bytes_concat_nested.sol -2.20293 -1.68521 -1.74009
array/concat/bytes_concat_3_args.sol -1.857 -1.48092 -1.50622
array/concat/bytes_concat_as_argument.sol -0.893026 -0.696776 -0.729254
array/slices/array_slice_calldata_as_argument_of_external_calls.sol -0.550443 -0.374564 -0.370974
getters/string_and_bytes.sol -0.379352 -0.341863 -0.37163
getters/bytes.sol -0.545325 -0.363546 -0.372142
getters/mapping_of_string.sol -0.413308 0 0
getters/struct_with_bytes.sol -0.470207 -0.333966 -0.341105
getters/struct_with_bytes_simple.sol -0.470207 -0.333966 -0.341105
literals/hex_string_with_underscore.sol -0.455899 -0.409226 -0.418006
constants/constants_at_file_level_referencing.sol -0.458107 -0.408144 -0.41708
constants/constant_string_at_file_level.sol -0.458616 -0.408662 -0.417584
constants/constant_string.sol -0.458694 -0.408662 -0.417584
smoke/fallback.sol -0.493931 -0.363531 -0.372127
smoke/structs.sol -0.488473 -0.403707 -0.408493
smoke/failure.sol -0.120672 -0.0553991 -0.0861717
smoke/arrays.sol -1.02012 -0.946541 -0.991493
smoke/bytes_and_strings.sol -0.55182 -0.502072 -0.538466
isoltestTesting/format_raw_string_with_control_chars.sol -0.533513 -0.391912 -0.393871
shanghai/evmone_support.sol -0.439443 -0.362468 -0.371413
calldata/calldata_internal_function_pointer.sol -0.699471 -0.658899 -0.636023
calldata/calldata_internal_library.sol -0.402163 -0.388007 -0.388906
calldata/calldata_memory_mixed.sol -1.10831 -0.99215 -0.981379
calldata/calldata_string_array.sol -0.519709 -0.382442 -0.385087
calldata/copy_from_calldata_removes_bytes_data.sol -0.0406737 -0.192239 -0.0689456
calldata/calldata_bytes_external.sol -0.36415 -0.415149 -0.369513
calldata/calldata_bytes_to_memory_encode.sol -1.19499 -1.06091 -1.01679
conversions/string_to_bytes.sol -0.536689 -0.39423 -0.396134

@nikola-matic
Copy link
Collaborator Author

There is some weird flukes in the data like the minimal changes in inlineAssembly/transient_storage_low_level_calls.sol 0 -1.01586e-06 -2.03171e-06 (which I guess should be unaffected of this change)

Not sure if we should worry about that (potentially as some kind of non-determinism outside of this PR already on develop) or whether there's a harmless explanation :-). Not a concern for this PR itself, but might be worth double-checking how this behaves on develop itself.

Just FYI, this is the diff between this branch and develop (with the enforced gas tests):

diff --git a/test/libsolidity/semanticTests/inlineAssembly/transient_storage_low_level_calls.sol b/test/libsolidity/semanticTests/inlineAssembly/transient_storage_low_level_calls.sol
index f4ba81a10..0e4cdd37b 100644
--- a/test/libsolidity/semanticTests/inlineAssembly/transient_storage_low_level_calls.sol
+++ b/test/libsolidity/semanticTests/inlineAssembly/transient_storage_low_level_calls.sol
@@ -66,17 +66,17 @@ contract C {
 // ----
 // testDelegateCall() -> true
 // gas irOptimized: 74557
-// gas legacy: 86825
-// gas legacyOptimized: 73487
+// gas legacy: 86734
+// gas legacyOptimized: 73404
 // testCall() -> true
 // gas irOptimized: 74501
-// gas legacy: 86786
-// gas legacyOptimized: 73439
+// gas legacy: 86695
+// gas legacyOptimized: 73356
 // tloadAllowedStaticCall() -> true
 // gas irOptimized: 73915
-// gas legacy: 86229
-// gas legacyOptimized: 72882
+// gas legacy: 86138
+// gas legacyOptimized: 72799
 // tstoreNotAllowedStaticCall() -> true
 // gas irOptimized: 98438720
-// gas legacy: 98439088
-// gas legacyOptimized: 98438763
+// gas legacy: 98439086
+// gas legacyOptimized: 98438762

Copy link
Member

@ekpyron ekpyron left a comment

Choose a reason for hiding this comment

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

Is there any good reason for not having merged this already a week ago?

@nikola-matic
Copy link
Collaborator Author

Is there any good reason for not having merged this already a week ago?

No one approved it.

@nikola-matic nikola-matic merged commit d8c3ca2 into develop Feb 19, 2024
69 checks passed
@nikola-matic nikola-matic deleted the use_mcopy_in_code_generation branch February 19, 2024 12:48
@ekpyron
Copy link
Member

ekpyron commented Feb 19, 2024

Is there any good reason for not having merged this already a week ago?

No one approved it.

Same question, though.

@nikola-matic
Copy link
Collaborator Author

Is there any good reason for not having merged this already a week ago?

No one approved it.

Same question, though.

Kamil wanted the gas reports; aside from that, I don't think there was anything egregiously wrong with the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants