Bitwise shifting instructions in EVM #215

axic · 2017-02-13T16:05:54Z

Introduce bitwise shifting.

(Replaces the old issue #145.)

chriseth · 2017-02-24T11:05:51Z

EIPS/eip-draft_bitwise_shifts.md

+The `ROL` instruction (rotate left) pops 2 values from the stack, `arg1` and `arg2`, and pushes on the stack the second popped value `arg2` circular shifted to the left by the number of bits in the first popped value `arg1`.
+
+```
+(arg1 shl arg2) or (arg1 shr (256 - arg2)


The most interesting use-cases for rolling are hash functions and other cryptographic primitives. In most cases, those require rolling inside 32 or 64 bits but seldomly inside 256 bits. Would it make sense to give another argument that determines the "width" of the roll? As the shift amount only takes a single byte, we could even encode both values inside the second argument, although that would make it less easy to use for variable roll amounts.

@chriseth A variable-width rotate instruction is interesting, though it might be better to wait for narrower data types.

@axic Are rotates really 4 times more expensive than a shift? Probably so. Most bigint libraries I've looked at, including Go, don't support them. So they will get implemented as you define them, e.g. (arg1 shl arg2) or (arg1 shr (256 - arg2). That also makes it tempting to leave them out, as the user can implement them that way for about the same performance and gas price. It's only for smaller data types where chips have native rotates that we really would need ROR and ROL.

I tend to think that leaving it out is better since it is not supported by many underlying libraries, hence it will have to be implemented by hand in the VM. And when doing so we shouldn't give a subsidised gas cost - hence my suggested cost.

As discussed offline, it makes no sense to include it in this form as 256-bit rotations are quite useless (= not used in any mainstream crypto algorithm).

Instead of having rotate which takes 3 parameters, the specific 32 or 64 bit rotates can be implemented with shifts in the code in question.

With the proposed SIMD operators rotate could be included.

Arachnid · 2017-04-14T11:55:43Z

Please renumber this as eip-215 and we will merge as a draft.

axic · 2017-04-22T23:16:23Z

I think it should be 145 based on the original issue id.

chfast · 2017-04-23T14:12:34Z

EIPS/eip-145.md

+```
+
+Notes:
+- `arg2` is interpreted as unsigned number.


This is the case for all shifts, so let's move it somewhere up into any generic description.

chfast

Looks good.

Arachnid · 2017-04-24T16:07:29Z

Are you happy for me to merge this?

axic · 2017-04-26T14:46:20Z

@Arachnid seems like we have to clarify one part of SAR today.

chfast · 2017-04-26T16:27:04Z

Can we define SAR Python-style like floor(arg1 / 2**arg2). Otherwise it will difficult to specify it using existing opcodes.

axic · 2017-07-03T20:22:16Z

Otherwise it will difficult to specify it using existing opcodes.

I think it wasn't specified in existing opcodes,udiv is not present. Though I agree we should be more specific in defining them.

axic · 2017-07-03T20:37:06Z

@chfast updated.

@Arachnid I think this should be ready now.

gcolvin · 2017-07-03T22:09:26Z

Looks good to me. Still wish we had rotate, but it can wait.

holiman · 2017-07-28T15:13:11Z

LGTM!

gcolvin · 2017-07-28T15:54:14Z

But where did the rotates go?

axic · 2017-07-28T15:56:28Z

256-bit rotates are pretty much useless (at least today). Your SIMD proposal supports 32/64bit rotates, that is way more useful.

gcolvin · 2017-07-28T20:41:29Z

Still would like them just for operational completeness, but I get what you are saying.

axic · 2017-07-31T12:58:59Z

@chfast added the comparison

axic · 2017-08-04T14:36:56Z

@chfast the operand order follows that of the other arithmetic opcodes, however I think we should reverse them, because I have a hunch it is a more natural usecase to shift a number already on the stack by a constant, than the other way around.

The current version results in a lot of DUP/SWAP cases, just like it is with DIV/SDIV.

chfast · 2017-08-04T14:41:29Z

I would go for it. This would also remove SWAP1 from my snippets.

gcolvin · 2017-08-05T05:57:49Z

Yes, the operand order for DIV and SDIV are broken.

chfast · 2017-08-11T15:08:55Z

@axic As there are no more comments about this, can you revert the operand order?
I can also do this by creating a PR to this PR :D

axic · 2017-08-11T18:11:30Z

@chfast I can do it, but if I won't by Tuesday can you push a PR? 😉

Also need to get tests done as a next step.

axic · 2017-09-22T13:16:19Z

@chfast updated, please review

chfast · 2017-09-22T13:38:16Z

EIPS/eip-145.md

@@ -27,51 +27,51 @@ The following instructions are introduced:

 ### `0x1b`: `SHL` (shift left)

-The `SHL` instruction (shift left) pops 2 values from the stack, `arg1` and `arg2`, and pushes on the stack the first popped value `arg1` shifted to the left by the number of bits in the second popped value `arg2`. The result is equal to
+The `SHL` instruction (shift left) pops 2 values from the stack, `arg1` and `arg2`, and pushes on the stack the second popped value `arg2` shifted to the left by the number of bits in the first popped value `arg1`. The result is equal to


I would skip "second popped" / "first popped". If the order is not clear, it should be clarified in the first part of the sentence.

How about

... pushes on the stack the result of the arg2 shifted to the left by the number of bits in arg1.

Actually:

shifted to the left by arg1 number of bits

wouldn't be more readable?

chfast

axic · 2017-09-22T15:20:42Z

@Souptacular @Arachnid @gcolvin this should be ready to merge ✨

gcolvin

Looks good, merge when you are ready, Alex.

axic · 2017-09-22T15:45:20Z

@gcolvin it has to be someone with merging rights :)

