feat: add `succinct-zkvm` target #138433

nhtyy · 2025-03-12T23:20:33Z

Adds a new riscv32im target with a new os (succinct-zkvm).

Previously, there was already a zkvm target which is in the process of being removed, due to this issue, which we think is the right call.

This PR DOES NOT attempt to resolve the aforementioned issue for the risc0 target.

rustbot · 2025-03-12T23:20:37Z

⚠️ Warning ⚠️

These commits modify submodules.

src/llvm-project

jieyouxu · 2025-03-12T23:34:33Z

src/bootstrap/Cargo.toml

+#
+# todo(succinct): remove this change that was explicity against the rules.
+# This just happens to work, and ideally cc-rs will fix the target parsing issue before our target is merged.
+cc = "=1.2.0"
+# cc = "=1.1.22"


Problem: you cannot bump bootstrap's cc here in this PR. cc > 1.1.22 still has problems with target parsing issue.

Left some comments above in the changes, it just so happened that I had forked that version of cc-rs and it worked. Happy to redo it with the correct version if its blocking on review

It's fine to experiment with a forked cc version if you need it for testing under PR CI or whatever, but we can't merge this into actual master until this bootstrap cc bump is reverted. As in, we unfortunately need to pin bootstrap cc version to 1.1.22. See #137460 where this was done deliberately.

EDIT: ah I see, it's not blocking for review, only blocking for approval/merge.

src/bootstrap/Cargo.lock

jieyouxu · 2025-03-12T23:38:18Z

I would recommend splitting this PR up into at least two PRs:

First PR adds an no_std minimal compiler target definition (stub, really) and roll a compiler lead reviewer via r? compiler_leads. This PR can also include the bootstrap bits.
Second PR adds the relevant std support and changes the target spec added by first PR to mark std support.

This is to make it easier to review because it's hard to review the mixed compiler/library changes in this one PR (compiler / library are maintained by different teams).

nhtyy · 2025-03-12T23:42:11Z

@jieyouxu TYSM for getting to this so quickly.

My plan here, is that given the volatile state of the zkvm (issue in description), I was expecting this to not be merged till that was fixed.

Do you think this OS should live in its own pal folder? I currently just shared with risc0 because there is alot of overlap. If thats the case I think creating a pal/succinct-zkvm will clean this PR up a lot and make reviewing much easier. Ideally, avoiding the need for two PRs.

Im actually just going to make this complete disjoint from the previous zkvm OS implementations. That should clean it up significantly.

jieyouxu · 2025-03-12T23:50:02Z

My plan here, is that given the volatile state of the zkvm (issue in description), I was expecting this to not be merged till that was fixed.

Ah. If this is another zkvm that's different enough than the other zkvm targets (in terms of the zkvm implementaiton), than IMO a separate target_os is definitely more faithful than just lumping all the different zkvm targets under one target_os = "zkvm". After all, what use is sharing a target_os if the underlying ZKVM "OSes" are in reality substantially different and incompatible with each other? Maybe even target_os = "succinct" (I don't know what would be a good target_os here for your target)?

And yeah, sorting out the current zkvm target situation would be ideal to avoid confusion and footguns in the long term.

Do you think this OS should live in its own pal folder? I currently just shared with risc0 because there is alot of overlap. If thats the case I think creating a pal/succinct-zkvm will clean this PR up a lot and make reviewing much easier. Ideally, avoiding the need for two PRs.

I can't really comment on this because I'm not a library maintainer/reviewer. This is partly why I recommended doing the library parts in a separate PR, so that you can discuss the library implementation decisions with a library reviewer.

nhtyy · 2025-03-13T00:27:24Z

Ok went ahead and moved everything to not rely on (mostly) any zkvm code.

The PR is definitely bigger, so I agree with your suggestion, Ill go ahead and split up these PRs soon.

rust-log-analyzer · 2025-03-13T00:34:05Z

The job mingw-check-tidy failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

info: removing rustup binaries
info: rustup is uninstalled
##[group]Image checksum input
mingw-check-tidy
# We use the ghcr base image because ghcr doesn't have a rate limit
# and the mingw-check-tidy job doesn't cache docker images in CI.
FROM ghcr.io/rust-lang/ubuntu:22.04

