Ohadn/qm31 operations #1938

ohad-nir-starkware · 2025-02-09T18:58:27Z

QM31Operations

Description

add packed reduced qm31 add, mul, sub, div via OpcodeExtension::QM31Operations.

Description of the pull request changes and motivation.

Checklist

Linked to Github Issue
Unit tests added
Integration tests added.
This change requires new documentation.
- Documentation has been added/updated.
- CHANGELOG has been updated.

This change is

github-actions · 2025-02-09T19:07:44Z

**Hyper Thereading Benchmark results**




hyperfine -r 2 -n "hyper_threading_main threads: 1" 'RAYON_NUM_THREADS=1 ./hyper_threading_main' -n "hyper_threading_pr threads: 1" 'RAYON_NUM_THREADS=1 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 1
  Time (mean ± σ):     28.232 s ±  0.025 s    [User: 27.463 s, System: 0.767 s]
  Range (min … max):   28.214 s … 28.250 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 1
  Time (mean ± σ):     26.996 s ±  0.028 s    [User: 26.214 s, System: 0.781 s]
  Range (min … max):   26.976 s … 27.016 s    2 runs
 
Summary
  hyper_threading_pr threads: 1 ran
    1.05 ± 0.00 times faster than hyper_threading_main threads: 1




hyperfine -r 2 -n "hyper_threading_main threads: 2" 'RAYON_NUM_THREADS=2 ./hyper_threading_main' -n "hyper_threading_pr threads: 2" 'RAYON_NUM_THREADS=2 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 2
  Time (mean ± σ):     15.638 s ±  0.034 s    [User: 27.614 s, System: 0.784 s]
  Range (min … max):   15.614 s … 15.663 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 2
  Time (mean ± σ):     14.981 s ±  0.048 s    [User: 26.393 s, System: 0.788 s]
  Range (min … max):   14.947 s … 15.014 s    2 runs
 
Summary
  hyper_threading_pr threads: 2 ran
    1.04 ± 0.00 times faster than hyper_threading_main threads: 2




hyperfine -r 2 -n "hyper_threading_main threads: 4" 'RAYON_NUM_THREADS=4 ./hyper_threading_main' -n "hyper_threading_pr threads: 4" 'RAYON_NUM_THREADS=4 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 4
  Time (mean ± σ):     11.458 s ±  0.226 s    [User: 40.572 s, System: 0.950 s]
  Range (min … max):   11.298 s … 11.618 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 4
  Time (mean ± σ):     11.366 s ±  0.008 s    [User: 39.382 s, System: 0.921 s]
  Range (min … max):   11.360 s … 11.372 s    2 runs
 
Summary
  hyper_threading_pr threads: 4 ran
    1.01 ± 0.02 times faster than hyper_threading_main threads: 4




hyperfine -r 2 -n "hyper_threading_main threads: 6" 'RAYON_NUM_THREADS=6 ./hyper_threading_main' -n "hyper_threading_pr threads: 6" 'RAYON_NUM_THREADS=6 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 6
  Time (mean ± σ):     11.391 s ±  0.052 s    [User: 40.913 s, System: 0.956 s]
  Range (min … max):   11.354 s … 11.428 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 6
  Time (mean ± σ):     10.906 s ±  0.223 s    [User: 39.835 s, System: 0.939 s]
  Range (min … max):   10.748 s … 11.064 s    2 runs
 
Summary
  hyper_threading_pr threads: 6 ran
    1.04 ± 0.02 times faster than hyper_threading_main threads: 6




hyperfine -r 2 -n "hyper_threading_main threads: 8" 'RAYON_NUM_THREADS=8 ./hyper_threading_main' -n "hyper_threading_pr threads: 8" 'RAYON_NUM_THREADS=8 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 8
  Time (mean ± σ):     11.166 s ±  0.074 s    [User: 41.329 s, System: 0.951 s]
  Range (min … max):   11.113 s … 11.218 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 8
  Time (mean ± σ):     10.790 s ±  0.183 s    [User: 40.255 s, System: 0.948 s]
  Range (min … max):   10.660 s … 10.920 s    2 runs
 
Summary
  hyper_threading_pr threads: 8 ran
    1.03 ± 0.02 times faster than hyper_threading_main threads: 8




hyperfine -r 2 -n "hyper_threading_main threads: 16" 'RAYON_NUM_THREADS=16 ./hyper_threading_main' -n "hyper_threading_pr threads: 16" 'RAYON_NUM_THREADS=16 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 16
  Time (mean ± σ):     11.003 s ±  0.155 s    [User: 41.489 s, System: 1.060 s]
  Range (min … max):   10.893 s … 11.112 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 16
  Time (mean ± σ):     10.785 s ±  0.243 s    [User: 40.490 s, System: 1.067 s]
  Range (min … max):   10.613 s … 10.957 s    2 runs
 
Summary
  hyper_threading_pr threads: 16 ran
    1.02 ± 0.03 times faster than hyper_threading_main threads: 16

github-actions · 2025-02-09T19:20:16Z

Benchmark Results for unmodified programs 🚀

Command	Mean [s]	Min [s]	Max [s]	Relative
`base big_factorial`	2.216 ± 0.026	2.197	2.281	1.02 ± 0.03
`head big_factorial`	2.170 ± 0.051	2.143	2.311	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base big_fibonacci`	2.166 ± 0.012	2.151	2.192	1.04 ± 0.01
`head big_fibonacci`	2.084 ± 0.014	2.070	2.114	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base blake2s_integration_benchmark`	8.098 ± 0.081	7.975	8.209	1.04 ± 0.01
`head blake2s_integration_benchmark`	7.822 ± 0.042	7.717	7.881	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base compare_arrays_200000`	2.274 ± 0.013	2.256	2.296	1.02 ± 0.01
`head compare_arrays_200000`	2.220 ± 0.029	2.190	2.268	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base dict_integration_benchmark`	1.505 ± 0.016	1.484	1.534	1.04 ± 0.01
`head dict_integration_benchmark`	1.454 ± 0.004	1.448	1.460	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base field_arithmetic_get_square_benchmark`	1.279 ± 0.007	1.269	1.291	1.03 ± 0.01
`head field_arithmetic_get_square_benchmark`	1.241 ± 0.011	1.224	1.259	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base integration_builtins`	8.176 ± 0.161	8.070	8.594	1.05 ± 0.02
`head integration_builtins`	7.791 ± 0.054	7.701	7.852	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base keccak_integration_benchmark`	8.434 ± 0.117	8.326	8.727	1.03 ± 0.01
`head keccak_integration_benchmark`	8.164 ± 0.035	8.127	8.249	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base linear_search`	2.261 ± 0.011	2.248	2.284	1.03 ± 0.01
`head linear_search`	2.191 ± 0.010	2.177	2.210	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_cmp_and_pow_integration_benchmark`	1.575 ± 0.005	1.568	1.583	1.02 ± 0.01
`head math_cmp_and_pow_integration_benchmark`	1.538 ± 0.008	1.527	1.557	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_integration_benchmark`	1.526 ± 0.009	1.517	1.547	1.03 ± 0.01
`head math_integration_benchmark`	1.487 ± 0.005	1.480	1.498	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base memory_integration_benchmark`	1.265 ± 0.004	1.259	1.271	1.02 ± 0.01
`head memory_integration_benchmark`	1.237 ± 0.013	1.228	1.272	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base operations_with_data_structures_benchmarks`	1.635 ± 0.003	1.629	1.640	1.02 ± 0.00
`head operations_with_data_structures_benchmarks`	1.602 ± 0.004	1.597	1.607	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base pedersen`	547.2 ± 3.4	544.0	553.3	1.02 ± 0.01
`head pedersen`	537.3 ± 4.6	534.3	549.7	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base poseidon_integration_benchmark`	644.2 ± 5.0	639.8	654.9	1.00 ± 0.01
`head poseidon_integration_benchmark`	641.4 ± 6.1	633.0	651.6	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base secp_integration_benchmark`	1.913 ± 0.006	1.903	1.922	1.02 ± 0.01
`head secp_integration_benchmark`	1.876 ± 0.012	1.860	1.898	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base set_integration_benchmark`	675.6 ± 11.5	668.6	707.5	1.02 ± 0.02
`head set_integration_benchmark`	663.1 ± 1.9	659.2	666.6	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base uint256_integration_benchmark`	4.465 ± 0.040	4.401	4.529	1.04 ± 0.01
`head uint256_integration_benchmark`	4.292 ± 0.021	4.259	4.316	1.00

codecov · 2025-02-09T19:25:17Z

Codecov Report

Attention: Patch coverage is 99.77974% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.45%. Comparing base (fa977b3) to head (39fa8ad).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
vm/src/vm/decoding/decoder.rs	97.91%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1938      +/-   ##
==========================================
+ Coverage   96.41%   96.45%   +0.03%     
==========================================
  Files         102      103       +1     
  Lines       41861    42285     +424     
==========================================
+ Hits        40362    40785     +423     
- Misses       1499     1500       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

DavidLevitGurevich

Reviewed 1 of 6 files at r4, 1 of 6 files at r5.
Reviewable status: 10 of 17 files reviewed, 1 unresolved discussion (waiting on @fmoletta, @gabrielbosio, @igaray, @juanbono, @Oppen, @pefontana, and @yuvalsw)

JulianGCalderon · 2025-02-19T20:22:24Z

vm/src/vm/decoding/decoder.rs

+            if (res != Res::Add && res != Res::Mul)
+                || op1_addr == Op1Addr::Op0
+                || pc_update != PcUpdate::Regular
+                || opcode != Opcode::AssertEq
+                || (ap_update_num != 0 && ap_update_num != 2)
+            {
+                return Err(VirtualMachineError::InvalidQM31AddMulFlags(flags & 0x7FFF));
+            }
+            OpcodeExtension::QM31Operation


Could you move this check logic down, to be more consistent with what is done with the Blake checks?

vm/src/vm/decoding/decoder.rs

ohad-nir-starkware

Reviewable status: 9 of 19 files reviewed, 3 unresolved discussions (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @JulianGCalderon, @Oppen, @pefontana, and @yuvalsw)

ohad-nir-starkware · 2025-02-19T23:11:12Z

vm/src/vm/decoding/decoder.rs

+            if (res != Res::Add && res != Res::Mul)
+                || op1_addr == Op1Addr::Op0
+                || pc_update != PcUpdate::Regular
+                || opcode != Opcode::AssertEq
+                || (ap_update_num != 0 && ap_update_num != 2)
+            {
+                return Err(VirtualMachineError::InvalidQM31AddMulFlags(flags & 0x7FFF));
+            }
+            OpcodeExtension::QM31Operation


vm/src/vm/decoding/decoder.rs

DavidLevitGurevich

Reviewed 1 of 6 files at r4, 3 of 7 files at r6, 2 of 2 files at r7, all commit messages.
Reviewable status: 15 of 19 files reviewed, 4 unresolved discussions (waiting on @fmoletta, @gabrielbosio, @igaray, @juanbono, @JulianGCalderon, @Oppen, @pefontana, and @yuvalsw)

vm/src/vm/vm_core.rs line 2616 at r7 (raw file):

    #[test]
    #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
    fn compute_res_qm31_add_relocatable_values() {

we need more cases covered

vm/src/vm/decoding/decoder.rs line 128 at r7 (raw file):

    }

    let qm31_operation_flags_invalid = (res != Res::Add && res != Res::Mul)

please change to ..._valid

DavidLevitGurevich

Reviewed 3 of 7 files at r6.
Reviewable status: 18 of 19 files reviewed, 5 unresolved discussions (waiting on @fmoletta, @gabrielbosio, @igaray, @juanbono, @JulianGCalderon, @ohad-nir-starkware, @Oppen, @pefontana, and @yuvalsw)

vm/src/typed_operations.rs line 22 at r7 (raw file):

) -> Result<MaybeRelocatable, VirtualMachineError> {
    match opcode_extension {
        OpcodeExtension::Stone => Ok(x.add(y)?),

why not just

OpcodeExtension::Stone => x.add(y),

DavidLevitGurevich

Reviewed 1 of 11 files at r3.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @fmoletta, @gabrielbosio, @igaray, @juanbono, @JulianGCalderon, @ohad-nir-starkware, @Oppen, @pefontana, and @yuvalsw)

cairo_programs/stwo_exclusive_programs/qm31_opcodes_test.cairo line 80 at r7 (raw file):

) -> felt {
    alloc_locals;

remove spaces

cairo_programs/stwo_exclusive_programs/qm31_opcodes_test.cairo line 118 at r7 (raw file):

        assert flags[1] = 1; // flag_op0_base_fp
    }
    assert flags[3] = 2 - is_imm - flags[0] - flags[1]; // flag_op1_base_fp

(2 - flags[0] - flags[1]) * (1 - is_imm)

cairo_programs/stwo_exclusive_programs/qm31_opcodes_test.cairo line 176 at r7 (raw file):

    }
    if (is_mul == TRUE) {
        dw 0x1c05380007ffd7ffc;

missing two asserts

ohad-nir-starkware

Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @JulianGCalderon, @Oppen, @pefontana, and @yuvalsw)

cairo_programs/stwo_exclusive_programs/qm31_opcodes_test.cairo line 80 at r7 (raw file):