-
Notifications
You must be signed in to change notification settings - Fork 28
feat: Only use inlining in verifiers when compile for RISC-V #197
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
base: dev
Are you sure you want to change the base?
Conversation
|
|
||
| # Build something simple to check for errors | ||
| # cargo build --release -Z build-std=core,panic_abort,alloc -Z build-std-features=panic_immediate_abort --features unrolled_base_layer,security_80 --no-default-features | ||
| # cargo build --release -Z build-std=core,panic_abort,alloc --features unrolled_base_layer,security_80 --no-default-features |
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.
There is already panic=abort in the Cargo.toml, and panic_immediate_abort. I believe in the old version of cargo objcopy this flag was simply ignored, but now it triggers a compile error.
| recursion_log_23_layer.reduced.vk.json | ||
| recursion_in_unified_layer.bin | ||
| recursion_in_unrolled_layer.bin | ||
| # base_layer.bin |
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.
Not sure if I should leave these commented out (for the future? don't know) if to remove them.
| @@ -1,2 +1,2 @@ | |||
| [toolchain] | |||
| channel = "nightly" No newline at end of file | |||
| channel = "nightly-2026-01-19" | |||
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.
Reproducible build mentions that the version should be pinned & equal to this one, so I guess it must be pinned.
So I pinned to the one I use locally now.
| (CARGO_TARGET_DIR=target/seventeen cargo objcopy --release -Z build-std=core,panic_abort,alloc -Z build-std-features=panic_immediate_abort --features recursion_in_unified_layer,security_80 --no-default-features -- -O binary recursion_in_unified_layer.bin; | ||
| CARGO_TARGET_DIR=target/seventeen cargo objcopy --release -Z build-std=core,panic_abort,alloc -Z build-std-features=panic_immediate_abort --features recursion_in_unified_layer,security_80 --no-default-features -- -R .text recursion_in_unified_layer.elf; | ||
| CARGO_TARGET_DIR=target/seventeen cargo objcopy --release -Z build-std=core,panic_abort,alloc -Z build-std-features=panic_immediate_abort --features recursion_in_unified_layer,security_80 --no-default-features -- -O binary --only-section=.text recursion_in_unified_layer.text) & | ||
| (CARGO_TARGET_DIR=target/seventeen cargo objcopy --release -Z build-std=core,panic_abort,alloc --features recursion_in_unified_layer,security_80 --no-default-features -- -O binary recursion_in_unified_layer.bin; |
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.
Can you add another two builds to with security_100 feature, and name binaries like recursion_in_unrolled_layer_100_bits.* ?
| [toolchain] | ||
| channel = "nightly" | ||
|
|
||
| channel = "nightly-2026-01-19" |
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.
The outer one I'd prefer not to pin in general (we can quickly fix assert_matches imports, the rest should work)
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.
verifier crate has different toolchain pinning to manage, but this one is for workspace in full
Extra changes:
On native, we don't really care about performance of the verifier, but it's important when compiled to RISC-V.
This PR makes is so that on native we never inline field ops, but always do on RISC-V.
With this change, I am able to build a project which depends on both prover and
full_statement_verifierin 59 seconds; previously it was several minutes even withopt-level = 0on verifier crates (and tens of minutes if I try to just compile to release), so the impact is very noticeable.Verifier check via reproducible build script:
On my branch:
On dev:
I did not commit the recompiled verifiers.