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

[WIP] Block machine: only use JIT on Goldilocks #2541

Draft
wants to merge 13 commits into
base: inline-witness-assignments
Choose a base branch
from

Conversation

georgwiese
Copy link
Collaborator

@georgwiese georgwiese commented Mar 13, 2025

Builds on #2553

Like #2540, but falls back to run-time witgen on fields other than Goldilocks.

@georgwiese georgwiese force-pushed the block-only-jit2 branch 2 times, most recently from 1196ce3 to 5b9575c Compare March 13, 2025 20:37
github-merge-queue bot pushed a commit that referenced this pull request Mar 13, 2025
Extracted from #2541

Fixes a bug that the machine parts doesn't include the intermediate
definitions that only appear in one of the receives. An example of this
is the [Arith
large](https://github.com/powdr-labs/powdr/blob/main/std/machines/large_field/arith.asm)
machine, which has intermediates like `x1c` in their list of arguments,
but doesn't refer to the in any constraint.

---------

Co-authored-by: chriseth <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Mar 14, 2025
Extracted from #2541

This makes JIT witgen work for the sqrt example.
@georgwiese georgwiese changed the base branch from main to use-compute-from March 14, 2025 14:37
@georgwiese georgwiese force-pushed the block-only-jit2 branch 5 times, most recently from 711c83f to 91d870e Compare March 18, 2025 14:49
@georgwiese georgwiese changed the base branch from use-compute-from to block-only-jit-base March 18, 2025 14:49
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: 91d870e Previous: b9ff8cf Ratio
jit-witgen-benchmark/jit_witgen_benchmark 40470810431 ns/iter (± 146383195) 30505971932 ns/iter (± 143565764) 1.33

This comment was automatically generated by workflow using github-action-benchmark.

github-merge-queue bot pushed a commit that referenced this pull request Mar 18, 2025
This PR changes the usage of `std::prover::provide_value` in
`Keccakf32Memory` to `std::prover::compute_from`, in the hope of being
able to use the JIT for this machine in #2541.

~Witgen time goes of `test_data/std/keccakf32_memory_test.asm` from 60s
to 76s, probably because too many things are being evaluated.~
This is fixed by 1c15647.
@georgwiese georgwiese changed the base branch from block-only-jit-base to inline-witness-assignments March 18, 2025 19:24
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.

1 participant