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

fix: initcode codesize regression #3450

Merged
merged 7 commits into from
May 23, 2023

Conversation

charles-cooper
Copy link
Member

@charles-cooper charles-cooper commented May 23, 2023

What I did

How I did it

How to verify it

Commit message

this commit fixes a regression in c202c4e3ec8. the commit message states
that we rely on the dead code eliminator to prune unused internal
functions in the initcode, but the dead code eliminator does not prune
dead code in all cases, including nested internal functions and loops.
this commit reintroduces the reachability analysis in
`vyper/codegen/module.py` as a stopgap until the dead code eliminator is
more robust.

Description for the changelog

Cute Animal Picture

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

this commit fixes a regression in c202c4e. the commit notes state
that we rely on the dead code eliminator to prune unused internal
functions in the initcode, and said commit tests the dead code
eliminator, but some real world contracts in fact experience code
blowup.
@codecov-commenter
Copy link

codecov-commenter commented May 23, 2023

Codecov Report

Merging #3450 (9881109) into master (036f153) will increase coverage by 0.00%.
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    #3450   +/-   ##
=======================================
  Coverage   89.30%   89.30%           
=======================================
  Files          84       84           
  Lines       10787    10794    +7     
  Branches     2460     2462    +2     
=======================================
+ Hits         9633     9640    +7     
  Misses        756      756           
  Partials      398      398           
Impacted Files Coverage Δ
vyper/codegen/module.py 97.61% <100.00%> (+0.08%) ⬆️
vyper/ir/compile_ir.py 93.76% <100.00%> (+0.04%) ⬆️

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

@charles-cooper charles-cooper requested a review from fubuloubu May 23, 2023 17:36
the new optimizer rules happily optimize out the custom blueprint code
for test_bad_code_offset, so manually deploy unoptimized bytecode for this
case
while i < len(assembly) - 2:
instr = assembly[i]
if isinstance(instr, list):
instr = assembly[i][-1]

Check warning

Code scanning / CodeQL

Variable defined multiple times

This assignment to 'instr' is unnecessary as it is [redefined](1) before this value is used.
@charles-cooper charles-cooper merged commit 510125e into vyperlang:master May 23, 2023
@charles-cooper charles-cooper deleted the fix/initcode_blowup branch May 23, 2023 21:53
@charles-cooper charles-cooper changed the title fix: initcode blowup fix: initcode codesize regression May 23, 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