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[venom]: stack reorder rework #4220

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

HodanPlodky
Copy link
Contributor

What I did

Fixed the issue that was manifested in the snekmate test. This bug is reported in #4215

How I did it

The bug was caused by error in stack_reorder function. The function depends on the order of each operand but since the order changes there is the possibility to move same stack operand twice. To fix this issue I kept the information where necessary operands on stack.

How to verify it

The example from the issue does compile now.

Commit message

fix[venom]: stack reorder fix

Description for the changelog

Cute Animal Picture

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

Copy link

codecov bot commented Sep 3, 2024

Codecov Report

Attention: Patch coverage is 11.76471% with 15 lines in your changes missing coverage. Please review.

Project coverage is 46.03%. Comparing base (f2975d7) to head (aed28e3).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
vyper/venom/venom_to_assembly.py 11.76% 15 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (f2975d7) and HEAD (aed28e3). Click for more details.

HEAD has 119 uploads less than BASE
Flag BASE (f2975d7) HEAD (aed28e3)
120 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #4220       +/-   ##
===========================================
- Coverage   59.40%   46.03%   -13.37%     
===========================================
  Files         109      109               
  Lines       15635    15651       +16     
  Branches     3443     3448        +5     
===========================================
- Hits         9288     7205     -2083     
- Misses       5680     7892     +2212     
+ Partials      667      554      -113     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@HodanPlodky HodanPlodky marked this pull request as ready for review September 3, 2024 13:20
cost += self.swap(assembly, stack, depth)
if final_stack_depth in positions[stack.peek(final_stack_depth)]:
Copy link
Member

Choose a reason for hiding this comment

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

i think the positions[stack.peek(final_stack_depth)] can be factored out into a variable

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.

2 participants