-
Notifications
You must be signed in to change notification settings - Fork 314
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
Change values in instruction metrics #435
Conversation
98028d7
to
2162feb
Compare
include/evmc/instructions.h
Outdated
@@ -188,11 +188,11 @@ struct evmc_instruction_metrics | |||
/** The instruction gas cost. */ | |||
int16_t gas_cost; | |||
|
|||
/** The number of items the instruction pops from the EVM stack before execution. */ | |||
int8_t num_stack_arguments; | |||
/** The number of the EVM stack items required for the instruction. */ |
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.
"the minimum number" maybe?
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.
Changed.
include/evmc/instructions.h
Outdated
|
||
/** The number of items the instruction pushes to the EVM stack after execution. */ | ||
int8_t num_stack_returned_items; | ||
/** The EVM stack height change caused by the instruction execution. */ |
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'd clarify that it's stack after execution - stack before
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.
Added.
The values in the table are going to be changed in the following commit.
2162feb
to
2c3eb42
Compare
This updates instruction metrics with changed values - they are easier to explain and represent number directly usable by VM implementations (will benefit aleth-interepreter).
This is tested against evmone's table: ethereum/evmone#191.
The modification can be easily verified by following the rule:
Continuation of #356.