test(DON'T MERGE): develop-v2.x.0 - #3139
Draft
shuklaayush wants to merge 224 commits into
Draft
Conversation
makes the basic framework for using rvr extensions in openvm - adds an rvr feature flag - defines `RvrExtensionCtx` struct to provide mappings between opcode/executor/air indices - defines `VmRvrExtension` trait that extensions can implement to be registered - updated macro so that rvr `ExtensionRegistry` can be auto-generated in `SdkVmConfig` closes INT-7474, INT-7475, INT-7479
#2730) Moves the rvr files related to compiling and execution into openvm-circuit. Those rvr files previously depended on openvm-circuit and in order to enable rvr execution through the openvm pipeline, they had to be made a part of openvm-circuit to prevent circular dependencies. closes INT-7537
- Vm execution instance is made to use rvr execution, depending on the feature. Helper functions to convert between the existing `VmState` and the rvr state are also added. - The `VmConfig` macro now has a `create_rvr_extensions` method implementation, but instead of defining a new `VmRvrConfig` trait, the `create_rvr_extensions` method piggybacks on the existing `VmExecutionConfig`. This is to avoid complex feature-gated trait bounds. closes INT-6810, INT-7476
Enables running benchmarks through rvr extension. Benchmark tests do not check execution correctness and currently execution involving extensions other than RV32IM fail because `VmRvrExtension` trait implementation is not properly wired. closes INT-7480
Previously rvr execution had to use `executor_idx_to_air_idx` information in order to construct `ExtensionRegistry`. This was a problem for pure execution which didn't need air indices so the interface diverged between rvr and aot/interpreted. Now for rvr pure execution, dummy index values of `NO_CHIP` are used instead to keep the interface consistent. towards INT-7611
Removes the rvr tests and instead adds rvr comparison steps in existing openvm tests in a similar way to aot. Unlike aot, metered cost execution is also run and compared for rvr and interpreted modes. closes INT-7627
- Introduces a new `Rv32IoExtension` in rvr that handles the rv32io instructions (hint_storew, hint_buffer, reveal). This is mainly to have a struct managing the hint_store chip index. - Adds rvr tests to the CI file in the same way as aot. closes INT-7466
Implements the rvr feature for the keccak256 extension and also adds rvr tests to CI. Now extensions don't take a `staticlib_path` argument manually and instead uses the auto-built staticlib made by a build.rs file. closes INT-7468
Implements the rvr feature for the Algebra extension. The rvr side of the Algebra extension is now also split into `ModularRvrExtension` and `Fp2RvrExtension`. A notable change is to have the C code for the Algebra extension which uses `libsecp256k1` to also unconditionally contain the C code needed in the ECC extension, since they are closely related and doing so would avoid configuration dependencies. closes INT-7470, INT-7704
Implements the rvr feature for all extensions that are left - BigInt, Sha2, ECC, Pairing, Deferral. Code for tests and CI are also updated. Changes for the Deferral extension includes additions to the VM state used in rvr execution. closes INT-7465
closes INT-7821
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Changed rvr execution to use the existing openvm `VmState` instead of defining a new state struct and copying data between the two forms. The references and pointers to the fields in `VmState` are passed to the rvr execution functions so they can be used in C code. - The Deferral extension now uses a callback registration system to expose the Deferral related data to C code instead of piggybacking on the same mechanism of `OpenVmHostCallbacks`. This is enabled for each extension so that they can have per-extension data and state that is maintained separately. closes INT-7572
- Makes rvr metered execution use the existing `SegmentationCtx` of openvm instead of its own new structs and code. This fixes the discrepancy between rvr and interpreted/aot segmentation logic and resolves the issue of rvr making too many segments. (https://github.com/axiom-crypto/openvm-eth/actions/runs/26112543464) - Fixes the calculation of `num_insns` in segments that are used as segment boundaries. The instruction counts were recorded as multiples of `segment_check_insns` (1000) that didn't map to the actual basic block boundaries. Addresses the problem of overflowing GPU memory. (https://github.com/axiom-crypto/openvm-eth/actions/runs/26127167098) closes INT-7835
moves the rvr compilation stage for metered and metered cost out of execution and into instance construction closes INT-7626
closes INT-7648
Air indices are now represented as an enum in rvr code. The `AirIndex` enum has `Uninitialized` and `NoChip` variants that replace the previous `NO_CHIP = u32::MAX`. `AirIndex::Uninitialized` is only used in pure execution where air indices don't matter and causes a panic in rvr metered and metered cost execution. closes INT-7611
…#2807) - **RVR metered execution can now suspend at segment boundaries.** Previously only the interpreter and AOT backends supported segment-by-segment metered runs; RVR ran metered execution straight to termination. This branch adds a parallel `RvrMeteredSegmentInstance` (`RvrMeteredInstanceWith<F, SegmentBoundary>`) whose `execute_metered_until_segment_boundary` returns after the metered segmentation callback creates a segment, mirroring the suspend/resume shape the other two backends already expose. The tracer countdown is carried across calls by checkpointing `tracer.check_counter` into `segmentation_ctx.instrets_until_check` on suspend and restoring it on entry; both values are `try_from`-validated against u32 at the entry point (new `ExecuteError::InvalidMeteredContext`) and the hot C-callback's matching cast is guarded by `debug_assert_eq!`. Mid-segment suspension is out of scope: `initialize_segment_memory` resets the per-segment page-indices checkpoint buffer assuming the page buffers have already been flushed at a segment boundary. - **Generated-C surface reorganized by policy.** Block-begin and suspender helpers move into `c/block/{instret,metered,metered_segment}.h` and `c/suspender/{none,instret_limit,segment_boundary}.h`; tracer headers move under `c/tracer/`. A new `SuspendPolicy` enum drives which pair is included, with `compile_impl` rejecting incoherent combinations (`Metered` × `InstretLimit`, `Pure|MeteredCost` × `SegmentBoundary`) at compile time. Compile-time selection without preprocessor directives in the generated C, per the AGENTS.md guidance. - **`MeteredCtx` round-trip via `MeteredCtxParts`.** `SegmentationState` now carries the full `MemoryCtx` and `suspend_on_segment` flag, so a suspended metered run can be converted back to a `MeteredCtx` (`into_metered_ctx`) and resumed without losing page-tracking or segmentation state. A new test exercises the field-by-field round-trip. - **All RVR codegen inputs embedded at compile time.** Removes every `CARGO_MANIFEST_DIR` runtime dependency from the RVR project-emit pipeline so binaries (Docker images, etc.) no longer need the source tree to invoke `compile_impl`. Core C files (`openvm_io.{c,h}`, `rvr_ext_wrappers.c`) switch from `fs::copy` to `fs::write(include_str!(…))`. Extension `.a` staticlibs migrate from `staticlib_path() -> &Path` to `staticlib_file() -> (&'static str, &'static [u8])` via `include_bytes!(env!("RVR_*_FFI_STATICLIB"))`, with a new `write_extension_staticlibs` helper writing them to the temp project for `make` to link. Modular's libsecp256k1 amalgamation include (~85 `.c`/`.h` files, with test/bench/ctime/valgrind files filtered out) is collected by `extensions/algebra/rvr/build.rs` into a generated `SECP256K1_C_FILES` const and returned via the new `RvrExtension::extra_c_include_files()` hook (for files written but not compiled as their own TUs); `extra_cflags` switches to relative `-Isecp256k1/src` / `-Isecp256k1` against the temp project root. Trait return types are tightened from `&str` to `&'static str`. - **Up-front toolchain detection.** `compile_impl` probes the C compiler, linker, and `make` in PATH before building and reports all missing tools at once via `RuntimeToolchainError`. Adds `RVR_MAKE` override, forwards `HOST_OS` to the Makefile (replacing its `uname -s` shell-out), and threads path context into `CompileError` I/O variants. - **Metrics consolidation.** The four near-identical `Instant::now() … counter!().absolute() … gauge!().set()` blocks across interpreter / AOT / RVR are replaced by a single `ExecutionMetricTimer` helper in `arch::execution_metrics` (guarded against div-by-zero on sub-microsecond runs). A complementary `CompilationTimer` (`arch::compilation_metrics`) wraps every `*_instance` constructor and emits a `compile_{pure,metered,metered_cost,metered_segment}_ms` gauge labeled by backend (`interpreter` / `aot` / `rvr`). - **E1/E2/E3 jargon dropped.** `execute_e1` span/metric names become `execute_pure`; `terminate_execute_e12_*` → `terminate_execute_*`; const generic `E1` → `PURE_EXECUTION`. Comment references to "(E1)/(E2)/(E3)" are removed in favor of "pure/metered/preflight". - **Metric names.** `execute_e1_insns` → `execute_pure_insns`, `execute_e1_insn_mi/s` → `execute_pure_insn_mi/s`. Dashboards or alerting keyed on the old names need to be updated. - **`RvrExtension` trait surface.** `extra_c_source_paths() -> Vec<PathBuf>` → `extra_c_sources() -> Vec<(&'static str, &'static str)>`; `staticlib_path/paths()` → `staticlib_file/files()` returning embedded bytes; new optional `extra_c_include_files()` for files written but not compiled as TUs. Existing impls need a one-time conversion to `include_str!` / `include_bytes!`. - **`ExecutorInventory` generic param renames** (`E1`/`E2`/`E3` → `CombinedE`/`NewE`/`TargetE`) are visible in error messages but compatible. - **`CompileError` shape.** `CProject(io::Error)` → `CProject { path, source }`; `Toolchain(String)` → `Toolchain(#[from] RuntimeToolchainError)`; new `ToolchainCommand { command, source }`. Callers matching on these variants need to update. - **Binary size.** Embedded `.a` staticlibs and libsecp256k1 sources grow the binary by roughly 5–30 MB depending on enabled extensions. resolves int-7917 --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Splits the `GenericSdk::execute_*` methods into `GenericSdk::compile_*` and `GenericSdk::execute_compiled_*` methods for pure, metered and metered cost. This is to be able to reuse the "compiled" instance which, for rvr, takes a long time to create. closes INT-7842
Three correctness fixes in the RVR backend so that its per-segment VM state is byte-identical to the Rust preflight executor's, plus a test-coverage fix that closes the gap that hid all three bugs from `check_rvr_equivalence`. equivalence check` - `check_rvr_equivalence` (and its AOT sibling) only walked AS=1 (registers) byte-by-byte, silently missing any divergence in RV32 main memory (AS=2), public values (AS=3), and deferral (AS=4). All three correctness fixes below live in AS=2 or AS=4 and would have surfaced on the first `air_test`-style run if the check had walked every address space. Extracted the closure into a `check_vm_state_eq(lhs, rhs) -> eyre::Result<()>` free function shared by both the RVR and AOT equivalence checks, replacing the AS=1-only loop with a slice-level diff over every `LinearMemory`. Short-circuits at the first mismatch and reports `(AS, byte offset, lhs, rhs)`. Microseconds on typical test VM configs. - `SegmentationState::on_periodic_check` was bumping `segmentation_ctx.instret` by a full `segment_check_insns` interval up-front, then incrementing `tracer.check_counter` by the same delta on the way out. The anchor and the countdown ended up ahead of the actual VM by exactly `remaining_counter`, so the next interval inherited an inconsistent baseline. In termination paths this could let the segmenter seal a non-terminal block as the final segment. The callback now: - computes the actual block-boundary instret directly: `prev_anchor + (segment_check_insns - remaining_counter)`, - writes that back as the new anchor, - resets `check_counter` to a full fresh interval rather than incrementing. This matches the Rust metered executor's behavior at the same point. Mod-builder evaluates `SymbolicExpr` inputs **modulo the configured prime**. For `SETUP_ADDSUB` / `SETUP_MULDIV` and their Fp2 counterparts, the compute formula resolves to `Input(0)`, which during setup is the modulus `p` itself — so the variable is `p % p = 0`. The VM writes 32 zero bytes (64 for Fp2) to `rd`. `rvr_ext_mod_setup` and `rvr_ext_fp2_setup` were copying `rs1`'s bytes (the modulus) to `rd`. Those bytes then leaked into the guest's stack as register-loaded values, propagating downstream as a memory divergence between RVR and preflight at later segment boundaries. The FFI now traces the `rs1`/`rs2` reads (still required for chip metering) but writes zero bytes to `rd`. The deferral CALL FFI in RVR only traced AS=4 access for metering and never updated the `(input_acc, output_acc)` accumulator bytes. The Rust preflight executor (`DeferralCallExecutor::execute_e12_impl`) hashes each `(old_acc, commit)` pair via poseidon2 and writes the new accumulator F's to DEFERRAL_AS. Every deferral CALL therefore left RVR's AS=4 a hash-round behind preflight, producing a memory divergence that cascaded through subsequent CALLs. Plumbed a `(*mut F, len_in_F_units)` alias of DEFERRAL_AS through `OpenVmIoState` (via a new `deferral_memory_ptr` helper in `bridge.rs` with a debug-mode alignment check on the `u8 → F` cast) and registered a `DeferralCompressFn` poseidon2 closure on the host side. `host_deferral_call_lookup` now hashes the accumulators and writes the new F bytes into AS=4 in F-element units that exactly match preflight's `vm_write::<F, BLOCK_SIZE>` layout. `F::from_u32` is bijective with the perm output for `MontyField31`, so the stored bytes are byte-identical to what the preflight executor writes. resolves int-7974
Memory read and write functions now have an optional `check_bounds` invocation before accessing the memory. `check_bounds` checks that the access lies within the VM's addressable memory region and aborts otherwise. The same is applied for `openvm_io.h` functions that work with the user IO address space in data memory. To turn off protected mode, add the `openvm-cli/unprotected` feature. Mirrors the interpreter's `check_bounds` and `panic_oob` functions, and `unprotected` Cargo feature. closes INT-7702 --------- Co-authored-by: Ayush Shukla <ayush@axiom.xyz> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…2820) A perf pass on RVR metered execution. The main change isolates the rare segment-check callback into a cold per-block helper so the hot block stays frameless. ## Cold per-block segment-check helper Hot metered RVR blocks were paying a stack-frame cost because the rare `on_check` callback could fire from the same C function. Generated asm showed every metered block — even single-instruction ones — getting an entry prologue and stack spills to preserve guest-register parameters across the possible callback. Hot blocks now only test `check_counter < block_insn_count` inline. On underflow they musttail-jump to a cold per-block `block_0xPC_checkpoint(...)` helper that runs the segment-check callback, suspends/exits if needed, or musttails back to the hot block with a refreshed counter. Same semantics; the hot path is frameless again. ## Cleanup landing in the same patch - **`uses_page_tracking()` IR predicate.** Only blocks that can touch memory emit AS_MEMORY page-tracking locals. ~59% of blocks in the reth benchmark didn't need them. Extension emitters default to `true`; `HintNonQrInstr` opts out, plain host-only phantoms (`HintInput`, `HintRandom`, `PrintStr`) don't trigger page locals. - **Metered block ABI hoist.** `check_counter` (`_cc`) and `trace_heights` (`_th`) are passed as block parameters, removing `state->tracer` loads from every block. Metered mode uses 8 hot guest registers instead of 10 to fit the new parameters. - **`CompileOptions::keep_artifacts`.** Retains the generated RVR C tempdir on success and logs the path. Useful for codegen / asm audits. - Per-width fast traced memory helpers and clang-format / formatting cleanups across the FFI C/Rust crates. results in a modest ~100ms (out of 1.7s) improvement in metered execution of [reth benchmark](https://github.com/axiom-crypto/openvm-eth/actions/runs/26514539244) on my laptop, the improvement is much more significant (~20%)
In `rvr`, some constants are redefined or set as variables. Resolved
some of the dependency issues (e.g. circular dependency) to import the
constants instead.
Related constants:
1) `WORD_SIZE`: imported from `openvm_platform::WORD_SIZE`
2) `AS_MEMORY`: imported from
`openvm_instructions::riscv::RV32_MEMORY_AS`
3) `AS_REGISTER`: imported from
`openvm_instructions::riscv::RV32_REGISTER_AS`
4) `AS_PUBLIC_VALUES`: imported from
`openvm_instructions::PUBLIC_VALUES_AS` (moved to `openvm_instructions`
from `openvm-circuit`. Is it right choice????)
5) `DEFERRAL_AS`: imported from `openvm_instructions::DEFERRAL_AS`
6) `MAX_BLOCK_INSNS` (`rvr-openvm-lift/src/cfg.rs`): was `let`, now
`const`
The following ones are kept redefined:
1) `CHUNK`, `DEFERRAL_DIGEST_SIZE`: logically are from
`openvm-stark-sdk`. `openvm-circuit` and `openvm-recursion-circuit`
already redefine CHUNK. (can not import from them due to circular
dependency).
2) `DEFAULT_PAGE_BITS`, `DEFAULT_SEGMENT_CHECK_INSNS`: logically are
from `openvm-circuit::arch::execution_mode::metered::{ctx,
segment_ctx}`. These are host-side metered-execution defaults. (can not
import from them due to circular dependency)
towards INT-7571
`MAX_MEM_PAGES_PER_INSN ` is a worst-case number of pages a single instruction can touch. The worst-case unique pages per instruction (`HINT_BUFFER`) is `MAX_HINT_BUFFER_WORDS * WORD_SIZE` bytes divided by page size. One page covers `CHUNK * 2^PAGE_BITS` bytes. So the formula is: `MAX_MEM_PAGES_PER_INSN = div_ceil(MAX_HINT_BUFFER_WORDS * WORD_SIZE, CHUNK * 2^PAGE_BITS) + 1` `+1` misalignment. closes INT-7462
Add save and load compiled artifacts feature in `rvr` mode. The feature consist of having the ability to save compilation artifacts on disk and load them into the sdk to execute (part 1). Reusing of the persisted artifacts whenever possible instead of recompiling based on some metadata (part 2) will be done in separate PR. This PR is related to the part 1. The following methods were added: 1) `Sdk::load_compiled_pure`, `Sdk::load_compiled_metered`, `Sdk::load_compiled_metered_cost` and related methods for loading pure, metered and metered cost `.so` files 2) `RvrPureInstance::save`, `RvrMeteredInstance::save`, `RvrMeteredCostInstance::save` and related methods for saving `.so` file towards INT-7843
Resolves INT-7460
Resolves INT-7481
resolves int-8075
## Summary Deletes the `benchmarks/execute/examples` directory from the `openvm-benchmarks-execute` crate: - `regex_execute.rs` (the profiling example) - `regex-elf` (a checked-in prebuilt ELF binary) The crate's `Cargo.toml` has no `[[example]]` entries and no code or CI workflow references these files, so the removal is self-contained — the package now exposes only its `execute` bench target (verified via `cargo metadata`). Note: `docs/crates/benchmarks.md` still mentions `cargo flamegraph --example regex_execute` in its profiling section; left untouched here since this PR is scoped to the directory deletion, but that section is now stale. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The `Rv64` prefix distinguished ported rv64 code from rv32 during the
rv32→rv64 port. With the port complete, the per-chip prefix is noise.
This PR drops it where redundant and keeps `Rv64` where it denotes the
ISA, mirroring main's `Rv32` naming. Pure rename, no behavioral changes:
- Dropped: `Rv64FooAir/Chip/Executor/Adapter/Cols/Record` →
`FooAir/...`; opcode classes (`Rv64LoadStoreOpcode` →
`LoadStoreOpcode`); constants (`RV64_REGISTER_NUM_LIMBS` →
`REGISTER_NUM_LIMBS`, `RV64_MEMORY_AS` → `MEMORY_AS`, ...); helpers and
test names (`rv64_bytes_to_u16_block` → `bytes_to_u16_block`, ...)
- Where a stripped name was taken by the width-generic core type, the
generic gained a `Core` stem: `FooExecutor<NUM_LIMBS, ...>` →
`FooCoreExecutor<...>`, concrete alias `Rv64FooExecutor` →
`FooExecutor`; CUDA-local `using Rv64FooCore` aliases are inlined at use
sites
- Kept `Rv64` (ISA designator, as main keeps `Rv32`):
`Rv64I`/`Rv64M`/`Rv64Io`/`Rv64Phantom`, `Rv64ImConfig` + builders/prover
exts, `Rv64{I,M,Io}TranspilerExtension`, extension VM configs with their
builders (`Rv64ModularConfig`, `Rv64WeierstrassConfig`,
`Rv64PairingConfig`, `Rv64DeferralConfig`, `Sha2Rv64Config`,
`Int256Rv64Config`, `Keccak256Rv64Config`, ...), sdk-config TOML keys
`rv64i`/`rv64m`, and the rvr C files (`rv64io_callbacks.*`, `rv64m.h`,
...)
Kept intentionally: the riscv64im-unknown-openvm-elf target triple,
official riscv-tests vectors (rv64ui-*), prebuilt ELF test data
(rv64im-*), ISA-spec instruction mnemonics (LOADW_RV64, ...), and RV64
ISA prose in comments and docs.
No VK/proof or `openvm.toml` schema changes; source-breaking only for
imports of renamed items. Verified: workspace check + clippy clean, CUDA
kernels compile, riscv-circuit/toolchain/transpiler/rvr test suites pass
incl. a full prove.
Closes INT-8829
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Resolves INT-8777 Separates public-value reveal from the RV64 load/store opcode family. Reveal now has its own execution, AIR, trace-generation, and CUDA paths, while load/store is limited to register and memory interactions. This keeps public-value handling isolated from RAM semantics and allows removal of the public-values address space to remain a separate follow-up change.
- Emit LLVM's post-optimization basic-block map and successor topology in managed guest builds. - Retain function entries and exact machine-CFG successors of computed branches; direct control flow remains reconstructed from assembly, while returns and tail calls use existing callsite/function-entry handling. - Serialize only deduplicated, decoded block-start PCs in `VmExe`, then consume them as additive validated hints during RVR CFG construction. - Keep transpiler output positional with ELF instruction slots, using output length as the single source of truth for consumed input. - Add concise CFG and native-build tracing for instruction, block, hint, split, and execution-kind visibility. For the Fibonacci fixture, LLVM successor topology adds 4,416 bytes to the input ELF (`61,744` to `66,160`, 7.15%) without changing `.text`. That section is discarded after extraction; the serialized `VmExe` hint contribution grows by 26 bytes (`95` to `121` bytes, from 40 function-entry hints to 53 exact hints). Run in an isolated Linux GPU-host worktree: - `cargo +nightly fmt --all -- --check` - `cargo nextest run --cargo-profile=fast -p openvm-transpiler` (10 passed) - strict Clippy for the affected build, transpiler, and CLI test targets - the previously failing `test_multi_target_transpile_default` integration test (passed) - real `openvm-1.94.1` guest build with LLVM 21.1.8; `llvm-readobj-21` confirmed feature bit 2 and successor records - exact same-program ELF and serialized-hint size comparison The full PR suite completed on `bac02fa2f8` with 69 passed, 1 skipped, and no failures or pending checks. A [branch-named 20-run Reth comparison](https://github.com/openvm-org/rvr-openvm/actions/runs/31086198329) resolved `feat/support-taking-hints-for-cfg-construction` to semantic head `204d8e596f`. Base and target used the identical target-built guest ELF. Exact filtering produced 2,442 valid hints but only one additional block split. Metered execution changed from 658.640 ms to 669.169 ms (+1.56%, interval +0.53% to +2.50%); host instructions, guest instructions, and segment count were unchanged. Resolves INT-9012
Restores public values as native U8 cells while keeping register and general memory U16 and deferral memory Field32. This preserves the byte-oriented public API and verifier input shape, with matching CPU, CUDA, RVR, Merkle, and REVEAL handling. Resolves INT-8076, INT-9017
Builds the initial memory image and the initial Merkle tree sparsely, so per-segment postflight cost scales with the pages a guest actually touches rather than with the configured address-space size.Observed per-segment initial-Merkle postflight time drops from ~221ms to ~2-22ms depending on how many pages a segment touches. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Perf bench: https://github.com/axiom-crypto/openvm-eth/actions/runs/31213205639 Closes INT-8149 as well. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
## Problem `test_deferrals_enabled_without_usage` fails in the SDK CUDA job (`--features cuda,root-prover,...`) with: ``` panicked at crates/vm/src/system/cuda/merkle_tree/mod.rs:630: subtree size exceeds the address space's configured leaf count ``` The panic is in the dummy app proof that root keygen runs (`compute_root_proof_heights` → `StarkProver::prove` → `transport_init_memory_to_device`), so it only reproduces with `root-prover` enabled. It was first seen on the stacked 2^32 branches, but it reproduces on `develop-v2.1.0` as well. ## Cause `compute_root_proof_heights` builds its dummy app config from `AppConfig::riscv64`, which carries **no** deferral extension, and then overwrites `app_vm_config.system.config` with the caller's system config. When the real app VM enables deferrals, that config sizes `DEFERRAL_AS` (`1 << 14` cells). From there the two halves of the VM disagree: - **AIRs and chips** come from `SdkVmConfig::to_inner()`, which re-runs `apply_optimizations()`. With no deferral extension present that zeroes `DEFERRAL_AS`, so the Merkle tree is configured for **0** leaves in that address space. - **The executor's memory image** comes from the outer config (`VirtualMachine::create_initial_state` → `config().as_ref()`), so it still allocates the full 64 KiB buffer. Before #3112 the GPU subtree size was taken from the circuit's config, so the oversized (all-zero) image was silently ignored. #3112 derives the dense-prefix size from the image's touched-page watermark instead, which is `≥ 1` for any non-empty buffer so the mismatch now trips the bound. ## Fix Re-run `apply_optimizations()` after the assignment at both copies of `compute_root_proof_heights` (`keygen/dummy.rs`, `prover/root.rs`) so the outer config matches the one the circuit is built from. This does not move the app VK or any trace heights: `create_airs()` already went through `to_inner()`, so only the executor-side allocation changes, and the address space it drops was all-zero. Also included: the assertion now names the address space and both leaf counts, since the old message did not say which address space diverged. ## Testing - `cargo nextest run --cargo-profile=fast --features cuda,root-prover` in `crates/sdk` — 13/13 pass (was: `test_deferrals_enabled_without_usage` panicking). - `cargo nextest run --cargo-profile=fast -p openvm-circuit --features cuda -E 'test(merkle)'` — 20/20 pass. ## Follow-up (not in this PR) `SdkVmConfig` has an unwritten invariant that `config.as_ref()` must equal `config.to_inner().system`; mutating `system.config` after construction silently breaks it and the failure surfaces far away, in the GPU Merkle build. Worth considering a constructor or a debug assertion that enforces it. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Provide the single-threaded guest critical-section implementation independently of the optional embedded heap allocator, and cover thin-LTO guest linking and execution with a regression test.
Resolves INT-8851
Adds support for 2^32 bytes memory address. Supersedes #2850. Benchmark: https://github.com/axiom-crypto/openvm-eth/actions/runs/31222063368 New design: https://github.com/axiom-crypto/openvm-eth/actions/runs/32282531103/job/96164388705 Bench after MEM_BITS change: https://github.com/axiom-crypto/openvm-eth/actions/runs/32427802054/job/96613241136 After fixing some codex audit findings: https://github.com/axiom-crypto/openvm-eth/actions/runs/32431879330 Closes INT-8080 --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Resolves INT-9224
Stacked on #3104 (base `feat/2-pow-32-memory-addresses-v2.1.0`); only the top two commits are new here. ## Summary Byte program counters now span the full 32-bit address space, matching the 2^32-byte memory port. A 32-bit byte pc does not fit in a BabyBear element, so every circuit-visible pc — trace columns, execution/program bus messages, connector public values, and the exe-commit preimage — instead carries the **pc index** `pc / DEFAULT_PC_STEP < 2^PC_BITS` (`PC_BITS = 30`). Runtime pcs stay byte addresses; conversion happens at the circuit boundary. ## Circuit representation (`crates/vm` + toolchain) - `openvm-instructions` gains `pc_to_idx` / `idx_to_pc` and `PC_STEP_BITS`; `MAX_ALLOWED_PC` becomes the last aligned 32-bit byte address (`u32::MAX - 3`). - Program cached trace, connector begin/end pcs, phantom rows, and `compute_exe_commit(pc_start)` all convert to indices. Segment stitching, the root-verifier AIR, and verify-stark pass the public-value felt through unchanged. - The `ExecutionBridge` default pc increment becomes `+1` index; all `(DEFAULT_PC_STEP, to_pc)` adapter tuples become `(1, to_pc)`. - AIRs that consume byte immediates (branches, JAL) scale them by `inverse(DEFAULT_PC_STEP)`. - Also fixes a latent metered pre-compute bug that double-added `pc_base`. ## Chips that materialize the byte pc The old `[u16, u14]` limb split cannot soundly represent 32-bit values mod p (BabyBear is a 31-bit field), so chips that write the byte pc into a register decompose it differently: - **JAL/JALR** return address `rd = 4 * (from_idx + 1)`: the low u16 limb is constrained as `4x` with `x < 2^PC_IDX_LOW_BITS` and the high limb as a full u16, which uniquely pins the 32-bit decomposition mod p. JAL `rd` is zero-extended per the bus address convention, while LUI keeps rv64 sign-extension. - **JALR** `to_pc_limbs` decompose the target *index*; a misaligned target (bit 1 set) is unsatisfiable in the AIR and rejected in `try_run_jalr`. - **AUIPC** decomposes the byte pc as `[4 * idx_low, u16 high]`; results ≥ 2^32 are rejected at tracegen. - **Branches** compute `to_pc = from_idx + byte_delta / 4`; fillers use signed integer target math (field addition wraps mod p past 2^30) with bounds validation, shared via `taken_branch_pc` / `checked_branch_target`. ## Misaligned branch/jump offsets The transpiler deliberately does **not** reject misaligned branch/jump offsets (second commit). It decodes every word of `.text`, so embedded data and never-taken branches can legitimately decode as misaligned control flow. Misalignment is instead enforced where the target is actually used: the interpreter traps on a misaligned pc, and tracegen rejects the row via `checked_branch_target` (branches) and the JAL target bounds check. A misaligned branch that is never taken falls through normally, matching hardware. ## CUDA parity `constants.h` gains pc helpers; program/phantom/adapter kernels write indices; the `jal_lui` / `jalr` / `auipc` kernels mirror the new limb splits and range-checker counts; rvr replay kernels validate jump targets with integer math (`replay_taken_branch_pc` / `replay_branch_target_in_bounds`); and `checkpoint_replay`'s `branch_target` drops its mod-p reduction. ## Tests Chip tests sample aligned byte pcs over the full 32-bit range, with new max-pc boundary tests for JAL/JALR/AUIPC. Testers record index-space bus messages while exposing byte pcs as `u32` (roundtripping through F would reduce mod p). The misaligned-offset fix restores `rvr_embedded_text_data`, `rvr_invalid_branch_fallthrough`, and `rvr_invalid_branch_taken`. Closes INT-8507 --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Ayush Shukla <ayush@axiom.xyz>
## Summary - document runtime program counters as aligned u32 byte addresses and circuit program counters as the single 30-bit index `pc_idx = byte_pc / DEFAULT_PC_STEP` - document full-u32 RV64 memory as guest byte addresses backed by u16 cells, with memory-bus addresses carrying one aligned block index - clarify PC and memory alignment, overflow, complete-access-span checks, executable commitments, continuation public values, and taken branch/jump behavior - document valid bit-32 RV64 register results for AUIPC and JAL/JALR link addresses at the top of the PC range - update only the VM, verifier, continuation, ISA, memory, transpiler, guest-programming, SDK, hint-store, and RV64 circuit documentation affected by the merged implementations The implementation changes are already present on `develop-v2.1.0` through #3104 and #3108; this PR now contains only the two documentation commits. ## Testing - `cd docs/vocs && pnpm install --frozen-lockfile && pnpm build` - `codespell` on all changed documentation files - `git diff --check origin/develop-v2.1.0...HEAD` Closes INT-9122 Closes INT-9123 Closes INT-8078 --------- Co-authored-by: Ayush Shukla <ayush@axiom.xyz>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
shuklaayush
added
run-benchmark
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
Note: cells_used metrics omitted because CUDA tracegen does not expose unpadded trace heights. Commit: 63791d0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.