[EraVM][MC] Refine the definitions of ret-like instructions #567

atrosinenko · 2024-05-23T15:01:15Z

Make R1 register be understood as the default operand of RETrl and
REVERTrl instructions (but make this alias non-default for printing
until switched to the new asm syntax).

Change panic instruction to either accept jump target operand
or none at all.

atrosinenko · 2024-05-23T15:02:27Z

@sayon @akiramenai Please check that this patch models the spec correctly.

atrosinenko · 2024-05-23T16:43:41Z

Undone changes a bit to merge as much as possible before renaming the mnemonics:

always print return- and revert-to-label with a hardcoded r1 before label (not a real operand as of this PR and not accepted on input)
use just panic for no-operand panic instruction (instead of ret.panic)

github-actions · 2024-05-23T17:14:28Z

Benchmark results:

╔═╡ Size (-%) ╞════════════════╡ All M3B3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞══════════════╡ All M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞════════════════╡ All M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞════════════════╡ All MzB3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞══════════════╡ All MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞════════════════╡ All MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞═════╡ EVMInterpreter M3B3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                                 NaN ║
╠═╡ Cycles (-%) ╞═══╡ EVMInterpreter M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞═════╡ EVMInterpreter M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs/gas ╞══════╡ EVMInterpreter M3B3 ╞═╣
║ ADD                                69.333 ║
║ MUL                                40.400 ║
║ SUB                                69.333 ║
║ DIV                                48.800 ║
║ SDIV                               65.600 ║
║ MOD                                47.600 ║
║ SMOD                               63.200 ║
║ ADDMOD                             39.625 ║
║ MULMOD                             36.625 ║
║ EXP                                 9.667 ║
║ SIGNEXTEND                         46.400 ║
║ LT                                 73.333 ║
║ GT                                 73.333 ║
║ SLT                                97.333 ║
║ SGT                                95.333 ║
║ EQ                                 73.333 ║
║ ISZERO                             65.000 ║
║ AND                                67.333 ║
║ OR                                 69.333 ║
║ XOR                                69.333 ║
║ NOT                                61.000 ║
║ BYTE                               77.333 ║
║ SHL                                75.333 ║
║ SHR                                73.333 ║
║ SAR                                91.333 ║
║ SGT                                95.333 ║
║ SHA3                               28.519 ║
║ ADDRESS                            93.781 ║
║ BALANCE                            73.230 ║
║ ORIGIN                           1389.969 ║
║ CALLER                             93.781 ║
║ CALLVALUE                          93.781 ║
║ CALLDATALOAD                       63.333 ║
║ CALLDATASIZE                       94.000 ║
║ CALLDATACOPY                       71.533 ║
║ CODESIZE                           91.000 ║
║ CODECOPY                          145.574 ║
║ GASPRICE                         1392.688 ║
║ EXTCODESIZE                         5.149 ║
║ EXTCODECOPY                         4.667 ║
║ RETURNDATASIZE                     89.000 ║
║ RETURNDATACOPY                     55.889 ║
║ EXTCODEHASH                         9.087 ║
║ BLOCKHASH                         244.700 ║
║ COINBASE                         1392.969 ║
║ TIMESTAMP                        1389.969 ║
║ NUMBER                           1389.969 ║
║ PREVRANDAO                       1389.969 ║
║ GASLIMIT                         1395.969 ║
║ CHAINID                          1389.969 ║
║ SELFBALANCE                       641.312 ║
║ BASEFEE                          1386.969 ║
║ POP                                81.500 ║
║ MLOAD                              80.186 ║
║ MSTORE                             82.069 ║
║ MSTORE8                            89.912 ║
║ SLOAD                              27.212 ║
║ SSTORE                              9.099 ║
║ JUMP                               37.333 ║
║ JUMPI                              32.818 ║
║ PC                                 94.281 ║
║ MSIZE                             103.781 ║
║ GAS                                88.281 ║
║ JUMPDEST                          127.562 ║
║ PUSH0                              91.281 ║
║ PUSH1                              78.854 ║
║ PUSH2                              96.854 ║
║ PUSH4                             158.854 ║
║ PUSH5                             180.854 ║
║ PUSH6                             202.854 ║
║ PUSH7                             224.854 ║
║ PUSH8                             246.854 ║
║ PUSH9                             268.854 ║
║ PUSH10                            290.854 ║
║ PUSH11                            312.854 ║
║ PUSH12                            332.854 ║
║ PUSH13                            356.854 ║
║ PUSH14                            378.854 ║
║ PUSH15                            400.854 ║
║ PUSH16                            422.854 ║
║ PUSH17                            444.854 ║
║ PUSH18                            466.854 ║
║ PUSH19                            488.854 ║
║ PUSH20                            510.854 ║
║ PUSH21                            532.854 ║
║ PUSH22                            554.854 ║
║ PUSH23                            576.854 ║
║ PUSH24                            598.854 ║
║ PUSH25                            620.854 ║
║ PUSH26                            642.854 ║
║ PUSH27                            664.854 ║
║ PUSH28                            686.854 ║
║ PUSH29                            708.854 ║
║ PUSH30                            730.854 ║
║ PUSH31                            752.854 ║
║ PUSH32                            774.854 ║
║ DUP1                               63.000 ║
║ DUP2                               67.000 ║
║ DUP3                               67.000 ║
║ DUP4                               67.000 ║
║ DUP5                               67.000 ║
║ DUP6                               67.000 ║
║ DUP7                               67.000 ║
║ DUP8                               67.000 ║
║ DUP9                               67.000 ║
║ DUP10                              67.000 ║
║ DUP11                              67.000 ║
║ DUP12                              67.000 ║
║ DUP13                              65.000 ║
║ DUP14                              67.000 ║
║ DUP15                              67.000 ║
║ DUP16                              67.000 ║
║ SWAP1                              67.667 ║
║ SWAP2                              67.667 ║
║ SWAP3                              67.667 ║
║ SWAP4                              67.667 ║
║ SWAP5                              67.667 ║
║ SWAP6                              67.667 ║
║ SWAP7                              67.667 ║
║ SWAP8                              67.667 ║
║ SWAP9                              67.667 ║
║ SWAP10                             67.667 ║
║ SWAP11                             67.667 ║
║ SWAP12                             67.667 ║
║ SWAP13                             67.667 ║
║ SWAP14                             67.667 ║
║ SWAP15                             65.667 ║
║ SWAP16                             67.667 ║
║ CALL                               56.371 ║
║ STATICCALL                         53.248 ║
║ DELEGATECALL                       52.286 ║
║ CREATE                              5.046 ║
║ CREATE2                             7.067 ║
║ RETURN                              1.000 ║
║ REVERT                              1.000 ║
╠═╡ Ergs/gas (-%) ╞═╡ EVMInterpreter M3B3 ╞═╣
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞═════╡ EVMInterpreter MzB3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                                 NaN ║
╠═╡ Cycles (-%) ╞═══╡ EVMInterpreter MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞═════╡ EVMInterpreter MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞════════╡ Precompiles M3B3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞══════╡ Precompiles M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞════════╡ Precompiles M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞════════╡ Precompiles MzB3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞══════╡ Precompiles MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞════════╡ Precompiles MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞══════════╡ Real life M3B3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞════════╡ Real life M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞══════════╡ Real life M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞══════════╡ Real life MzB3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞════════╡ Real life MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞══════════╡ Real life MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

akiramenai

The implementation LGTM, thanks.
@sayon could you double check the constraint on ret-like input with the spec?

sayon · 2024-05-27T09:42:18Z

The constraints LGTM.

Question: for the disassembler, will the non-sensical variants like ret r9, somelabel be displayed? It is encode-able, but in any case either the register or the label will be ignored. Far return ignores labels, near returns ignore register.
Therefore in spirit of our decisions for the other instructions it would make sense to display it in a disassembler (for diagnostics) but reject it in assembler.

atrosinenko · 2024-05-27T10:24:25Z

@sayon Instructions like ret r9, somelabel are supported in the main branch now and this PR is mostly about removing support for them, as described in the current spec (aside from that, this PR removes register operand from PANIC altogether). Is the original encoding (reg vs reg, label) better?

On the other hand, maybe defining all 24 variants of NOP was too permissive - but it is encoded very similar to other arithmetic instructions (except for not having the set_flags modifier) and has no side effects except for updating SP anyway.

Considering the nonsensical-ness of ret r9, somelabel: are there use cases where it is not known at compile-time if it is a near or far return? The existing spec on assembler syntax seems to suggest any instruction is statically either near or far ret/revert/panic.

atrosinenko · 2024-05-27T10:38:56Z

The question seems to boil down to the following: what happens if an instruction with the opcode set to 1070 (OpRet with to_label being true) ends up being executed as far return at run-time:

if it is rejected by the VM, then it may be worth rejecting it in the assembler for clarity
if it is permitted by design, then it may be worth allowing it both in the assembler and the spec, but I'm not sure

sayon · 2024-05-27T10:57:24Z

It is statically impossible to distinguish between emitted far returns and near returns.
We know which one we want during codegen.
From VM's perspective, there is a single type of return instruction and it decides dynamically whether it is far or near return, depending on the topmost stack frame.

A third party may forge an instruction like ret r5, somelabel which will behave differently based on the type of the topmost callframe. It should not be a problem for security or anything, but it would be nice to distinguish it in disassembler.

Here we write that the semantic is decided in runtime; please tell me if it is not clear and I will try to rework/reformulate/make it more visible.

