solidity
solidity copied to clipboard
[Yul] Re-introduce ``sub`` opcode
During CSE, every sub by a constant is replaced by an add to get uniform expressions. At the end of the optimization phase, after constant-reintroduction, this should be reversed if the cost of the modified constant is better.
This is superceded by the constant optimiser, isn't it?
No, the constant optimizer only operates on actual numbers. Expressions like sub(codesize(), 10) will not be improved by the constant optimizer.
Could this be more general optimization like "add(X, A) -> sub(A, -X) in case the cost is better", so not only for expressions that start like sub?
Or would this somehow interfere with the part where we're trying to get rid of all subs for uniformness?
Yeah, that's how I'd do it. I don't see much point in trying to limit it to just the ones that have been replaced for CSE. This would actually be unnecessarily complicated because we'd have to track them. As long as the step is not executed before the CSE, I think it should not destroy any important property.
Just looking at #13535: I would say adding add(X, A) to sub(A, -X) is not the right way. One thing we want to avoid in the expression simplifier is infinite loops, as it doesn't really backtrace or understand the history of replacements. So if there is a loop in the simplification rule, hitting it is wasting time.
I believe Chris is referring to something more specific, which is the constant optimizer. Some expression have shorter bytecode size than others, which is useful when you are optimizing for bytecode. For example, add(x, 115792089237316195423570985008687907853269984665640564039457584007913129639935) would have bigger bytecode size than sub(x, 1).
IMO, this should be done as a separate step or closely related to constant optimizer.
Was talking with @0age about improving codesize in Seaport. Turns out, this optimization would really improve contracts hitting size limits.
Here is some data for context, mostly related to the dispatcher comparing the calldatasize() with another value.
This issue has been marked as stale due to inactivity for the last 90 days. It will be automatically closed in 7 days.
Hi everyone! This issue has been automatically closed due to inactivity. If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen. However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.