Optimized PUSH0 and POP in clearStorageLoop

[molly-ting]

Description

contract A {
    uint256[] stateArr;
    function deleteArr() public {
        delete stateArr;
    }
}

The above function will generate the following opcode:

PUSH0 DUP2 PUSH0 SWAP1 SSTORE POP

The first pushed zero is not used before it's popped.

Optimization

https://github.com/ethereum/solidity/blob/b849b327781cb71478709b28c4d0d372492cbdc1/libsolidity/codegen/ArrayUtils.cpp#L951-L954 https://github.com/ethereum/solidity/blob/b849b327781cb71478709b28c4d0d372492cbdc1/libsolidity/codegen/LValue.cpp#L451-L508 The first push0 is generated in line 952 of the function clearStorageLoop and is popped in line 954. It seems that only the branch in line 494 of the function setToZero will use this pushed zero, while the other branches will not. Is it possible to pass the value for this branch or separate this branch into another function so that we do not need to generate push0 and pop? Besides, in this case, the two parameters of the exp instruction in the branch at line 494 are constants 0 and 0x100, and the result is 1. Why not just directly push 1 for this case?

[cameel]

Thank you for the interest in improving the compiler! Unfortunately, we can't really afford to spend time on small optimizations of this kind for the legacy pipeline. Legacy is a dead end and I'd highly recommend focusing on the IR pipeline instead. That's where all the development is happening now.

As to this specific issue, this is a general convention used by the classes that inherit from LValue. setToZero() is a virtual function that expects the whole L-value on the stack. Special-casing it for some kinds of storage items would complicate things because now we would no longer have uniform interface for clearing slots and we'd have to account for special cases everywhere.

Overcomplication of this kind easily leads to bugs and makes the compiler harder to maintain. It just does not scale well. This is why we're not planning to address cases like this one by one at code generation stage. In the IR codegen the approach is different - we generate simple, correct code and then hand it off to the optimizer, which also has much more context to work with. If it's still an issue via IR, we could add it to the list of planned optimizations, though note that currently our focus is more on making the optimizer fast, so it would have to wait for its turn.

[molly-ting]

Thanks for your reply. I also found other cases involving computations with zero. Could you please have a look at these to determine if they are of the same type and will not be optimized in the legacy pipeline? @cameel

1. https://github.com/ethereum/solidity/blob/0f982266663246437cca867ebd3973df99c7512f/libsolidity/codegen/ExpressionCompiler.cpp#L2710-L2723 Although it only exists in the old version, this could generate PUSH1 0x0 PUSH1 MLOAD PUSH1 ADD MSTORE, which could be optimized to PUSH1 MLOAD PUSH1 MSTORE.
2. https://github.com/ethereum/solidity/blob/0f982266663246437cca867ebd3973df99c7512f/libsolidity/codegen/ArrayUtils.cpp#L495-L496 https://github.com/ethereum/solidity/blob/0f982266663246437cca867ebd3973df99c7512f/libsolidity/codegen/LValue.cpp#L236-L244 This could generate PUSH1 0x0 SWAP1 SLOAD SWAP1 PUSH2 0x100 EXP SWAP1 DIV, which is equivalent to SLOAD.
3. https://github.com/ethereum/solidity/blob/0f982266663246437cca867ebd3973df99c7512f/libsolidity/codegen/ArrayUtils.cpp#L239-L240 https://github.com/ethereum/solidity/blob/0f982266663246437cca867ebd3973df99c7512f/libsolidity/codegen/LValue.cpp#L308-L320 This could generate PUSH0 PUSH2 0x100 EXP DUP2 SLOAD DUP2 PUSH20 MUL NOT AND which is equivalent to DUP1 SLOAD PUSH32 AND. The value of PUSH32 is the rsult of !((u256(1) << (8 * m_dataType->storageBytes())) - 1).
4. https://github.com/ethereum/solidity/blob/0f982266663246437cca867ebd3973df99c7512f/libsolidity/codegen/CompilerUtils.cpp#L651-L661 https://github.com/ethereum/solidity/blob/0f982266663246437cca867ebd3973df99c7512f/libsolidity/codegen/LValue.cpp#L141-L148 https://github.com/ethereum/solidity/blob/0f982266663246437cca867ebd3973df99c7512f/libsolidity/codegen/CompilerUtils.cpp#L201-L244 https://github.com/ethereum/solidity/blob/0f982266663246437cca867ebd3973df99c7512f/libsolidity/codegen/CompilerUtils.cpp#L731-L741 The function pushZeroValue could push zeros. In storeInMemoryDynamic, there could be constant mathematical computations with the pushed zeros, such as line 734 and line 738 in combineExternalFunctionType.