Skip to content
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

feat: use new push0 opcode #3361

Merged
merged 9 commits into from
May 18, 2023
Merged

Conversation

charles-cooper
Copy link
Member

@charles-cooper charles-cooper commented Apr 14, 2023

per shanghai fork

What I did

implement #3360

note: this looks like it shaves about 1-2% off bytecode for https://github.com/yearn/yearn-vaults-v3/blob/00f0778aa1202cc217174efb6fd50a65c75a3c76/contracts/VaultV3.vy and https://github.com/curvefi/curve-stablecoin/blob/a29fc2e1c395793d4d3966e10cf2431fa179bfe6/contracts/Stableswap.vy.

How I did it

How to verify it

Commit message

per the shanghai fork - during codegen, use the new `push0` opcode
instead of the `push1 0` sequence

Description for the changelog

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

vyper/ir/compile_ir.py Outdated Show resolved Hide resolved
vyper/ir/compile_ir.py Outdated Show resolved Hide resolved
vyper/ir/compile_ir.py Outdated Show resolved Hide resolved
@@ -555,13 +556,10 @@ def _height_of(witharg):
o = _compile_to_assembly(code.args[0], withargs, existing_labels, break_dest, height)
o.extend(
[
"PUSH1",
MemoryPositions.FREE_VAR_SPACE,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this only 0 or 1?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right now FREE_VAR_SPACE is 0, but since it could change, use the PUSH abstraction

@@ -23,7 +23,8 @@ def num_to_bytearray(x):

def PUSH(x):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonder if this might be better named push_stack or something clearer, I thought it was an opcode at first

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be -- it is supposed to be an abstraction of the PUSH opcodes

@charles-cooper
Copy link
Member Author

this is waiting on upstream -- py-evm 0.7.0a1 was released with shanghai updates including PUSH0, but a new eth-tester has not been released which results in a dependency conflict when trying to install the new py-evm.

@charles-cooper charles-cooper marked this pull request as ready for review May 18, 2023 12:55
@charles-cooper
Copy link
Member Author

looks like eth-tester updated on may 12, see https://pypi.org/project/eth-tester/#history

@charles-cooper charles-cooper enabled auto-merge (squash) May 18, 2023 13:31
@codecov-commenter
Copy link

codecov-commenter commented May 18, 2023

Codecov Report

Merging #3361 (0891892) into master (4f9f813) will increase coverage by 0.08%.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##           master    #3361      +/-   ##
==========================================
+ Coverage   89.09%   89.17%   +0.08%     
==========================================
  Files          84       84              
  Lines       10763    10763              
  Branches     2452     2452              
==========================================
+ Hits         9589     9598       +9     
+ Misses        781      771      -10     
- Partials      393      394       +1     
Impacted Files Coverage Δ
vyper/compiler/output.py 91.66% <100.00%> (ø)
vyper/evm/opcodes.py 100.00% <100.00%> (ø)
vyper/ir/compile_ir.py 93.71% <100.00%> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@charles-cooper charles-cooper merged commit 1ac8362 into vyperlang:master May 18, 2023
@pcaversaccio pcaversaccio mentioned this pull request May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants