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

switch vm stack to little endian representation #7

Merged
merged 5 commits into from
Feb 15, 2022

Conversation

matklad
Copy link

@matklad matklad commented Feb 11, 2022

H256 is [u8; 32]
U256 is [u64; 4]

According to EVM spec, when H256 is interpreted as a number, the byte
order is big endian. WASM, in contrast, uses little-endian
representation (the same used by CPUs internally).

Before this PR, evm stored operand stack as Vec<H256>, so any
arithmetic op entailed indianness conversion. As WASM lacks equivalent
to bswap instruction, this is a lot of work.

After this PR, stack is using U256 (so, little-endian in memory). This
is also reperesentation used by all of geth, evmone and odin.

For the uniswap benchmark, the before-after WASM (not total) gas cost is

110436615921984
90540800264244

Notes:

  • I've made this on top of optimize mload instruction to not allocate a temporary vector rust-ethereum/evm#97
  • I think it also makes sense to rename macros further (pop_u256 -> pop), such that we only use a suffix for non-standard operation. I didn't do this in the PR to minimize diff.
  • I think this should be upstreamable. The fact that both geth and evmone use little-endian stack seems to me a very strong argument that this is a generally good idea.
  • $DEITY bless strong types! The fact that H256 and U256 are different types makes me so much more confident in this change!

matklad and others added 4 commits February 11, 2022 14:07
Naked-eye based optimization -- it feels suboptimal to allocate a `Vec`
to fetch `[0; u32]`. Running a macro-benchmark in aurora shows that this
is about a 1% win,  so the compiler probably can't eliminate this vec
entirely.
H256 is [u8; 32]
U256 is [u64; 4]

According to EVM spec, when H256 is interpreted as a number, the byte
order is big endian. WASM, in contrast, uses little-endian
representation (the same used by CPUs internally).

Before this PR, evm stored operand stack as `Vec<H256>`, so any
arithmetic op entailed indianness conversion. As WASM lacks equivalent
to bswap instruction, this is a lot of work.

After this PR, stack is using U256 (so, little-endian in memory). This
is also reperesentation used by all of geth, evmone and odin.

For the uniswap benchmark, the before-after WASM (not total) gas cost is

110436615921984
 90540800264244
Copy link
Member

@birchmd birchmd left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @matklad for these performance improvements!

@matklad
Copy link
Author

matklad commented Feb 11, 2022

(pushed one more tiny improvement)

Copy link

@joshuajbouw joshuajbouw left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks man.

aleksuss added a commit to aleksuss/sputnikvm that referenced this pull request Jul 2, 2024
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