feat: add mod benchmark #1362
feat: add mod benchmark #1362
Conversation
This adds a preliminary implementation of the MOD module. The main outstanding issue is the fact that there is no support at this time for division. Realistically, this is needed since otherwise the implementation is incredibly inefficient.
These tests have been generated from mainnet blocks.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| return | ||
| smod: | ||
| RES, tmp = signed_divide(ARG_1, ARG_2) | ||
| return |
There was a problem hiding this comment.
Bug: SMOD operation returns quotient instead of remainder
The smod case incorrectly returns the quotient instead of the remainder. The signed_divide function returns (quot, rem), so RES, tmp = signed_divide(...) assigns the quotient to RES. For SMOD (signed modulo), it should be tmp, RES = signed_divide(...) to return the remainder, matching the pattern used for unsigned MOD at line 16.
| var abs_quot, abs_rem u256 | ||
| ;; | ||
| abs_num, num_s = abs(num) | ||
| abs_den, den_s = abs(num) |
There was a problem hiding this comment.
Bug: Wrong variable passed to abs function
The signed_divide function has a copy-paste error where abs_den, den_s = abs(num) computes the absolute value of the numerator instead of the denominator. It should be abs(den) to get the absolute value of the den parameter for the division operation.
| ;; negative result | ||
| quot = negate(abs_quot) | ||
| rem = negate(abs_rem) | ||
| return |
There was a problem hiding this comment.
Bug: Remainder sign incorrectly follows quotient sign logic
The signed_divide function applies the same sign logic to both quotient and remainder, but EVM semantics require different rules. The quotient sign depends on num_s XOR den_s (correctly implemented), but the remainder sign must depend only on num_s (the dividend's sign). Currently, both quot and rem are negated when num_s != den_s, which produces incorrect remainder values when the dividend and divisor have opposite signs.
Note
Adds
asm/bench/modwith zkASM implementation (DIV/MOD/SDIV/SMOD) and corresponding test plus accept vectors.Test_AsmBench_Modinpkg/test/assembly_bench_test.goto runasm/bench/mod.testdata/asm/bench/mod.zkasmimplementingmoddispatcher forEVM_INST_DIV,EVM_INST_MOD,EVM_INST_SDIV,EVM_INST_SMOD.signed_divide,abs,negate, and a temporary recursivedivide.testdata/asm/bench/mod.acceptscovering various inputs for unsigned/signed division and modulo.Written by Cursor Bugbot for commit 9b64fe3. This will update automatically on new commits. Configure here.