Skip to content

Add bit shifting opcodes (EIP145)#2541

Merged
axic merged 6 commits intodevelopfrom
asm-bitshift
Feb 27, 2018
Merged

Add bit shifting opcodes (EIP145)#2541
axic merged 6 commits intodevelopfrom
asm-bitshift

Conversation

@axic
Copy link
Copy Markdown
Contributor

@axic axic commented Jul 8, 2017

@axic axic force-pushed the asm-bitshift branch 2 times, most recently from 7bb0dd5 to 4da634f Compare July 11, 2017 00:40
@axic axic mentioned this pull request Sep 25, 2017
@chriseth
Copy link
Copy Markdown
Contributor

Perhaps it makes sense to introduce a meta programming language at least for plain assembly / julia files that can specify new functions, what their arguments and return values are and which EVM opcode they map to.

@axic
Copy link
Copy Markdown
Contributor Author

axic commented Jan 5, 2018

This needs tests, but that can only be added when ethereum/aleth#4054 is finished.

@axic
Copy link
Copy Markdown
Contributor Author

axic commented Feb 9, 2018

I think we should merge this (apart from the SimplificationRules) to aid Constantinople testing.

@pirapira @chriseth any opposition?

@pirapira
Copy link
Copy Markdown
Contributor

pirapira commented Feb 9, 2018

@axic well, @winsvega says he is willing to use a branch of this repo. So, a merge is not necessary.

@axic
Copy link
Copy Markdown
Contributor Author

axic commented Feb 14, 2018

Should use the new evmTarget from #1117.

@axic axic force-pushed the asm-bitshift branch 2 times, most recently from d78fc1f to 6d21714 Compare February 23, 2018 08:34
@axic axic changed the title [WIP] Add bit shifting opcodes (EIP145) Add bit shifting opcodes (EIP145) Feb 23, 2018
@axic
Copy link
Copy Markdown
Contributor Author

axic commented Feb 23, 2018

@chriseth I'd like to merge this. When we introduce constantinople in evm-version, the warning can be replaced based on that.

Pulled out the simplification rule.

+-------------------------+------+-----------------------------------------------------------------+
| byte(n, x) | | nth byte of x, where the most significant byte is the 0th byte |
+-------------------------+------+-----------------------------------------------------------------+
| shl(x, y) | | logical shift left y by x |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should add comments that tell the user which EVM version is required.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you mean just a commit in the description or a new column at some point?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Either a new column or just in parentheses.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps a small column with H/B/C

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thinking about reusing the middle column. It currently is used for empty (pushes one item), - (doesn't push anything onto the stack), * (special). It could just have B for byzantium ones?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will add the C after #3604 is merged.

_location,
"The \"" +
boost::to_lower_copy(instructionInfo(_instr).name)
+ "\" instruction is experimental and not available in regular clients. "
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blockchains instead of clients? Ethereum implementations?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well since it was designated for constantinople we could just update the text to reflect that.

@axic
Copy link
Copy Markdown
Contributor Author

axic commented Feb 26, 2018

Need to update the analyser code to output warning dependent on EMV version (#3569).

@axic
Copy link
Copy Markdown
Contributor Author

axic commented Feb 27, 2018

Updated the documentation and the warning message.

+-------------------------+-----+---+-----------------------------------------------------------------+
| byte(n, x) | | F | nth byte of x, where the most significant byte is the 0th byte |
+-------------------------+-----+---+-----------------------------------------------------------------+
| shl(x, y) | | C | logical shift left y by x |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

by x bits?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@axic axic mentioned this pull request Feb 27, 2018
1 task
@axic axic merged commit 2abc5be into develop Feb 27, 2018
@axic axic deleted the asm-bitshift branch February 27, 2018 13:47
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