-
-
Notifications
You must be signed in to change notification settings - Fork 793
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
Reduce code size by sharing code for clamps #2387
Conversation
All clamps have the following code (assume condition is on the stack) ISZERO _revert JUMPI # if not condition goto _revert PUSH1 0 DUP REVERT _revert JUMPDEST Since clamps are super common this code gets repeated. This reduces code size by adding the PUSH1 0 DUP REVERT to each program's postamble, and then clamps are simplified to the following (assume condition is on the stack) _revert JUMPI # if not condition goto _revert -- cool now 5 opcodes are skipped This commit also optimizes the following common "truthy" sequences: ISZERO ISZERO ISZERO -> ISZERO ISZERI ISZERO _jumpdest JUMPI -> _jumpdest JUMPI
2ebdca1
to
1f374d4
Compare
Codecov Report
@@ Coverage Diff @@
## master #2387 +/- ##
==========================================
+ Coverage 85.74% 85.84% +0.09%
==========================================
Files 91 91
Lines 8995 9021 +26
Branches 2145 2152 +7
==========================================
+ Hits 7713 7744 +31
+ Misses 788 785 -3
+ Partials 494 492 -2
Continue to review full report at Codecov.
|
Tried testing this by compiling a Curve contract and running the related tests, it's failing in Brownie during the compilation phase because of an error in the source map. |
Could this change instead be applied during LLL optimizations in |
@iamdefinitelyahuman it would require adding a conditional jump macro to LLL. Otherwise, we will have sequences like the following (if condition (goto fail)) Which would generate an extra 3 instruction JUMP block like this condition
PUSH _escape
JUMPI
PUSH _fail_block
JUMP
_escape
JUMPDEST instead of not_condition
PUSH _fail_block
JUMPI But the way I have it also causes optimization problems being generated so late, for instance the jumpdest label can't be optimized into a dup. |
@iamdefinitelyahuman source map error should be fixed in 82c5641 (I still have reservations about the approach of doing this in the opcode gen instead of as an LLL optimization step) EDIT: After sleeping on it I decided I'm OK with doing it in the opcode gen since I plan to rewrite all of LLL anyway. |
What I did
See title
How I did it
All clamps have the following code:
Since clamps are super common this code gets repeated. This reduces code
size by adding the PUSH1 0 DUP REVERT to each program's postamble, and
then clamps are simplified to the following
Then at the end of the contract, the following "postamble" is inserted:
This commit also optimizes the following common "truthy" sequences:
How to verify it
Tests pass. I compared with master (ba0676b) and compiling a couple large contracts resulted in nearly 10% codesize reduction:
Description for the changelog
Reduce code size by ~10% by sharing clamp code
Cute Animal Picture