Use generated layout to determine location of registers in rv32im circuit #476
Conversation
This file contains 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
Contributor
shkoo
commented
Mar 28, 2023
shkoo
commented
- Refactor layout to minimize duplicate code between risczero/rv32im
- Add a buffer pretty-printer to dump a buffer based on a layout
- rv32im-verify no longer has a brittle dependency on the specific layout of the "out" buffer
shkoo
force-pushed
the
nils/rv32im-layout
branch
2 times, most recently
from
2faa9bb
to
ed12a7d
Compare
…cuit * Refactor layout to minimize duplicate code between risczero/rv32im * Add a buffer pretty-printer to dump a buffer based on a layout * rv32im-verify no longer has a brittle dependency on the specific layout of the "out" buffer
shkoo
force-pushed
the
nils/rv32im-layout
branch
from
ed12a7d
to
5d18aa7
Compare
Benchmark for Linux-cuda b54243fClick to hide benchmark
Benchmark for Linux-default b54243fClick to hide benchmark
Benchmark for macOS-default b54243fClick to hide benchmark
Benchmark for macOS-metal b54243fClick to hide benchmark
|
|
I was planning to go in the direction of a serde codec that knows how to encode/decode the global structure. At that point, I think zirgen would generate a struct with serde attributes. |
flaub
approved these changes
Apr 5, 2023
shkoo
force-pushed
the
nils/rv32im-layout
branch
from
11254c2
to
780b0c6
Compare
Benchmark for Linux-cuda 41ad48fClick to hide benchmark
Benchmark for Linux-default 41ad48fClick to hide benchmark
Benchmark for macOS-default 41ad48fClick to hide benchmark
Benchmark for macOS-metal 41ad48fClick to hide benchmark
|
shkoo
force-pushed
the
nils/rv32im-layout
branch
from
780b0c6
to
f6136d4
Compare
Benchmark for Linux-cuda 19784deClick to hide benchmark
Benchmark for Linux-default 19784deClick to hide benchmark
Benchmark for macOS-default 19784deClick to hide benchmark
Benchmark for macOS-metal 19784deClick to hide benchmark
|
shkoo
force-pushed
the
nils/rv32im-layout
branch
from
f6136d4
to
59a4495
Compare
Benchmark for Linux-cuda ded4ab6Click to hide benchmark
Benchmark for Linux-default ded4ab6Click to hide benchmark
Benchmark for macOS-default ded4ab6Click to hide benchmark
Benchmark for macOS-metal ded4ab6Click to hide benchmark
|
flaub
reviewed
Apr 5, 2023
| @@ -695,7 +695,7 @@ fn pause_continue() { | |||
| #[cfg_attr(feature = "insecure_skip_seal", ignore)] | |||
| #[cfg_attr(feature = "cuda", serial)] | |||
| fn continuation() { | |||
| let segment_limit_po2 = 16; // 64k cycles | |||
| let segment_limit_po2 = 17; // 128k cycles | |||
There was a problem hiding this comment.
Why would layout changes require us to bump this?
There was a problem hiding this comment.
I suspect it was related to changes in risczero-wip that hadn't been propagated to risc0 (or vice versa), since it tested fine until I merged in main.
risczero-wip also needed some work to work with the new hash api. :(
I wonder if there's a way to keep risc0 and risczero-wip in sync automatically
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.