Yul optimizer: Remove Yul constant optimizer

[blishko]

This PR proposes to completely remove Yul constant optimizer.
Currently, there are two places in the compiler that attempt to optimize constants by replacing them by expressions that compute them. This is ConstantOptimizer in libyul/backends/evm, the other or ConstantOptimizer in libevmasm. Here "optimize" means optimizing for bytecode size. Constants can always be obtained by PUSH* instruction followed by the value, but the constant can be computed with a few instructions that together take less bytes.

I believe that trying to solve the same problem in two different places can lead to unnecessary complexity.
As an example, a proposed optimization, such as #16729, would have to be implemented on both places.
Conceptually, I believe this optimization belongs to the evmasm optimizer.
Especially in the context of SSA-CFG, Yul should preserve the information about literals and let further stages down the codegen pipeline deal with this EVM-specific optimization.

[blishko]

Here is the diff from c_ext_benchmarks.
There are large regressions on ir-no-optimize, but this is expected, since in this version, the large constants are preserved and appear in the bytecode as pushes.
The important table is ir-optimize-evm+yul which shows improvement on average, with some small regressions (like euler).

ir-no-optimize

project	bytecode_size	deployment_gas	method_gas
brink	+5.78% ❌
colony	0%
elementfi	+3.87% ❌
ens	+5.65% ❌
euler
gnosis
gp2	+5.12% ❌
pool-together	+5.57% ❌
uniswap	+4% ❌	+3.57% ❌	+2.5% ❌
yield_liquidator	+5.24% ❌	+4.99% ❌	-0.46% ✅
zeppelin

ir-optimize-evm+yul

project	bytecode_size	deployment_gas	method_gas
brink	+0.07% ❌
colony	+0%
elementfi	+0.13% ❌
ens	-0.81% ✅	-1.77% ✅	+0.02% ❌
euler	+0.57% ❌	+0.66% ❌	+0.13% ❌
gnosis
gp2	-0.08% ✅
pool-together	-0.25% ✅
uniswap	-0.38% ✅	-0.42% ✅	-0.22% ✅
yield_liquidator	-0.99% ✅	-0.99% ✅	+0.06% ❌
zeppelin	-0.26% ✅	-0.07% ✅	-0.22% ✅

ir-optimize-evm-only

project	bytecode_size	deployment_gas	method_gas
brink	+0.06% ❌
colony	0%
elementfi	+0.11% ❌
ens	+0.18% ❌	+0.07% ❌	+0%
euler
gnosis
gp2	+0.12% ❌
pool-together	+0.14% ❌
uniswap	+0.04% ❌	+0.04% ❌	+0.02% ❌
yield_liquidator	+0.07% ❌	+0.04% ❌	0%
zeppelin	+0.17% ❌

legacy-no-optimize

project	bytecode_size	deployment_gas	method_gas
brink	0%
colony	0%
elementfi	0%
ens	0%
euler	0%	+0%	+0%
gnosis	0%
gp2	0%
pool-together	0%
uniswap	0%	-0%	+0%
yield_liquidator	0%	-0%	0%
zeppelin

legacy-optimize-evm+yul

project	bytecode_size	deployment_gas	method_gas
brink	0%
colony	+0%
elementfi	+0.01% ❌
ens	+0.03% ❌	+0.04% ❌	-0%
euler	+0.04% ❌	+0.04% ❌	-0%
gnosis	+0.06% ❌
gp2	+0.11% ❌
pool-together	+0.01% ❌
uniswap	+0.02% ❌	+0.02% ❌	+0%
yield_liquidator	+0.05% ❌	+0.04% ❌	+0%
zeppelin	+0%	+0.01% ❌	-0.03% ✅

legacy-optimize-evm-only

project	bytecode_size	deployment_gas	method_gas
brink	0%
colony	0%
elementfi	0%
ens	0%	+0%	0%
euler	0%	-0%	-0%
gnosis	0%
gp2	0%
pool-together	0%
uniswap	0%	+0%	-0%
yield_liquidator	0%	0%	0%
zeppelin

!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

[blishko]

