Update EIP-663: Move to Draft

[charles-cooper]

When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md

We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met:

• The PR edits only existing draft PRs.
• The build passes.
• Your GitHub username or email address is listed in the 'author' header of all affected PRs, inside .
• If matching on email address, the email address is the one publicly listed on your GitHub profile.

[eth-bot]

✅ All reviewers have approved.

[charles-cooper]

if stack depth addressability is a concern, EXCHANGE2 can be added (which can address at least up to 512 stack depth)

[ekpyron]

This is not a reasonable modification to EIP-663. Note that with this PR, the EIP's content does not even match the EIP title anymore.
You're trying to solve quite a different problem with EXCHANGE, so this seems more like hijacking the EIP to fully repurpose it, rather than an amendment :-).

I do appreciate that there are reasons why one may want an EXCHANGE opcode - stack scheduling does require more creative algorithms without it - but I'd argue that it should be proposed as a separate EIP. Other than both having something to do with stack, there is little relation between EIP-663 and the proposed EXCHANGE. I'd even be in support of such a new EIP, but as an (for us optional) addition to, not as replacement of, EIP-663 (and contingently on the acceptance of such a new EIP, EIP-663 could then be amended by an additional EXCHANGE2 - or conversely the new EIP should add that, if EIP-663 is accepted).

[charles-cooper]

This is not a reasonable modification to EIP-663. Note that with this PR, the EIP's content does not even match the EIP title anymore. You're trying to solve quite a different problem with EXCHANGE, so this seems more like hijacking the EIP to fully repurpose it, rather than an amendment :-).

I do appreciate that there are reasons why one may want an EXCHANGE opcode - stack scheduling does require more creative algorithms without it - but I'd argue that it should be proposed as a separate EIP. Other than both having something to do with stack, there is little relation between EIP-663 and the proposed EXCHANGE. I'd even be in support of such a new EIP, but as an (for us optional) addition to, not as replacement of, EIP-663 (and contingently on the acceptance of such a new EIP, EIP-663 could then be amended by an additional EXCHANGE2 - or conversely the new EIP should add that, if EIP-663 is accepted).

sorry, i don't think "hijacking" is a fair characterization here. i have been bringing this up for nearly a year now on the discussion thread (https://ethereum-magicians.org/t/eip-663-unlimited-swap-and-dup-instructions/3346/40), and regularly attending EOF meetings, and the suggestion was made that i make a PR directly against EIP-663.

additionally, i didn't add different variants (and there are several variants which i think are useful) of SWAP/EXCHANGE since i have been informed that opcode slot space is at a premium. so i tried to make a minimal change. but (and maybe this is just context from regularly attending and participating in the EOF calls, but did not come across expressed clearly in the PR) EXCHANGE and its EXCHANGE2 variant (with 2-byte immediate) fully subsume the functionality of the original proposed SWAPN opcode, with an opcode slot vs bytecode size tradeoff.

[ekpyron]

sorry, i don't think "hijacking" is a fair characterization here. i have been bringing this up for nearly a year now on the discussion thread (https://ethereum-magicians.org/t/eip-663-unlimited-swap-and-dup-instructions/3346/40), and regularly attending EOF meetings, and the suggestion was made that i make a PR directly against EIP-663.

additionally, i didn't add different variants (and there are several variants which i think are useful) of SWAP/EXCHANGE since i have been informed that opcode slot space is at a premium. so i tried to make a minimal change. but (and maybe this is just context from regularly attending and participating in the EOF calls, but did not come across expressed clearly in the PR) EXCHANGE and its EXCHANGE2 variant (with 2-byte immediate) fully subsume the functionality of the original proposed SWAPN opcode, with an opcode slot vs bytecode size tradeoff.

I apologize for the phrasing, I didn't mean to insinuate misintent - and I know that Vyper has been in favour of an exchange opcode in EOF for a long time. But I want it to be as clear as possible that this PR as it is stated here (which markedly does not include EXCHANGE2) is not an option. It clearly changes the EIP in such a way that it no longer solves the issue this EIP is meant to address, i.e. limited reach on stack.

If this is to be proposed as amendment of EIP-663 and not as a separate proposal, it has to keep addressing this problem, which the current state of the PR doesn't. A variant of the PR that introduces EXCHANGE2 instead or additionally would be a different story.

