-
Notifications
You must be signed in to change notification settings - Fork 159
Introducing new MInt
hooks to BYTES, LIST, and MINT modules
#4837
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
base: develop
Are you sure you want to change the base?
Conversation
8fb5336
to
c92a760
Compare
06a61b0
to
74fae1a
Compare
MInt
hooksMInt
hooks to BYTES, LIST, and MINT modules
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a couple of comments but otherwise looks good to me. There are some CI checks failing though so I won't approve until they pass.
@@ -2092,15 +2097,17 @@ of the `Bytes` term). | |||
|
|||
```k | |||
syntax Bytes ::= Bytes "[" index: Int "<-" value: Int "]" [function, hook(BYTES.update)] | |||
syntax {Width} Bytes ::= Bytes "[" index: MInt{Width} "<-" value: MInt{Width} "]" [function, hook(BYTES.updateMInt)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you did not mention the limitation of this working only for 64 and 256 bitwidths. Is it not limited to those?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, only 64-bit. I've updated the List module to contain the limitations for all new hooks.
@@ -1027,6 +1030,7 @@ You can get the number of elements of a list in O(1) time. | |||
|
|||
```k | |||
syntax Int ::= size(List) [function, total, hook(LIST.size), symbol(sizeList), smtlib(smt_seq_len)] | |||
syntax {Width} MInt{Width} ::= size(List) [function, hook(LIST.sizeMInt), symbol(sizeMInt)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this is a partial function, not defined for sizes greater than 2^width.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a comment about this!
c85fd15
to
225feb9
Compare
9872fab
to
19dd5b4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
This PR intends to add these new hooks described below to make it possible to translate our KEVM's
wordStack
to use MInt instead of Int to get a better performance from it.The new hooks proposed by this PR are:
Most of these hooks have an initial implementation for 64 and 256-bit integers, the necessary width for KEVM, but it's simple to add other bit widths as needed in the future!