akiramenai · 2024-05-27T11:53:47Z

+1 to @sayon. It sounds like it makes sense to emit an error if to assemble ret r9, somelabel, but disassemble it if it's really in the code.
I don't know though if it's a problem that we can't generally assemble what was disassebled.

sayon · 2024-05-27T12:06:25Z

If this is a problem, we may allow to assemble ret r9, somelabel, but emit a warning. Later we may introduce some CLI option like "allow-extended-instruction-set" to be able to emit such instructions (same with e.g. nop stack-=[...], r0, stack+=[...], r9), but normally reject them.

Make R1 register be understood as the default operand of `RETrl` and `REVERTrl` instructions (but make this alias non-default for printing until switched to the new asm syntax). Change panic instruction to either accept jump target operand or none at all.

atrosinenko · 2024-05-27T13:31:04Z

Updated the patch and the description. Now instructions like ret r9, @somelabel are supported again, but the aliases are added for the canonical case of register operand being r1. Register operands are still rejected for any variants of panic instruction.

Until switching to the new syntax, the aliases for "r1 by default" are disabled when printing (for compatibility with the standalone assembler), so the test has the following seemingly meaningless lines:

; CHECK:  ret.ok.to_label       r1, @label
...
; CHECK:  ret.ok.to_label       r3, @label

After switching to the new syntax and removing let EmitPriority = 0 in { ... } they will be something like

; CHECK:  retl       @label
...
; CHECK:  retl       r3, @label

atrosinenko requested review from asl, sayon and akiramenai May 23, 2024 15:01

atrosinenko force-pushed the eravm-mc/refine-ret-like branch 2 times, most recently from 0948901 to 3f58870 Compare May 23, 2024 16:38

asl force-pushed the eravm-mc/far-call-stsh-modifier branch from 0ef436a to 8f90120 Compare May 23, 2024 18:30

atrosinenko mentioned this pull request May 23, 2024

[EraVM][TableGen] Split EraVMOpcodes for ret-like instructions (NFC) #569

Merged

Base automatically changed from eravm-mc/far-call-stsh-modifier to main May 24, 2024 08:18

atrosinenko force-pushed the eravm-mc/refine-ret-like branch from 3f58870 to e089f1f Compare May 24, 2024 09:14

akiramenai approved these changes May 24, 2024

View reviewed changes

atrosinenko force-pushed the eravm-mc/refine-ret-like branch from b0f1e15 to e089f1f Compare May 27, 2024 08:40

atrosinenko mentioned this pull request May 27, 2024

[EraVM][MC] Add missing variants of NOP instruction #571

Merged

atrosinenko force-pushed the eravm-mc/refine-ret-like branch from e089f1f to aa62e12 Compare May 27, 2024 13:21

atrosinenko requested a review from akiramenai May 27, 2024 13:24

akiramenai approved these changes May 27, 2024

View reviewed changes

akiramenai merged commit 6b107d9 into main May 27, 2024
8 checks passed

akiramenai deleted the eravm-mc/refine-ret-like branch May 27, 2024 16:40

This was referenced May 28, 2024

[MC] Emit warnings for non-standard and deprecated instructions #577

Open

[MC] Refine Near / Far Ret / Revert / Panic #455

Closed

atrosinenko mentioned this pull request May 28, 2024

[MC][AsmParser] Rename mnemonics according to the specification #441

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EraVM][MC] Refine the definitions of ret-like instructions #567

[EraVM][MC] Refine the definitions of ret-like instructions #567

atrosinenko commented May 23, 2024 •

edited

Loading

atrosinenko commented May 23, 2024

atrosinenko commented May 23, 2024

github-actions bot commented May 23, 2024 •

edited

Loading

akiramenai left a comment

sayon commented May 27, 2024

atrosinenko commented May 27, 2024

atrosinenko commented May 27, 2024

sayon commented May 27, 2024

akiramenai commented May 27, 2024

sayon commented May 27, 2024

atrosinenko commented May 27, 2024

[EraVM][MC] Refine the definitions of ret-like instructions #567

[EraVM][MC] Refine the definitions of ret-like instructions #567

Conversation

atrosinenko commented May 23, 2024 • edited Loading

atrosinenko commented May 23, 2024

atrosinenko commented May 23, 2024

github-actions bot commented May 23, 2024 • edited Loading

akiramenai left a comment

Choose a reason for hiding this comment

sayon commented May 27, 2024

atrosinenko commented May 27, 2024

atrosinenko commented May 27, 2024

sayon commented May 27, 2024

akiramenai commented May 27, 2024

sayon commented May 27, 2024

atrosinenko commented May 27, 2024

atrosinenko commented May 23, 2024 •

edited

Loading

github-actions bot commented May 23, 2024 •

edited

Loading