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.

Sign up for GitHub

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 definition of m in EXCHANGE #174

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+4 −3
Open

Change definition of m in EXCHANGE #174

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
52a9eba
Change definition of EXCHANGE immediates
frangio Dec 12, 2024
44b5781
define in terms of i,j intermediate vars
frangio Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions spec/eof.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,9 +256,10 @@ The following instructions are introduced in EOF code:
- `EXCHANGE (0xe8)` instruction
- deduct 3 gas
- read uint8 operand `imm`
- `n = imm >> 4 + 1`, `m = imm & 0x0F + 1`
- `n + 1`th stack item is swapped with `n + m + 1`th stack item (1-based).
- Stack validation: `stack_height >= n + m + 1`
- `i = imm >> 4`, `j = imm & 0x0F`
Copy link
Member

Choose a reason for hiding this comment

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

ah, I didn't see this has been changed to i/j already. I propose to align with EEST (and be compatible with past version), so:

  • n and m remain what they were
  • x and y are the new args
  • end with: "xth stack item is swapped with yth stack item (1-based)."

This way, any EVM/test/whatever code, which doesn't update to your convention, still uses the same names for the same things.

Copy link
Member

Choose a reason for hiding this comment

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

given the SWAPn/SWAPN/EXCHANGE consistency argument, I take the above comment back. Pls make sure then that conventions are sound within this document (I think they are as of 44b5781), and I think we'll work from there

Copy link
Member

@pdobacz pdobacz Dec 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

welp, there's no equivalence with the old SWAP1..16, but that I think is not fixable, so

EXCHANGE x y ≡ SWAPN x SWAPN y SWAPN x ≡ SWAP[x-1] SWAP[y-1] SWAP[x-1]

encoded in bytecode as

0xe8nibble[x-2]nibble[y-x-1] ≡ 0xe7[x-2] 0xe7[y-2] 0xe7[x-2] ≡ 0x[90+x-2][90+y-2][90+x-2]

Concretely

EXCHANGE 2 3 ≡ SWAPN 2 SWAPN 3 SWAPN 2 ≡ SWAP1 SWAP2 SWAP3

0xe800 ≡ 0xe700e701e700 ≡ 0x909190

The not fixable part can be swept under the rug by the assembler, which would not allow verbatim SWAP1..16 in assembler code, but would use them in output bytecode for low instances of SWAPN[...] as optimization.

I risk to say that DUP/DUPN are aligned (and similarly "unfixable" and it's similarly "fine"). But please double check my above ramblings, I made like 3 errors in the +-1's there

Copy link
Author

Choose a reason for hiding this comment

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

This is not the notation/encoding I have in mind. For example, I'd write SWAPN 1 (n=1, encoded 0xe700) as equivalent to SWAP1 (0x90).

I'm going to write some more comprehensive notes to try to get everyone aligned.

- `n = i + 1`, `m = j + i + 2`
- `n + 1`th stack item is swapped with `m + 1`th stack item (1-based).
- Stack validation: `stack_height >= m + 1`
- `RETURNDATALOAD (0xf7)` instruction
- deduct 3 gas
- pop `offset` from the stack
Expand Down
Loading