ARG DEBIAN_FRONTEND=noninteractive
RUN apt-get update && apt-get install -y --no-install-recommends \
  g++ \
  make \
---

COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
COPY host-x86_64/mingw-check/validate-error-codes.sh /scripts/

# NOTE: intentionally uses python2 for x.py so we can test it still works.
# validate-toolstate only runs in our CI, so it's ok for it to only support python3.
ENV SCRIPT TIDY_PRINT_DIFF=1 python2.7 ../x.py test \
           --stage 0 src/tools/tidy tidyselftest --extra-checks=py,cpp
#
# This file is autogenerated by pip-compile with Python 3.10
# by the following command:
#
#    pip-compile --allow-unsafe --generate-hashes reuse-requirements.in
---
#12 2.944 Building wheels for collected packages: reuse
#12 2.945   Building wheel for reuse (pyproject.toml): started
#12 3.159   Building wheel for reuse (pyproject.toml): finished with status 'done'
#12 3.160   Created wheel for reuse: filename=reuse-4.0.3-cp310-cp310-manylinux_2_35_x86_64.whl size=132720 sha256=0c2fd2aaf7b0bf8d6e131220aff14712a774c2ca462f3204d25460cbcf610b63
#12 3.160   Stored in directory: /tmp/pip-ephem-wheel-cache-6r1ie3zt/wheels/3d/8d/0a/e0fc6aba4494b28a967ab5eaf951c121d9c677958714e34532
#12 3.162 Successfully built reuse
#12 3.163 Installing collected packages: boolean-py, binaryornot, tomlkit, reuse, python-debian, markupsafe, license-expression, jinja2, chardet, attrs
#12 3.560 Successfully installed attrs-23.2.0 binaryornot-0.4.4 boolean-py-4.0 chardet-5.2.0 jinja2-3.1.4 license-expression-30.3.0 markupsafe-2.1.5 python-debian-0.1.49 reuse-4.0.3 tomlkit-0.13.0
#12 3.560 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
#12 4.098 Collecting virtualenv
#12 4.153   Downloading virtualenv-20.29.3-py3-none-any.whl (4.3 MB)
#12 4.306      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 4.3/4.3 MB 28.6 MB/s eta 0:00:00
#12 4.370 Collecting filelock<4,>=3.12.2
#12 4.380   Downloading filelock-3.17.0-py3-none-any.whl (16 kB)
#12 4.418 Collecting platformdirs<5,>=3.9.1
#12 4.426   Downloading platformdirs-4.3.6-py3-none-any.whl (18 kB)
#12 4.448 Collecting distlib<1,>=0.3.7
#12 4.456   Downloading distlib-0.3.9-py2.py3-none-any.whl (468 kB)
#12 4.469      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 469.0/469.0 KB 41.5 MB/s eta 0:00:00
#12 4.550 Installing collected packages: distlib, platformdirs, filelock, virtualenv
#12 4.731 Successfully installed distlib-0.3.9 filelock-3.17.0 platformdirs-4.3.6 virtualenv-20.29.3
#12 4.731 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
#12 DONE 4.8s

#13 [7/8] COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
#13 DONE 0.0s
---
extracting /checkout/obj/build/cache/2025-02-18/rust-std-beta-x86_64-unknown-linux-gnu.tar.xz
extracting /checkout/obj/build/cache/2025-02-18/rustc-beta-x86_64-unknown-linux-gnu.tar.xz
extracting /checkout/obj/build/cache/2025-02-18/cargo-beta-x86_64-unknown-linux-gnu.tar.xz
##[group]Building bootstrap
    Updating git repository `https://github.com/nhtyy/cc-rs`
    Updating crates.io index
---
   Compiling version_check v0.9.5
   Compiling generic-array v0.14.7
   Compiling libc v0.2.167
   Compiling shlex v1.3.0
   Compiling cc v1.2.0 (https://github.com/nhtyy/cc-rs?branch=n%2Fadd-succinct-target#424acb08)
   Compiling quote v1.0.37
   Compiling syn v2.0.87
   Compiling aho-corasick v1.1.3
   Compiling regex-syntax v0.8.5
   Compiling crossbeam-utils v0.8.20
---
DirectMap4k:      131008 kB
DirectMap2M:     7208960 kB
DirectMap1G:    11534336 kB
##[endgroup]
Executing TIDY_PRINT_DIFF=1 python2.7 ../x.py test            --stage 0 src/tools/tidy tidyselftest --extra-checks=py,cpp
+ TIDY_PRINT_DIFF=1 python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py,cpp
##[group]Building bootstrap
    Finished `dev` profile [unoptimized] target(s) in 0.05s
##[endgroup]
WARN: currently no CI rustc builds have rustc debug assertions enabled. Please either set `rust.debug-assertions` to `false` if you want to use download CI rustc or set `rust.download-rustc` to `false`.
downloading https://static.rust-lang.org/dist/2025-02-18/rustfmt-nightly-x86_64-unknown-linux-gnu.tar.xz
---
     // Prevent the usage of `Instant` in some cases:
     // - It's currently not supported for wasm targets without Emscripten nor WASI.
     // - It's currently not supported for zkvm targets.
-    let is_instant_unsupported =
-        (cfg!(target_family = "wasm") && cfg!(target_os = "unknown")) || cfg!(target_os = "zkvm") || cfg!(target_os = "succinct-zkvm");
+    let is_instant_unsupported = (cfg!(target_family = "wasm") && cfg!(target_os = "unknown"))
+        || cfg!(target_os = "zkvm")
+        || cfg!(target_os = "succinct-zkvm");
 
     let start_time = (!is_instant_unsupported).then(Instant::now);
     run_tests(opts, tests, |x| on_test_event(&x, &mut st, &mut *out))?;
Diff in /checkout/library/test/src/lib.rs:555:
 
     // Emscripten can catch panics but other wasm targets cannot
     let ignore_because_no_process_support = desc.should_panic != ShouldPanic::No
-        && (cfg!(target_family = "wasm") || cfg!(target_os = "zkvm") || cfg!(target_os = "succinct-zkvm"))
+        && (cfg!(target_family = "wasm")
+            || cfg!(target_os = "zkvm")
+            || cfg!(target_os = "succinct-zkvm"))
         && !cfg!(target_os = "emscripten");
 
     if force_ignore || desc.ignore || ignore_because_no_process_support {
Diff in /checkout/library/std/src/random.rs:48:
 /// VxWorks                | `randABytes` after waiting for `randSecure` to become ready
 /// WASI                   | [`random_get`](https://github.com/WebAssembly/WASI/blob/main/legacy/preview1/docs.md#-random_getbuf-pointeru8-buf_len-size---result-errno)
 /// ZKVM                   | `sys_rand`
-/// SUCCINCT-ZKVM          | `sys_rand` 
+/// SUCCINCT-ZKVM          | `sys_rand`
 ///
 /// Note that the sources used might change over time.
 ///
Diff in /checkout/library/std/src/sys/pal/succint-zkvm/abi.rs:1:
 //! ABI definitions for symbols exported by <_>-zkvm targets.
-//! 
+//!
 //! Ideally, these should be the minimum viable symbols to support the std-lib.
 
 #![allow(dead_code)]
Diff in /checkout/library/std/src/sys/random/zkvm.rs:25:
 pub fn fill_bytes(bytes: &mut [u8]) {
     unsafe { abi::sys_rand(bytes.as_mut_ptr(), bytes.len()) };
 }
+
Diff in /checkout/library/std/src/sys/alloc/mod.rs:7:
 // add fast paths for low alignment values.
 #[allow(dead_code)]
 const MIN_ALIGN: usize = if cfg!(any(
-    all(target_arch = "riscv32", any(target_os = "espidf", target_os = "zkvm", target_os = "succinct-zkvm")),
+    all(
+        target_arch = "riscv32",
+        any(target_os = "espidf", target_os = "zkvm", target_os = "succinct-zkvm")
+    ),
     all(target_arch = "xtensa", target_os = "espidf"),
 )) {
     // The allocator on the esp-idf and zkvm platforms guarantees 4 byte alignment.
Diff in /checkout/library/std/src/sys/random/succinct_zkvm.rs:3:
 pub fn fill_bytes(bytes: &mut [u8]) {
     unsafe { abi::sys_rand(bytes.as_mut_ptr(), bytes.len()) };
 }
+
Diff in /checkout/src/bootstrap/src/core/sanity.rs:35:
 const STAGE0_MISSING_TARGETS: &[&str] = &[
     // just a dummy comment so the list doesn't get onelined
     "wasm32-wali-linux-musl",
-    "riscv32im-succinct-zkvm-elf"
+    "riscv32im-succinct-zkvm-elf",
 ];
 
 /// Minimum version threshold for libstdc++ required when using prebuilt LLVM
fmt: checked 5905 files
Build completed unsuccessfully in 0:01:19

jhpratt · 2025-03-13T02:27:29Z

r? compiler

jieyouxu · 2025-03-13T02:47:10Z

r? compiler_leads

bors · 2025-03-13T14:48:02Z

☔ The latest upstream changes (presumably #138448) made this pull request unmergeable. Please resolve the merge conflicts.

nhtyy · 2025-03-13T18:02:14Z

Closing this in favor of separating the std library impl from adding target.

rustbot assigned jhpratt Mar 12, 2025

This comment was marked as resolved.

Sign in to view

nhtyy changed the title ~~feat: add Succinct target, impl stdlib functionality~~ feat: add succinct-zkvm target Mar 12, 2025

This comment has been minimized.

Sign in to view

jieyouxu requested changes Mar 12, 2025

View reviewed changes

feat: succinct target

5c1dbd3

nhtyy force-pushed the succinct-target branch from 67c45e3 to 5c1dbd3 Compare March 13, 2025 00:24

rustbot assigned Nadrieril and unassigned jhpratt Mar 13, 2025

rustbot assigned davidtwco and unassigned Nadrieril Mar 13, 2025

nhtyy closed this Mar 13, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add `succinct-zkvm` target #138433

feat: add `succinct-zkvm` target #138433

nhtyy commented Mar 12, 2025

This comment was marked as resolved.

rustbot commented Mar 12, 2025

This comment has been minimized.

jieyouxu Mar 12, 2025

nhtyy Mar 12, 2025

jieyouxu Mar 12, 2025 •

edited

Loading

jieyouxu Mar 12, 2025

jieyouxu commented Mar 12, 2025 •

edited

Loading

nhtyy commented Mar 12, 2025 •

edited

Loading

jieyouxu commented Mar 12, 2025 •

edited

Loading

nhtyy commented Mar 13, 2025

rust-log-analyzer commented Mar 13, 2025

jhpratt commented Mar 13, 2025

jieyouxu commented Mar 13, 2025

bors commented Mar 13, 2025

nhtyy commented Mar 13, 2025

feat: add succinct-zkvm target #138433

feat: add succinct-zkvm target #138433

Conversation

nhtyy commented Mar 12, 2025

This comment was marked as resolved.

rustbot commented Mar 12, 2025

This comment has been minimized.

jieyouxu Mar 12, 2025

Choose a reason for hiding this comment

nhtyy Mar 12, 2025

Choose a reason for hiding this comment

jieyouxu Mar 12, 2025 • edited Loading

Choose a reason for hiding this comment

jieyouxu Mar 12, 2025

Choose a reason for hiding this comment

jieyouxu commented Mar 12, 2025 • edited Loading

nhtyy commented Mar 12, 2025 • edited Loading

jieyouxu commented Mar 12, 2025 • edited Loading

nhtyy commented Mar 13, 2025

rust-log-analyzer commented Mar 13, 2025

jhpratt commented Mar 13, 2025

jieyouxu commented Mar 13, 2025

bors commented Mar 13, 2025

nhtyy commented Mar 13, 2025

feat: add `succinct-zkvm` target #138433

feat: add `succinct-zkvm` target #138433

jieyouxu Mar 12, 2025 •

edited

Loading

jieyouxu commented Mar 12, 2025 •

edited

Loading

nhtyy commented Mar 12, 2025 •

edited

Loading

jieyouxu commented Mar 12, 2025 •

edited

Loading