-
Notifications
You must be signed in to change notification settings - Fork 49
MLIR implementation of euclidean algorithm #1357
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.
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
Conversation
Benchmarking resultsBenchmark for program
|
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
11.023 ± 0.052 | 10.961 | 11.123 | 4.54 ± 0.03 |
cairo-native (embedded AOT) |
2.428 ± 0.013 | 2.408 | 2.449 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2.519 ± 0.027 | 2.491 | 2.571 | 1.04 ± 0.01 |
Benchmark for program dict_snapshot
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
522.0 ± 4.4 | 515.1 | 530.1 | 1.00 |
cairo-native (embedded AOT) |
2094.0 ± 9.8 | 2072.7 | 2105.1 | 4.01 ± 0.04 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2221.0 ± 14.0 | 2197.1 | 2240.9 | 4.25 ± 0.05 |
Benchmark for program factorial_2M
Open benchmarks
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.792 ± 0.014 | 4.779 | 4.820 | 1.86 ± 0.03 |
cairo-native (embedded AOT) |
2.576 ± 0.047 | 2.537 | 2.700 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2.593 ± 0.014 | 2.571 | 2.615 | 1.01 ± 0.02 |
Benchmark for program fib_2M
Open benchmarks
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.741 ± 0.021 | 4.717 | 4.782 | 2.28 ± 0.02 |
cairo-native (embedded AOT) |
2.083 ± 0.019 | 2.060 | 2.120 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2.127 ± 0.011 | 2.114 | 2.143 | 1.02 ± 0.01 |
Benchmark for program linear_search
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
559.7 ± 7.9 | 546.6 | 570.0 | 1.00 |
cairo-native (embedded AOT) |
2152.3 ± 9.3 | 2138.9 | 2166.7 | 3.85 ± 0.06 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2304.7 ± 23.2 | 2259.3 | 2338.0 | 4.12 ± 0.07 |
Benchmark for program logistic_map
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
377.1 ± 6.8 | 371.6 | 390.1 | 1.00 |
cairo-native (embedded AOT) |
2238.2 ± 7.0 | 2224.7 | 2244.4 | 5.94 ± 0.11 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2419.1 ± 31.7 | 2387.0 | 2480.0 | 6.42 ± 0.14 |
Benchmark results Main vs HEAD.Base
Head
Base
Head
Base
Head
Base
Head
Base
Head
Base
Head
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1357 +/- ##
==========================================
+ Coverage 76.22% 76.27% +0.05%
==========================================
Files 111 111
Lines 26520 26602 +82
==========================================
+ Hits 20214 20291 +77
- Misses 6306 6311 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…tive into euclidean-mlir-func
Co-authored-by: Julian Gonzalez Calderon <[email protected]>
…tive into euclidean-mlir-func
src/metadata/runtime_bindings.rs
Outdated
| rhs_value: Value<'c, '_>, | ||
| circuit_modulus: Value<'c, '_>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that this is a generic implementation of egcd, we could rename arguments to a and b. (no need to keep the circuit-related names)
a: Value<'c, '_>,
b: Value<'c, '_>,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, done in 7335dd8
Nope, i'll change the description to clarify that |
JulianGCalderon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Co-authored-by: Gabriel Bosio <[email protected]>
Co-authored-by: Gabriel Bosio <[email protected]>
Co-authored-by: Gabriel Bosio <[email protected]>
The euclidean algorithm was being built for every inverse gate in a circuit. The aim of this PR is to improve compilation times in circuit related contracts by reducing the MLIR size and this is done by declaring the euclidean algorithm only once and the just calling it.
Benchmark
The following are the compilation times it took with the
no_inlineattribute and with an O2 optimization level. TheBeforecolumn contains the compilation times for the main branch (ref -> without the optimization done in #1348), while theAftercolumn contains the compilations times for this branch (ref -> again, without the optimization done in #1348 ). These were ran on a MacBook Air M1 with 16 GB.0x1b5fbe104c033025dbb7fb37011781cc9344e881b4828cdaa023a80fecafde40x3100defca27214e5f78f25e48a5b05e45899c6834cb4d34f48384c18e14dff70x79666cdb4fc3cbcafbd74f4ea4e2855bf455c5a7c70915f5679325c540327710xa40ecaa08eaef629d433b15f6b017461df8bea9b4e7299eb8ba3632e32b5a50x4ffeed293927cd56686a9038a10026a2d3b9602f789d1f163c1c4ac9a822a820x2269858a40ea0535cb373b0c981c91b907466edf6d65bcaf669760bbee0ae4d0x5ff378cb2f16804539ecb92e84f273aafbab57d450530e9fe8e87771705a6730x4edde37ca59d9dff8f4ac8945b1c4860b606abd61d74727904ad7494fccdfa9Correctness
Blocks 1560000 to 1580000 were executed and no diffs were found when comparing with the VM
Checklist