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[venom]: add effects to instructions #4264

Merged

Conversation

charles-cooper
Copy link
Member

@charles-cooper charles-cooper commented Sep 27, 2024

What I did

add effects to instructions, which is helpful for several different ongoing efforts

How I did it

How to verify it

Commit message

this commit adds an `effects.py` file to venom, which describes
the effects of opcodes. this is useful for several ongoing efforts,
including CSE elimination and DFT pass improvements.

Description for the changelog

Cute Animal Picture

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

@charles-cooper charles-cooper marked this pull request as ready for review September 28, 2024 15:01
vyper/venom/basicblock.py Outdated Show resolved Hide resolved
Co-authored-by: Harry Kalogirou <harkal@nlogn.eu>
vyper/venom/effects.py Outdated Show resolved Hide resolved
charles-cooper and others added 2 commits September 30, 2024 07:11
Co-authored-by: HodanPlodky <36966616+HodanPlodky@users.noreply.github.com>
"selfbalance": BALANCE,
"log": MEMORY,
"revert": MEMORY,
"return": MEMORY,
Copy link
Collaborator

Choose a reason for hiding this comment

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

return has an offset to memory, right? so in theory you might be forced to zero-extend the memory

Copy link
Collaborator

Choose a reason for hiding this comment

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

applies to other opcodes as well

Copy link
Member Author

Choose a reason for hiding this comment

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

do we really care about zero extension? i think gas is an effect we don't need to preserve

Copy link
Collaborator

Choose a reason for hiding this comment

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

what about msize?

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm that's true. but return can't be reordered past other instructions anyways

Copy link
Collaborator

Choose a reason for hiding this comment

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

you have the same for eg mload

from enum import Flag, auto


class Effects(Flag):
Copy link
Collaborator

Choose a reason for hiding this comment

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

modeling code (e.g., for code copy) isn't useful?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not that useful, since it can't change during execution

Copy link
Member Author

@charles-cooper charles-cooper Oct 1, 2024

Choose a reason for hiding this comment

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

(At least that I know of)

"mcopy": MEMORY,
}

reads = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why aren't create opcodes modeled here?

RETURNDATA = Effects.RETURNDATA


writes = {
Copy link
Collaborator

@cyberthirst cyberthirst Oct 1, 2024

Choose a reason for hiding this comment

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

selfdestruct (atleast balance)?

@cyberthirst cyberthirst self-assigned this Oct 3, 2024
@HodanPlodky
Copy link
Collaborator

Have you considered the effect of logs on other logs ie. in CSE you cannot replace the log by another log or what would happened if you switch log order. Not sure if it should be modeled in this PR

@charles-cooper
Copy link
Member Author

Have you considered the effect of logs on other logs ie. in CSE you cannot replace the log by another log or what would happened if you switch log order. Not sure if it should be modeled in this PR

That's a good one! We definitely should not reorder logs.

@charles-cooper charles-cooper enabled auto-merge (squash) October 5, 2024 00:26
@charles-cooper charles-cooper merged commit 02f8654 into vyperlang:master Oct 5, 2024
156 checks passed
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.

4 participants