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

CUDA acceleration for trace and constraint commitments #1617

Open
wants to merge 3 commits into
base: next
Choose a base branch
from

Conversation

gswirski
Copy link

@gswirski gswirski commented Jan 9, 2025

Integrate CUDA hardware acceleration from miden-gpu. Supersedes #1474 and #1589

Blocked on the release of https://github.com/0xPolygonMiden/miden-gpu/pull/41. Make sure to update Cargo.toml

pollster = { version = "0.4", optional = true }

[target.'cfg(target_arch = "x86_64")'.dependencies]
miden-gpu = { path = "../../miden-gpu", optional = true }
Copy link
Author

Choose a reason for hiding this comment

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

This and line 33 need to be changed to a publicly released version.

ProvingOptions::RECURSIVE_96_BITS.with_partitions(3, 8),
ProvingOptions::RECURSIVE_128_BITS.with_partitions(3, 8),
ProvingOptions::RECURSIVE_96_BITS.with_partitions(4, 8),
ProvingOptions::RECURSIVE_128_BITS.with_partitions(4, 8),
Copy link
Author

Choose a reason for hiding this comment

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

Please double check if these make sense to you.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes - these make sense. I might've even removed the options with 3 partitions, but I guess no harm for keeping them.

Copy link
Author

Choose a reason for hiding this comment

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

I removed the option with 3 partitions.

My intuition is that it's better to add it if somebody needs it (and can ensure that it works), rather than have an option that may or may not be used and therefore may or may not work in a future release.

Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you! I left some comments inline - they are all pretty minor. Once the next version of miden-gpu is released, we'll be able to merge this PR.

ProvingOptions::RECURSIVE_96_BITS.with_partitions(3, 8),
ProvingOptions::RECURSIVE_128_BITS.with_partitions(3, 8),
ProvingOptions::RECURSIVE_96_BITS.with_partitions(4, 8),
ProvingOptions::RECURSIVE_128_BITS.with_partitions(4, 8),
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes - these make sense. I might've even removed the options with 3 partitions, but I guess no harm for keeping them.

prover/src/gpu/metal/mod.rs Outdated Show resolved Hide resolved
prover/Cargo.toml Outdated Show resolved Hide resolved
prover/src/lib.rs Outdated Show resolved Hide resolved
/// multiple devices. The number of partitions should be equal to the number of devices.
pub const fn with_partitions(mut self, num_partitions: usize) -> Self {
let hash_rate = match self.hash_fn {
HashFunction::Blake3_192 => 6,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could be $8$ as well. For this variant we just truncate the returned hash, but the function can still absorb up to 64 bytes in a single permutation.

Copy link
Author

Choose a reason for hiding this comment

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

Changed to 8. The match is not really necessary but I kept it so that if HashFunctions enum ever changes, this code will be revisited.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants