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

logup multiplicity in witness assignment #198

Merged
merged 4 commits into from
Sep 10, 2024
Merged

Conversation

hero78119
Copy link
Collaborator

@hero78119 hero78119 commented Sep 9, 2024

a lock-free, thread-safe structure to count lookup multiplicicity for opcode witness assignment

The lock-free, thread-safe structure is to support streaming witness assignment from emulator. Emulator emit a steptrace and insert to channel, while multi-thread are fetch step from channel and do the instance assighment concurrently while still updating the same logup multiplicity in some shared matters

@hero78119 hero78119 force-pushed the feat/lk_witness branch 3 times, most recently from 94527e6 to 1c777b5 Compare September 9, 2024 09:18
@@ -1,5 +1,6 @@
#![feature(box_patterns)]
#![feature(stmt_expr_attributes)]
#![feature(variant_count)]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is for mem::variant_count::()] to count entries of ROMType

@kunxian-xia
Copy link
Collaborator

Can we make it to support byte logical operations besides range check?

@hero78119 hero78119 requested a review from chaosma September 9, 2024 09:34
@hero78119
Copy link
Collaborator Author

hero78119 commented Sep 9, 2024

Can we make it to support byte logical operations besides range check?

Yes it can, and I just push a new commit as a demo
7ddc98e

The scope of lk_multiplicitty is just keep Table[ROMType::Ltu][Key] = Count, and how to interpret Key & Count is outside of the scope, which I believe will be in Table circuit assign function. And the Key will be split back to byte x byte and respective is_lt := byte < byte will be retrieved at that moment and generate final table with challenge.

ceno_zkvm/src/witness.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@chaosma chaosma left a comment

Choose a reason for hiding this comment

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

LGTM

ceno_zkvm/src/witness.rs Outdated Show resolved Hide resolved
ceno_zkvm/src/instructions.rs Show resolved Hide resolved
@kunxian-xia kunxian-xia merged commit 59b8743 into master Sep 10, 2024
4 checks passed
@kunxian-xia kunxian-xia deleted the feat/lk_witness branch September 10, 2024 09:23
kunxian-xia pushed a commit that referenced this pull request Sep 10, 2024
To address comment
#198 (comment) For
further reduce the Arc clone overhead.

I think in the future we might redesign `assign_instances` to support
streaming assign in the future, such that StepRecord will emit as
streaming from emulator and multi-threads as worker to collect batch
data and do assignment.
We can explore further usage when tuning performance
hero78119 added a commit that referenced this pull request Sep 30, 2024
a lock-free, thread-safe structure to count lookup multiplicicity for
opcode witness assignment

The lock-free, thread-safe structure is to support streaming witness
assignment from emulator. Emulator emit a steptrace and insert to
channel, while multi-thread are fetch step from channel and do the
instance assighment concurrently while still updating the same logup
multiplicity in some shared matters
hero78119 added a commit that referenced this pull request Sep 30, 2024
To address comment
#198 (comment) For
further reduce the Arc clone overhead.

I think in the future we might redesign `assign_instances` to support
streaming assign in the future, such that StepRecord will emit as
streaming from emulator and multi-threads as worker to collect batch
data and do assignment.
We can explore further usage when tuning performance
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants