Skip to content
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

Status of the riscv32im-risc0-zkvm-elf target #135376

Open
Noratrieb opened this issue Jan 11, 2025 · 10 comments
Open

Status of the riscv32im-risc0-zkvm-elf target #135376

Noratrieb opened this issue Jan 11, 2025 · 10 comments
Labels
O-risc0 Operating system: RISC0 zkVM https://risczero.com/ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Noratrieb
Copy link
Member

We currently have a target called riscv32im-risc0-zkvm-elf, which targets the RISC Zero Zero Knowledge VM. This target is maintained by @SchmErik, @jbruestle, @flaub #134721 has revealed some issues in this target.

The biggest issue is that the target sets target_os = "zkvm". A "Zero Knowledge VM" is a generic concept and not an operating system. The correct target_os would be risc0.

But there is another question, whether this target should exist in the first place. The alternative to this target would be using the existing riscv32im-unknown-none-elf target with a RISC0-provided crate for the system calls.

The thing that is currently gained from having this target is that it can have std, where the very few syscalls that exist are used to implement some standard library interfaces.
Concretely, the following functionality is provided:

  • program arguments
  • environment variables (read-only)
  • stdio
  • the system global allocator
  • and of course HashMap
  • (no_threads) std::sync

other features like std::fs, std::io, std::process, std::thread, std::time, std::net are not supported.

@SchmErik, who is a maintainer of the target, highlights how the std support is useful in #134721 (comment):

Having std support is important to us because it allows developers to use crates outside of no_std. This has enabled many others to use our target much more easily with existing crates

Additionally, they mentioned how having the target allows them to add cfgs to forked ecosystem crates to make them use more of RISC Zero's APIs (though this could also be implemented without a target and a normal custom cfg).

It is always unsatisfactory to have targets with such incomplete standard libraries (at least it's not as bad as wasm32-unknown-unknown in this case). On the other hand, (just like the wasm target) the APIs that are implemented are likely useful.

This issue is about figuring out what to do with this target, whether to keep it (renamed) or remove it alltogether.

@Noratrieb Noratrieb added I-compiler-nominated Nominated for discussion during a compiler team meeting. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 11, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 11, 2025
@Noratrieb Noratrieb added O-risc0 Operating system: RISC0 zkVM https://risczero.com/ and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jan 11, 2025
@workingjubilee
Copy link
Member

workingjubilee commented Jan 11, 2025

As far as target APIs we would consider as a minimum basis, notably, in the recent discussion on a prospective VexOS target, having at least a rudimentary form of std::time was considered important.

@SchmErik In general, people use Rust because "if it compiles, it works". However, we have already encountered this problem with wasm32-unknown-unknown: by having an almost-entirely-stubbed-out stdlib implementation, we create a situation where if it compiles, it DOESN'T work.

And it seems to me many people will discover their code doesn't work at significant expense! That is, because this is a target designed primarily for computation on expensive-to-operate distributed systems, even the cheapest executions can be significantly more costly than simply running the compiler and executing a built program on my local machine. Time, if nothing else.

The model for the wasm32-unknown-unknown target is a quagmire that is due to a unique situation for wasm32 and does not seem to apply in your case. It exists because there was an unwillingness to commit to e.g. having both a wasm32-wasi and a wasm32-web target... and also, wasm32-wasi didn't even exist yet. A lot was in-flux and still being designed at the time.

Regarding the target_os matter: I think you have somewhat misunderstood how to use the compiler's configuration features if you think a target_os or even target_vendor is required, honestly.

@Urgau
Copy link
Member

Urgau commented Jan 11, 2025

We (T-compiler) should probably always involve T-libs when a target can only have "incomplete standard libraries" before accepting/merging a new target (with/without std).

As @workingjubilee already mentioned, T-libs was in the loop regarding the recent VexOS target, and even-through that target is only missing std::process, std::thread and std::net it was deemed insufficient for Tier >= 2 (the target was allowed for Tier 3).

We discussed this in the libs meeting today. While this is indeed missing a lot of what would normally be present in a std target, it does at least have the basics down like support for Instant (which is one of the biggest issues with wasm32-unknown-unknown). We're happy for this to be accepted at tier 3 but would likely reject it for higher tiers.

Based on that I would be in favour of removing std support and fixing target_os, which if I understand correctly would be equivalent to riscv32im-unknown-none-elf, therefore rendering the target irrelevant (and so should just be removed?).

@Noratrieb
Copy link
Member Author

Noratrieb commented Jan 11, 2025

Removing std would imply removing the target alltogether, as std is the only point of the target.

@Noratrieb
Copy link
Member Author

I'm also nominating for libs discussion about whether libs wants to have a target with std like this.

@Noratrieb Noratrieb added the I-libs-nominated Nominated for discussion during a libs team meeting. label Jan 14, 2025
@Amanieu
Copy link
Member

Amanieu commented Jan 15, 2025

From the target tier policy document:

Tier 3:

std for targets with an operating system or equivalent layer of system-provided functionality), but may leave some code unimplemented (either unavailable or stubbed out as appropriate), whether because the target makes it impossible to implement or challenging to implement.

Tier 2:

Tier 2 targets must not leave any significant portions of core or the standard library unimplemented or stubbed out, unless they cannot possibly be supported on the target.

The libs team is happy to have as a tier 3 target but would block it from any higher tier.

@Amanieu Amanieu removed the I-libs-nominated Nominated for discussion during a libs team meeting. label Jan 15, 2025
@apiraino
Copy link
Contributor

Discussed in T-compiler triage.

So, we're fine with a stubbed std implementation. The question is rather about the target_os, changing that would break the target (comment).

One solution could be to giving them large time (say, 6 months) to fix the target_os and then swtich the compile target (comment).

@rustbot label -I-compiler-nominated

@rustbot rustbot removed the I-compiler-nominated Nominated for discussion during a compiler team meeting. label Jan 16, 2025
@Noratrieb
Copy link
Member Author

Okay. @SchmErik, @jbruestle, @flaub as the maintainers of the target, we ask you to transition the target to change the target_os (in the tuple and for the cfg) to be something properly reflecting that this is a RISC Zero target (risc0 or something would work) (you can keep or change the vendor). We understand that this may not be possible to do immediately, and we can certainly give you some time, but we want you to persue this.
As mentioned above, the target, including its std implementation, will stay.

@flaub
Copy link
Contributor

flaub commented Jan 16, 2025

Regarding the target_os matter: I think you have somewhat misunderstood how to use the compiler's configuration features if you think a target_os or even target_vendor is required, honestly.

Is there anything more you can say about this? What would you recommend is a better approach?

@Noratrieb
Copy link
Member Author

Projects could set something like --cfg risc0 in their rustflags as custom cfgs that could be picked up the crates. But given that the target also has a standard library, doing cfgs on target_os is fine.

@SchmErik
Copy link
Contributor

Okay. @SchmErik, @jbruestle, @flaub as the maintainers of the target, we ask you to transition the target to change the target_os (in the tuple and for the cfg) to be something properly reflecting that this is a RISC Zero target (risc0 or something would work) (you can keep or change the vendor). We understand that this may not be possible to do immediately, and we can certainly give you some time, but we want you to persue this. As mentioned above, the target, including its std implementation, will stay.

Thanks for the info! Yes, I think this sounds good. I'll chat with @flaub and we'll be in touch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-risc0 Operating system: RISC0 zkVM https://risczero.com/ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants