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

Add mips-zkm-zkvm-elf triple #134721

Closed

Conversation

eigmax
Copy link

@eigmax eigmax commented Dec 24, 2024

We are trying to add a new target mips-zkm-zkvm-elf into rustc.

We have implemented a MIPS zkVM(zero knowledge virtual machine) to generate validity/ZK proof for general purpose computing. It allows the dev to compile the rust program into MIPS ELF, and the it proves the MIPS emulator/VM and generate a succinct proof to tell the verifier that the computing has been executed as expected.

This new target can significantly reduce the installation overhead for Rust dev.

@rustbot
Copy link
Collaborator

rustbot commented Dec 24, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jieyouxu (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot
Copy link
Collaborator

rustbot commented Dec 24, 2024

These commits modify compiler targets.
(See the Target Tier Policy.)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 24, 2024
@rust-log-analyzer

This comment has been minimized.

Comment on lines 25 to 26
// Some crates (*cough* crossbeam) assume you have 64 bit
// atomics if the target name is not in a hardcoded list.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, as for crossbeam, this issue was already resolved over a year ago. crossbeam-rs/crossbeam#1033

@jieyouxu

This comment was marked as off-topic.

@jieyouxu jieyouxu added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. needs-mcp This change is large enough that it needs a major change proposal before starting work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. needs-mcp This change is large enough that it needs a major change proposal before starting work. labels Dec 24, 2024
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Before this target can be accepted, this PR will need two things:

  1. Can you please also add platform support docs for this Tier 3 target?
  2. Please also confirm that the target conforms with the Tier 3 target policy, as described in Adding a new target.

@saethlin
Copy link
Member

EDIT: nevermind, I thought we didn't have any zKVM-related targets, but we apparently do.

It is not at all clear to me (as a compiler maintainer who just tried to use Google to learn about zkvm) that these two uses of the term "zkvm" are referring to the same thing. It would be rather bad if they are not, so I would like some step taken to prevent/clarify problems here.

@eigmax
Copy link
Author

eigmax commented Dec 24, 2024

@saethlin there is one already. https://doc.rust-lang.org/rustc/platform-support/riscv32im-risc0-zkvm-elf.html

@jieyouxu thanks! I will follow the instruction and add the docs.

@saethlin
Copy link
Member

saethlin commented Dec 24, 2024

there is one already

I have read that. It is not clear to me, based on the terminology and authorship of this PR that these two things called "zkvm" are affiliated with each other, or are referring to a generic concept that they both agree they do not own.

@eigmax
Copy link
Author

eigmax commented Dec 24, 2024

I have read that. It is not clear to me, based on the terminology and authorship of this PR that these two things called "zkvm" are affiliated with each other, or are referring to a generic concept that they both agree they do not own.

Good question. you are right, zkVM is a general concept that representing it can generate the validity proof (I would use this instead of zero knowledge proof for easier understanding) for some instruction set. For RISC0, it builds a zkVM for RV32, and for zkMIPS, we build a zkVM for MIPS32R2. More generally speaking, there are also some other impls, like zkWASM, or custom instruction set, like Miden.

For your concern, should dev be aware of different zkVMs? I think we should due to:

  1. It's obvious that different apps may be built for different architecture, like MIPS or RV, and the validity proof should constrain the specific function and logic on specific architecture.
  2. The original idea we build the zkMIPS is that MIPS is already used to build the Fraud Proof VM by some blockchain projects, like Optimism. And zkMIPS can help those projects to solve their long finality time issue.

@workingjubilee
Copy link
Member

workingjubilee commented Dec 25, 2024

@eigmax Does "zkvm" actually have a specific meaning with regards to these targets that they all uniformly share, or are they allowed to have entirely different ZK proof implementations? Do they share a syscall standard or some other abstract interface? Why does "zkvm" matter for the target's name?

To put it another way, if tomorrow Apple decided to make their entire Darwin OS run on a ZKVM, would we want to rename all the Apple targets to {arch}-apple-darwin-zkvm and so on, or would this fact be largely unimportant from the perspective of merely compiling software for the target?

@workingjubilee
Copy link
Member

workingjubilee commented Dec 25, 2024

cc @flaub @jbruestle @SchmErik Can you explain your perspective on the matter of using target_os = "zkvm" for two targets that don't seem to obviously share an OS interface?

Comment on lines 288 to 290
//@ revisions: mips_zkm_zkvm_elf
//@ [mips_unknown_linux_uclibc] compile-flags: --target mips-zkm-zkvm-elf
//@ [mips_unknown_linux_uclibc] needs-llvm-components: mips
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You copied the revision name, this is incorrect.

@qethu
Copy link

qethu commented Dec 25, 2024

@eigmax Does "zkvm" actually have a specific meaning with regards to these targets that they all uniformly share, or are they allowed to have entirely different ZK proof implementations? Do they share a syscall standard or some other abstract interface? Why does "zkvm" matter for the target's name?

To put it another way, if tomorrow Apple decided to make their entire Darwin OS run on a ZKVM, would we want to rename all the Apple targets to {arch}-apple-darwin-zkvm and so on, or would this fact be largely unimportant from the perspective of merely compiling software for the target?

I suspect that the OS section is useful here, is that the ZK may be some different with BareMetal embedded software developing.
Such as it can set main as the entrance of the ELF file.

@qethu
Copy link

qethu commented Dec 25, 2024

cc @flaub @jbruestle @SchmErik Can you explain your perspective on the matter of using target_os = "zkvm" for two targets that don't seem to obviously share an OS interface?

I guess that there may be a case that, a same app can be useful for both baremetal embedded hardware and zKVM emulator.
The 2 platform may be quite different, such as the code to initialize the hardware is not needed by zKVM;
and the entrance can be main (or other pure Rust code) on zKVM.

@workingjubilee
Copy link
Member

Would that mean cfg(target_os = "zkvm") would be useless in terms of selecting the correct behavior, because there would be no consistent handling between these two "OS"?

I do not think it is appropriate to introduce future possibilities about "what if we make fn main() work on this bare metal target?" right now. The more important future possibility is what if one of these targets does and the other one doesn't, for any given behavior?

@workingjubilee
Copy link
Member

In general, code compiled for "bare metal" targets does not necessarily have code to initialize hardware. Case in point: wasm32v1-none is also a bare metal target and also is a VM.

@eigmax
Copy link
Author

eigmax commented Dec 26, 2024

Would that mean cfg(target_os = "zkvm") would be useless in terms of selecting the correct behavior, because there would be no consistent handling between these two "OS"?

I do not think it is appropriate to introduce future possibilities about "what if we make fn main() work on this bare metal target?" right now. The more important future possibility is what if one of these targets does and the other one doesn't, for any given behavior?

The cfg is useful. When it runs on zkVM, the IO and some libraries it can use are different (like Math), it requires no_std basically. if it runs on non-zkVM(host mode), there is no limitation. When we generate a proof, we first run it on host mode to calculate its output, and then run it on guest mode with program's input and output via a special IO channel, where the output is to check the correctness of the result.

@workingjubilee
Copy link
Member

workingjubilee commented Dec 27, 2024

@eigmax

The cfg is useful. When it runs on zkVM, the IO and some libraries it can use are different (like Math), it requires no_std basically. if it runs on non-zkVM(host mode), there is no limitation. When we generate a proof, we first run it on host mode to calculate its output, and then run it on guest mode with program's input and output via a special IO channel, where the output is to check the correctness of the result.

Will the IO and libraries between mips-zkm-zkvm-elf be the same as the ones of riscv32im-risc0-zkvm-elf, or are they allowed to be different?

If I write a program that has an instance of #[cfg(target_os = "zkvm")] code, how much will it have to vary between those two targets?

An operating system is defined by having things in common for the target, for all the instances of that target OS string. While some architecture specific variation is to be expected, nothing you have said so far suggests that you are working together to make sure that your "OS" has even the faintest similarity to their "OS". "They both involve very fancy math" is not a functional similarity at the compiler level.

A target OS is not defined simply by having things different from bare metal. All bare metal binaries are allowed to be unique and beautiful snowflakes.

@eigmax
Copy link
Author

eigmax commented Dec 27, 2024

It's not same, we used different syscall(RV vs MIPS) to read and write data stream between the host and guest program, for the libraries, it's can be same, but not now. When saying a zkVM, it's basically an OS with a proof system. The proof systems are totally different from each others.

@saethlin
Copy link
Member

Just so we are all crystal clear, the existing zkvm target has these syscalls, which it uses yolo linkage to pick up:


And it helpfully documents

    // Wrappers around syscalls provided by risc0-zkvm-platform:

If you want to say you are target_os=zkvm, you are agreeing that these are your syscalls too. Which I am completely confident they are not, because not only did you just say they are not, the existing risc0 zkvm target says they are specific to the risc0 zkvm.

It is my opinion that this PR should be rejected, and I would prefer that we delete the existing risc0 zkvm target. Both of these zkvms are just bare-metal runtimes. The std implementation that the risc0 zkvm target has is incomplete and panicky with no path to being a complete std implementation. Just like the much-hated/regretted wasm32-unknown-unknown.

@workingjubilee
Copy link
Member

@eigmax So at this point the main question I think we have is: Why shouldn't we just have a MIPS bare metal target? I mean, we might need another such target. But then you can build programs for that, and supply the API calls needed via a library, and we can talk if your target grows complicated enough that it actually supplies half of the stdlib.

@eigmax
Copy link
Author

eigmax commented Dec 28, 2024

@workingjubilee yes, at least we need a mips target here, like mips-mti-none-elf.

I believe it's not the time to import the a new zkVM OS, since there is not standard ABI. I was confused by the merge of risc0's target.

@saethlin
Copy link
Member

I was confused by the merge of risc0's target.

I am too. We'll sort this all out and keep you informed. It will take some time.

@workingjubilee
Copy link
Member

It looks like we have a mipsel-unknown-none target but not a mips-unknown-none target? I assume you want a big endian bare metal MIPS target, right, @eigmax? Yeah, that sounds fine.

@eigmax
Copy link
Author

eigmax commented Dec 28, 2024

It looks like we have a mipsel-unknown-none target but not a mips-unknown-none target? I assume you want a big endian bare metal MIPS target, right, @eigmax? Yeah, that sounds fine.

Yes, that's the point. Thanks!

@saethlin saethlin added the I-compiler-nominated Nominated for discussion during a compiler team meeting. label Dec 29, 2024
@eigmax eigmax closed this Jan 1, 2025
@SchmErik
Copy link
Contributor

SchmErik commented Jan 8, 2025

cc @flaub @jbruestle @SchmErik Can you explain your perspective on the matter of using
target_os = "zkvm" for two targets that don't seem to obviously share an OS interface?

Hi @workingjubilee and @saethlin,

We were away for the holidays but now we are back. We’ve read the conversation on this thread and wanted to respond.

There have been zkVM’s before risc0’s but they have used a custom ISA. We are one of the first zkVMs to use an existing ISA and as we have gained more traction, many other implementations have popped up.

The whole intention of using target_os = “zkvm” is to be able to use this as cfg to modify the behavior of existing crates to use our accelerators. For instance, we have a sha256 accelerator in the risc0 zkvm that takes the cost of doing a SHA2 compression from several million cycles down to 73 cycles. In order to do this, we fork the sha2 crate and patch it using the cfg in order to call into our accelerators.

In hindsight, using target_os = "zkvm" was probably not the best option for the target triple because now it appears that we’re squatting on this target_os and that’s not fair or appropriate for other zkvm projects. Are there suggestions you have on making this better? Would using target_vendor be more appropriate here?

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. If our std implementation is imposing unnecessary hardship in maintenance, we’d love to work on solving these issues. We have in the past and will continue to address feedback when tagged on PR’s, report any issues we find, and to be good citizens of the ecosystem.

Thanks,
Erik

cc @flaub and @jbruestle

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

I have created #135376 to summarize the discussion on RISC0 and nominated that instead to get the discussion out of this closed PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants