Investigate effects of disabling libevmasm CSE after the Yul optimizer.

ekpyron commented 1 year ago

We're currently running the full libevmasm optimizer after the Yul optimizer, including complex parts like libevmasm's CommonSubexpressionEliminator.

There is multiple reasons for this being suboptimal:

Accurately transporting debugging data across the libevmasm CSE may be challenging.
The libevmasm CSE has seen little development or maintanence in recent years.
Running complex transformations on the assembly may skew our evaluation of the Yul optimizer. Ideally, most complex transformations are done on the Yul level (and if necessary on the level of the EVM code transform), and on the assembly level we'd merely perform transformations based on concrete opcode sequences (like the low-level inliner and the libevmasm peephole optimizer).

However, disabling the libevmasm CSE currently still induces gas and/or code size regressions. Conversely, libevmasm CSE can also apparently actually hurt gas costs. The range seems to be around +/- 1% runtime gas or deployment cost.

So what we should do is:

Properly evaluate the effect of disabling the libevmasm CSE after the Yul optimizer.
Examine the worst regression cases and try to find flaws in the Yul optimizer or code transform and schedule solving them as separate tasks.

matheusaaguiar commented 8 months ago

I disabled the CSE optimization and ran the benchmark_diff.py script which generated the table below. The relevant section is ir-optimize-evm+yul. I left the other ones, because they can be useful to compare the impact of the CSE alone without the yul optimizations taking place before, like in ir-optimize-evm-only. Unfortunately, the increase in gas cost is significant in some tests, with ens deployment cost increase standing out at 2.93%

Benchmark Statistics

`ir-no-optimize`

project	bytecode_size	deployment_gas	method_gas
brink	`0%`
colony	`0%`
elementfi	`0%`
ens	`0%`
perpetual-pools	`0%`	`-0%`	`+0.03% ❌`
uniswap	`0%`	`-0%`	`+0%`
yield_liquidator	`0%`	`+0%`	`0%`
zeppelin

`ir-optimize-evm+yul`

project	bytecode_size	deployment_gas	method_gas
brink	`+0.96% ❌`
colony	`0%`
elementfi	`+1.18% ❌`
ens	`+1.05% ❌`	`+2.93% ❌`	`+0.05% ❌`
perpetual-pools	`+0.92% ❌`	`+0.86% ❌`	`+0.09% ❌`
uniswap	`+1.36% ❌`	`+1.28% ❌`	`+1.42% ❌`
yield_liquidator	`+1.1% ❌`	`+1.14% ❌`	`+0.03% ❌`
zeppelin	`+1.62% ❌`	`+1.6% ❌`	`+0.05% ❌`

`ir-optimize-evm-only`

project	bytecode_size	deployment_gas	method_gas
brink	`+15.73% ❌`
colony	`0%`
elementfi	`+9.84% ❌`
ens	`+13.43% ❌`	`+11.33% ❌`	`+1.56% ❌`
perpetual-pools	`+11.85% ❌`	`+10.42% ❌`	`+3.62% ❌`
uniswap	`+10.42% ❌`	`+10.29% ❌`	`+9.38% ❌`
yield_liquidator	`+13.24% ❌`	`+12.68% ❌`	`+1.33% ❌`
zeppelin	`+12.54% ❌`

`legacy-no-optimize`

project	bytecode_size	deployment_gas	method_gas
brink	`0%`
colony	`0%`
elementfi	`0%`
ens	`0%`
perpetual-pools	`0%`	`-0%`	`-0.01% ✅`
uniswap	`0%`	`0%`	`+0%`
yield_liquidator	`0%`	`0%`	`+0%`
zeppelin	`0%`	`+0%`	`-0.04% ✅`

`legacy-optimize-evm+yul`

project	bytecode_size	deployment_gas	method_gas
brink	`0%`
colony	`0%`
elementfi	`0%`
ens	`0%`	`0%`	`0%`
perpetual-pools	`0%`	`+0%`	`+0.01% ❌`
uniswap	`0%`	`-0%`	`-0%`
yield_liquidator	`0%`	`+0%`	`0%`
zeppelin	`0%`	`-0%`	`+0.17% ❌`

`legacy-optimize-evm-only`

project	bytecode_size	deployment_gas	method_gas
brink	`0%`
colony	`0%`
elementfi	`0%`
ens	`0%`	`+0%`	`0%`
perpetual-pools	`0%`	`+0%`	`-0.02% ✅`
uniswap	`0%`	`-0%`	`+0%`
yield_liquidator	`0%`	`-0%`	`+0%`
zeppelin	`0%`	`-0%`	`+0.06% ❌`

!V = version mismatch !B = no value in the "before" version !A = no value in the "after" version !T = one or both values were not numeric and could not be compared -0 = very small negative value rounded to zero +0 = very small positive value rounded to zero

matheusaaguiar commented 8 months ago

This is the output of gas_diff.py.

Click for a table of gas differences

| File name | IR optimized | Legacy optimized | Legacy | |------------------------------------------------------------------------------------------------|----------------|--------------------|----------| | `abiEncoderV1/abi_decode_v2_storage.sol` | +0% | | | | `abiEncoderV1/abi_encode_calldata_slice.sol` | -0% | | | | `abiEncoderV1/struct/struct_storage_ptr.sol` | +0% | | | | `abiEncoderV2/abi_encode_calldata_slice.sol` | -0% | | | | `abiEncoderV2/abi_encode_v2.sol` | +0% | | | | `abiEncoderV2/abi_encode_v2_in_function_inherited_in_v1_contract.sol` | +0% | | | | `abiEncoderV2/calldata_array.sol` | -0% | | | | `abiEncoderV2/calldata_overlapped_dynamic_arrays.sol` | +0% | | | | `abiEncoderV2/storage_array_encoding.sol` | +0% | | | | `abiencodedecode/abi_decode_simple_storage.sol` | +0% | | | | `array/array_memory_index_access.sol` | +0% | | | | `array/array_storage_index_access.sol` | +0% | | | | `array/array_storage_index_zeroed_test.sol` | +0% | | | | `array/array_storage_push_empty.sol` | +0% | | | | `array/arrays_complex_from_and_to_storage.sol` | +0% | | | | `array/byte_array_transitional_2.sol` | +0% | | | | `array/bytes_length_member.sol` | +0% | | | | `array/constant_var_as_array_length.sol` | -0% | | | | `array/copying/array_copy_cleanup_uint40.sol` | +0% | | | | `array/copying/array_copy_clear_storage.sol` | +0% | | | | `array/copying/array_copy_different_packing.sol` | +0% | | | | `array/copying/array_copy_including_array.sol` | +0% | | | | `array/copying/array_copy_nested_array.sol` | +0% | | | | `array/copying/array_copy_storage_storage_different_base_nested.sol` | +0% | | | | `array/copying/array_copy_storage_storage_dyn_dyn.sol` | +0% | | | | `array/copying/array_copy_storage_storage_static_static.sol` | +0% | | | | `array/copying/array_copy_storage_storage_struct.sol` | +0% | | | | `array/copying/array_copy_storage_to_memory_nested.sol` | +0% | | | | `array/copying/array_copy_target_leftover.sol` | +0% | | | | `array/copying/array_copy_target_simple.sol` | +0% | | | | `array/copying/array_copy_target_simple_2.sol` | +0% | | | | `array/copying/array_elements_to_mapping.sol` | +0% | | | | `array/copying/array_nested_calldata_to_storage.sol` | +0% | | | | `array/copying/array_nested_memory_to_storage.sol` | +0% | | | | `array/copying/array_of_function_external_storage_to_storage_dynamic.sol` | +0% | | | | `array/copying/array_of_function_external_storage_to_storage_dynamic_different_mutability.sol` | +0% | | | | `array/copying/array_of_struct_memory_to_storage.sol` | +0% | | | | `array/copying/array_of_structs_containing_arrays_calldata_to_storage.sol` | +0% | | | | `array/copying/array_of_structs_containing_arrays_memory_to_storage.sol` | +0% | | | | `array/copying/array_storage_multi_items_per_slot.sol` | -0% | | | | `array/copying/array_to_mapping.sol` | +0% | | | | `array/copying/bytes_inside_mappings.sol` | +0% | | | | `array/copying/bytes_storage_to_storage.sol` | -1% | | | | `array/copying/calldata_array_dynamic_to_storage.sol` | +0% | | | | `array/copying/calldata_array_to_mapping.sol` | +0% | | | | `array/copying/cleanup_during_multi_element_per_slot_copy.sol` | -0% | | | | `array/copying/copy_byte_array_in_struct_to_storage.sol` | +0% | | | | `array/copying/copy_byte_array_to_storage.sol` | +0% | | | | `array/copying/copy_function_internal_storage_array.sol` | +0% | | | | `array/copying/copy_removes_bytes_data.sol` | +0% | | | | `array/copying/copying_bytes_multiassign.sol` | +0% | | | | `array/copying/elements_of_nested_array_of_structs_calldata_to_storage.sol` | +0% | | | | `array/copying/elements_of_nested_array_of_structs_memory_to_storage.sol` | +0% | | | | `array/copying/function_type_array_to_storage.sol` | +0% | | | | `array/copying/memory_dyn_2d_bytes_to_storage.sol` | +0% | | | | `array/copying/nested_array_element_storage_to_storage.sol` | +0% | | | | `array/copying/nested_array_of_structs_calldata_to_storage.sol` | +0% | | | | `array/copying/nested_array_of_structs_memory_to_storage.sol` | +0% | | | | `array/copying/nested_array_of_structs_storage_to_storage.sol` | +0% | | | | `array/copying/nested_dynamic_array_element_calldata_to_storage.sol` | +0% | | | | `array/copying/storage_memory_nested.sol` | +0% | | | | `array/copying/storage_memory_nested_bytes.sol` | +0% | | | | `array/copying/storage_memory_nested_from_pointer.sol` | +0% | | | | `array/copying/storage_memory_nested_struct.sol` | +0% | | | | `array/copying/storage_memory_packed_dyn.sol` | +0% | | | | `array/create_memory_array.sol` | +0% | | | | `array/delete/bytes_delete_element.sol` | +0% | | | | `array/dynamic_array_cleanup.sol` | +0% | | | | `array/dynamic_arrays_in_storage.sol` | +0% | | | | `array/dynamic_multi_array_cleanup.sol` | +0% | | | | `array/fixed_array_cleanup.sol` | +0% | | | | `array/fixed_arrays_as_return_type.sol` | +0% | | | | `array/fixed_arrays_in_constructors.sol` | -1% | | | | `array/function_array_cross_calls.sol` | 1% | | | | `array/invalid_encoding_for_storage_byte_array.sol` | +0% | | | | `array/pop/array_pop_array_transition.sol` | +0% | | | | `array/pop/array_pop_uint16_transition.sol` | +0% | | | | `array/pop/byte_array_pop_long_storage_empty.sol` | -0% | | | | `array/pop/byte_array_pop_long_storage_empty_garbage_ref.sol` | +0% | | | | `array/pop/byte_array_pop_masking_long.sol` | +0% | | | | `array/push/array_push_nested_from_calldata.sol` | +0% | | | | `array/push/array_push_struct.sol` | +0% | | | | `array/push/array_push_struct_from_calldata.sol` | +0% | | | | `array/push/byte_array_push_transition.sol` | +0% | | | | `array/push/nested_bytes_push.sol` | -0% | | | | `array/push/push_no_args_2d.sol` | +0% | | | | `array/push/push_no_args_bytes.sol` | +0% | | | | `array/reusing_memory.sol` | -0% | | | | `byte_array_to_storage_cleanup.sol` | 2% | | | | `calldata/copy_from_calldata_removes_bytes_data.sol` | +0% | | | | `constructor/arrays_in_constructors.sol` | +0% | | | | `constructor/bytes_in_constructors_packer.sol` | 1% | | | | `constructor/bytes_in_constructors_unpacker.sol` | 2% | | | | `constructor/constructor_arguments_external.sol` | -0% | | | | `constructor/constructor_static_array_argument.sol` | -0% | | | | `constructor/no_callvalue_check.sol` | +0% | | | | `constructor_with_params.sol` | -0% | | | | `constructor_with_params_diamond_inheritance.sol` | -0% | | | | `constructor_with_params_inheritance.sol` | -0% | | | | `events/event_dynamic_array_storage.sol` | +0% | | | | `events/event_dynamic_array_storage_v2.sol` | +0% | | | | `events/event_dynamic_nested_array_storage_v2.sol` | +0% | | | | `events/event_emit_from_other_contract.sol` | +0% | | | | `events/event_indexed_string.sol` | +0% | | | | `externalContracts/FixedFeeRegistrar.sol` | -0% | | | | `externalContracts/base64.sol` | 1% | | | | `externalContracts/deposit_contract.sol` | +0% | | | | `externalContracts/prbmath_signed.sol` | +0% | | | | `externalContracts/prbmath_unsigned.sol` | +0% | | | | `externalContracts/ramanujan_pi.sol` | 1% | | | | `externalContracts/snark.sol` | +0% | | | | `externalContracts/strings.sol` | 7% | | | | `functionCall/creation_function_call_with_args.sol` | -1% | | | | `functionCall/creation_function_call_with_salt.sol` | -1% | | | | `functionCall/external_call_to_nonexisting.sol` | -0% | | | | `functionCall/external_call_to_nonexisting_debugstrings.sol` | +0% | | | | `functionCall/failed_create.sol` | 1% | | | | `functionCall/gas_and_value_basic.sol` | -0% | | | | `functionCall/gas_and_value_brace_syntax.sol` | -0% | | | | `immutable/multi_creation.sol` | -0% | | | | `immutable/use_scratch.sol` | -0% | | | | `inheritance/value_for_constructor.sol` | -0% | | | | `isoltestTesting/balance_other_contract.sol` | +0% | | | | `libraries/internal_types_in_library.sol` | +0% | | | | `salted_create/salted_create.sol` | +0% | | | | `salted_create/salted_create_with_value.sol` | +0% | | | | `smoke/constructor.sol` | +0% | | | | `storage/empty_nonempty_empty.sol` | +0% | | | | `structs/calldata/calldata_struct_with_nested_array_to_storage.sol` | +0% | | | | `structs/conversion/recursive_storage_memory.sol` | +0% | | | | `structs/copy_from_mapping.sol` | +0% | | | | `structs/copy_struct_array_from_storage.sol` | +0% | | | | `structs/copy_substructures_from_mapping.sol` | +0% | | | | `structs/copy_substructures_to_mapping.sol` | +0% | | | | `structs/copy_to_mapping.sol` | +0% | | | | `structs/struct_containing_bytes_copy_and_delete.sol` | +0% | | | | `structs/struct_delete_storage_with_array.sol` | +0% | | | | `types/mapping/copy_from_mapping_to_mapping.sol` | +0% | | | | `userDefinedValueType/calldata.sol` | +0% | | | | `userDefinedValueType/erc20.sol` | -0% | | | | `various/address_code.sol` | 4% | | | | `various/create_calldata.sol` | 3% | | | | `various/destructuring_assignment.sol` | +0% | | | | `various/erc20.sol` | -1% | | | | `various/many_subassemblies.sol` | +0% | | | | `various/selfdestruct_pre_cancun.sol` | -1% | | | | `various/selfdestruct_pre_cancun_multiple_beneficiaries.sol` | -1% | | | | `various/selfdestruct_pre_cancun_redeploy.sol` | -1% | | | | `various/senders_balance.sol` | -1% | | | | `various/skip_dynamic_types_for_structs.sol` | +0% | | | | `various/value_complex.sol` | -0% | | | | `various/value_insane.sol` | -0% | | | | `viaYul/copy_struct_invalid_ir_bug.sol` | +0% | | |

ethereum / solidity