The results on externalContracts in the semantic tests are mixed.
Legacy codegen shows regressions (which I don't fully understand).
SSA-CFG codegen shows mostly improvements and via-ir is mixed.

[moh-eulith]

Alternatively, maybe the code could be refactored to have a single implementation used in both places? That would avoid the regression and allay the concerns about implementation duplication.

[cameel]

For the record, I've been discussing this with @blishko on the #solidty-dev channel, but I think this is a better place for such a discussion.

Here's what was said so that it does not just disappear in the depths of ephemeral chats:

@blishko

I won't be able to join the call tomorrow, so I want to report my findings here.
I started looking at #16729.
Based on a quick check, I am in favour of this change. I will want to inspect it more closely, but it looks simple and it improves bytecode size substantially.

While looking at that PR I started wondering why the compiler is trying to optimize constants both at the Yul level and at the evmasm level. I think it should not be done at the Yul level at all, so I trying ripping the whole thing out and it seems to be actually OK: #16738
I think we should just remove the Yul constant optimizer completely.

@cameel

I've also been wondering about the overlap between these two constant optimizers. This is partially why I was keeping old contribution we had there on hold - I wanted to first properly understand what's going on there and in which of these new enhancements should be added.

Though I expected it would be the other way. I.e. that the Yul-level constant optimizer would be the more future-proof one. Why do you think it's the one we should remove? I know it has less features now, but there must have some advantage over the evmasm one once it's complete, otherwise why would anyone even write it?

@blishko

From my perspective, working on SSA-CFG, I want the constants to be preserved as long as possible.
To me this constant deconstruction should be the very last thing that happens before the final bytecode is generated, so it naturally belongs to the asm level.
Do you see a good argument why it should be happening at the Yul level?

@cameel

For now I can think of two:

• AFAIK the original intention was for the evmasm optimizer to be dropped eventually so the original constant optimizer would be gone with it.
• Auditability of optimizations. In Yul you can use the data objects for the code-copy method and that's much clearer to understand. In fact, I suspect this is why the data objects were introduced, because currently they're not used by the codegen at all (except for .auxdata but that one's special).

TBH these reasons are weakened by the introduction SSA CFG. Before that Yul was meant to be the main level where the optimization was supposed to happen. And the fact that the use of data objects in the Yul optimizer was not (yet?) implemented makes it seem even more useless. So perhaps it's fine to reassess that.

Though the main thing that does not sit right with me is that this does no add up and I just feel that there must have been something more to it. If a component feels useless my first reaction is that we need to dig deeper into it and understand why rather than just remove it :)

@blishko

Regarding the first reason:
I am perfectly willing to believe that was the original intent and it might have made sense at some point. However, the situation has changed, and I think we should adapt to the current reality. In my opinion, the introduction of SSA CFG not only weakens this reason, but refutes it.
Based on my current experience:

• I think we should be preserving the constants going into SSA-CFG representation. We will need constant propagation at SSA level anyway (and soon) and that's just going to undo the work of Yul constant optimizer.
• We will still need some post-processing at the asm level even after SSA-CFG. I already have an example of a concrete optimization that cannot be done on SSA-CFG (or Yul) level, but can be done on asm level. So I think there really is a place for evmasm optimizer (though not necessarily in the current form). And this constant optimization belongs to the asm level.

For the second reason, I am unfortunately not familiar with data objects, so I can't really comment on this at the moment.

Though the main thing that does not sit right with me is that this does no add up and I just feel that there must have been something more to it. If a component feels useless my first reaction is that we need to dig deeper into it and understand why rather than just remove it :)

There was likely an intent behind it, and it is likely the reason you stated. I just think the situation has changed, and the right approach is to perform this constant optimization on evmasm level. I was curious what would happen if I just disable it and the result seems to be not much if evmasm optimizer still runs in the end. My intuition is that the two optimizers are performing very similar transformations (possibly with some differences) that's why disabling Yul constant optimizer has no great effect if the other one still runs.

@cameel

data objects are just a way to represent data stored in the binary at Yul level:

object "A" {
    code {
        datacopy(0, dataoffset("const"), datasize("const"))
        let const := mload(0)
    }
    
    data "const" "deadbeef"
}

There is a section for them in each subassembly.

My intuition is that the two optimizers are performing very similar transformations (possibly with some differences) that's why disabling Yul constant optimizer has no great effect if the other one still runs.

Yes, there are significant differences. I actually investigated it back when I was reviewing that old optimization PR. For example:

• no code copy method
• always tries both negation and gap detection, not either-or like the evmasm one
• different (non-recursive) check for 2-byte and shorter sequences when decomposing big numbers
• different gas estimation (e.g. does not take into account how many times a constant is used in the assembly)
• likely miscounts the unlimited literals as actual strings

I have an old note where I gathered this info. It's unfinished and I was waiting to publish it when I'm done with analyzing it but since the topic got relevant again, I cleaned it up now so that you can check it in the current state: Differences between Yul and evmasm ConstantOptimizer implementations.

These differences are why I'm hesitant to just delete it even if there are no significant cost differences. It's not clear to me which differences are intentional and which are just sloppiness. The Yul implementation is newer so I'd lean towards the former - these changes should be investigated and ported to the evmasm version if they make sense.

Overall, I'd be fine with merging the two, I just don't like the idea of analyzing the situation only partially and executing it in small steps with incomplete information, because that leads to going back and forth and increases our review load. I'd rather have someone look critically at the whole component once, figure out what it should be doing and where it should be in the pipeline. Then, once we have a plan, we can go straight to the solution without meandering.

[moh-eulith]

I haven't looked at the yul optimization steps in detail, but doesn't yul make inlining choices based on size?
PUSH32 0xffff...fffff is a lot more bytes than PUSH0 NOT

[cameel]

That's an interesting point. Technically, it does:

solidity/libyul/optimiser/FullInliner.cpp

Lines 201 to 204 in 2ca6163

	// Inline really, really tiny functions
	size_t size = m_functionSizes.at(calledFunction->name);
	if (size <= 1)
	return true;

The thing is though how the size is defined. It's based on our CodeCost metric, which (intentionally) does not really reflect bytecode size exactly. For example every literal gets a cost of 1:

solidity/libyul/optimiser/Metrics.h

Line 71 in 2ca6163

size_t literalCost = 1;

This probably makes sense, since ConstantOptimiser is a hard-coded step that only runs after the cleanup sequence, so while we're going through the sequence the literal is not yet in its final form. Any assumption the inliner might make about it would likely be wrong. The metric is rather meant to reflect some more abstract measure of size, in terms of the number of Yul statements.

[blishko]

@cameel, thanks for pointing out your document. I will try to do a deep dive into the differences (though it will have to wait a few weeks). I agree that, for example, always analyzing both positive and negative form of the literal should be beneficial.
My opinion is still that all this should be happening on the evmasm level.