[ekpyron]

This argument here also technically doesn't hold up to scrutiny.
Proper EVM stack scheduling can avoid situations in which you need to exchange two slots neither of which are at the stack top. I.e. if you find yourself in the situation of having to swap the 3rd and 2nd position on stack, then you allocated an inefficient position for the values on stack when you produced them in the first place. I've given a talk at Devconnect Amsterdam about the topic, if you're interested.
Conversely, all values in the EVM are produced at the stack top and it thereby cannot be avoided to swap the stack top with a deeper position to create optimal stack layouts in the first place.

So, technically, SWAPN is actually the more efficient choice, since the more common and unavoidable operation on stack top requires only two bytes instead of three even for deep access, while conversely patterns like SWAP2 SWAP3 SWAP2 can generally be avoided by proper stack scheduling at the compiler level.

That being said, having written stack scheduling like that myself, I know that it is not the most trivial thing to do - that's why I've not immediately tried to shut down the idea of EXCHANGE2: it does allow for simpler more naive stack scheduling albeit at the cost of an increase in code size cost globally compared to optimal scheduling.

But either way, DUPN and EXCHANGE are not a replacement of SWAPN, so it would have to be EXCHANGE2 as well, no matter what.

[charles-cooper]

generally be avoided by proper stack scheduling at the compiler level.

the situation can definitely be improved with better algorithms, but optimal stack scheduling, especially across basic blocks and subroutines, is hard. (although i don't have a proof, i suspect it is NP-complete, in the same way that register allocation is). and, even if such a theoretically optimal stack scheduler were available, it is sometimes simply not possible in the general case to schedule things to be in the right order ahead of time, for instance, when side effects are interleaved with stack item production.

But either way, DUPN and EXCHANGE are not a replacement of SWAPN, so it would have to be EXCHANGE2 as well, no matter what.

having things too deep in the stack is also solvable by better memory spilling algorithms, so i don't think phrasing this choice in absolute terms is very constructive here. but sure, if EXCHANGE + EXCHANGE2 are not considered as taking up too many slots, why not.

[charles-cooper]

Note that with this PR, the EIP's content does not even match the EIP title anymore.

the original EIP itself did not match the EIP title! the title implies "unlimited" reach, but the opcodes only provide 256- slots of addressability. (and even the three-byte variant, of course, only allows addressing up to the 1024 limit imposed by the EVM). so clearly there is a limit, but it is deemed practically useful. the proposed EXCHANGE opcode also increases addressability, the same as the original EIP, it just increases the reach somewhat less (in exchange for the other useful semantics as already described).

[ekpyron]

The easiest thing to do and to avoid having to senselessly split hairs here would be to just make the introduction of EXCHANGE a separate EIP, since it's obviously a radically different change compared to the unmodified EIP-663 (you cannot reasonably deny that).
As I said, I'd grant that that EIP would have (independent) merit.

[charles-cooper]

modified the PR so it adds EXCHANGE in addition to(!) SWAPN.

[gumb0]

This limits the swapping of nth item to only items [n+1, n+16], so it can go 32 items deep but only with max distance 16 between items. E.g. you can't swap 1st with 32d.

Is this acceptable limitation for you?

[charles-cooper]

yes, we might want to change the encoding still.

[pdobacz]

A few more off-by-one nitpicks, since we increased the reach of EXCHANGE. Also the stack depths mentioned in the abstract aren't 100% accurate, but maybe that's intended to not overwhelm with detail

[pdobacz]

I think my other comment got lost: SWAPN cases got lost in merges, this entire section needs a revisit at the end. Also specifically it's n + m + 1 now for EXCHANGE

[pdobacz]

n + m + 1

[gumb0]

We hope to get EIP-3540 out of Stagnant soon #8217
then this can be left in Review

[pdobacz]

@charles-cooper ☝️ this can be undone now, 3540 is out of Stagnant.

[SamWilsn]

This'll still need approval from @axic to add the new authors.

[SamWilsn]

Same kind of comment for the description.

[github-actions]

The commit b602f71 (as a parent of 4462c22) contains errors.
Please inspect the Run Summary for details.

[eth-bot]

All Reviewers Have Approved; Performing Automatic Merge...

[eth-bot]

All Reviewers Have Approved; Performing Automatic Merge...