axic force-pushed the bitwise-ops branch from d7919f0 to 17038a7 Compare February 13, 2017 16:07

axic mentioned this pull request Feb 13, 2017

EVM opcodes: bitwise shifting #145

Closed

axic changed the title ~~Draft for bitwise shifting opcodes~~ Bitwise shifting instructions in EVM Feb 13, 2017

axic force-pushed the bitwise-ops branch 2 times, most recently from 73cf336 to 4bab879 Compare February 13, 2017 16:22

Draft of bitwise shifting opcodes

4bab879

chriseth reviewed Feb 24, 2017

View reviewed changes

Arachnid added EIP labels Apr 14, 2017

axic added 4 commits April 23, 2017 11:28

Remove ROR/ROL

a63aef3

Add link to client support (cpp-ethereum)

0b34616

Swap shift operands (value is the top item)

7bc4477

Rename to EIP145

27847a8

chfast reviewed Apr 23, 2017

View reviewed changes

Clarify shift amount (arg2)

d4f5382

chfast approved these changes Apr 23, 2017

View reviewed changes

axic added 2 commits July 3, 2017 21:31

Use floor and not udiv/sdiv

e5ca919

Explain sign of arg1/arg2

4218665

axic force-pushed the bitwise-ops branch from 3db4f38 to 4218665 Compare July 3, 2017 20:36

This was referenced Jul 18, 2017

Add bit shifting opcodes (EIP145) ethereum/solidity#2541

Merged

Implement bitwise shifting (EIP145) ethereum/aleth#4054

Merged

axic mentioned this pull request Jul 28, 2017

All Core Devs Meeting 21 Agenda ethereum/pm#19

Closed

Add comparison with current opcodes

0b08d11

Souptacular mentioned this pull request Aug 6, 2017

Ethereum Core Devs Meeting 22 Agenda ethereum/pm#20

Closed

axic added 2 commits September 22, 2017 14:04

Clarify text (user -> use)

9464fa7

Swapp operand order for bitwise shifting

d01c9af

axic mentioned this pull request Sep 22, 2017

Ethereum Core Devs Meeting 25 Agenda ethereum/pm#23

Closed

chfast reviewed Sep 22, 2017

View reviewed changes

Clarify wording about arg1/arg2

74d9a17

chfast approved these changes Sep 22, 2017

View reviewed changes

gcolvin approved these changes Sep 22, 2017

View reviewed changes

gcolvin merged commit 35c7ade into ethereum:master Sep 22, 2017

axic deleted the bitwise-ops branch September 22, 2017 16:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bitwise shifting instructions in EVM #215

Bitwise shifting instructions in EVM #215

axic commented Feb 13, 2017 •

edited

Loading

chriseth Feb 24, 2017

gcolvin Feb 25, 2017

axic Mar 23, 2017

axic Apr 23, 2017

Arachnid commented Apr 14, 2017

axic commented Apr 22, 2017

chfast Apr 23, 2017

chfast left a comment

Arachnid commented Apr 24, 2017

axic commented Apr 26, 2017

chfast commented Apr 26, 2017

axic commented Jul 3, 2017

axic commented Jul 3, 2017

gcolvin commented Jul 3, 2017

holiman commented Jul 28, 2017

gcolvin commented Jul 28, 2017

axic commented Jul 28, 2017

gcolvin commented Jul 28, 2017

axic commented Jul 31, 2017

axic commented Aug 4, 2017 •

edited

Loading

chfast commented Aug 4, 2017

gcolvin commented Aug 5, 2017

chfast commented Aug 11, 2017 •

edited

Loading

axic commented Aug 11, 2017

axic commented Sep 22, 2017

chfast Sep 22, 2017

axic Sep 22, 2017 •

edited

Loading

chfast left a comment

axic commented Sep 22, 2017

gcolvin left a comment

axic commented Sep 22, 2017

Bitwise shifting instructions in EVM #215

Bitwise shifting instructions in EVM #215

Conversation

axic commented Feb 13, 2017 • edited Loading

chriseth Feb 24, 2017

Choose a reason for hiding this comment

gcolvin Feb 25, 2017

Choose a reason for hiding this comment

axic Mar 23, 2017

Choose a reason for hiding this comment

axic Apr 23, 2017

Choose a reason for hiding this comment

Arachnid commented Apr 14, 2017

axic commented Apr 22, 2017

chfast Apr 23, 2017

Choose a reason for hiding this comment

chfast left a comment

Choose a reason for hiding this comment

Arachnid commented Apr 24, 2017

axic commented Apr 26, 2017

chfast commented Apr 26, 2017

axic commented Jul 3, 2017

axic commented Jul 3, 2017

gcolvin commented Jul 3, 2017

holiman commented Jul 28, 2017

gcolvin commented Jul 28, 2017

axic commented Jul 28, 2017

gcolvin commented Jul 28, 2017

axic commented Jul 31, 2017

axic commented Aug 4, 2017 • edited Loading

chfast commented Aug 4, 2017

gcolvin commented Aug 5, 2017

chfast commented Aug 11, 2017 • edited Loading

axic commented Aug 11, 2017

axic commented Sep 22, 2017

chfast Sep 22, 2017

Choose a reason for hiding this comment

axic Sep 22, 2017 • edited Loading

Choose a reason for hiding this comment

chfast left a comment

Choose a reason for hiding this comment

axic commented Sep 22, 2017

gcolvin left a comment

Choose a reason for hiding this comment

axic commented Sep 22, 2017

axic commented Feb 13, 2017 •

edited

Loading

axic commented Aug 4, 2017 •

edited

Loading

chfast commented Aug 11, 2017 •

edited

Loading

axic Sep 22, 2017 •

edited

Loading