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.

Sign up for GitHub

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

Jump to bottom

feat: PIL relations modifications for bc decomposition #11935

Merged
merged 10 commits into from
Feb 13, 2025
Merged

feat: PIL relations modifications for bc decomposition #11935

merged 10 commits into from
Feb 13, 2025

Conversation

jeanmon
Copy link
Contributor

@jeanmon jeanmon commented Feb 12, 2025 •
edited by fcarreiro
Loading 

----------------------------------------------------------------------
Benchmark                            Time             CPU   Iterations
----------------------------------------------------------------------
alu_acc_random                   0.126 us        0.126 us      5553823
bc_decomposition_acc_random       8.73 us         8.73 us        80583
bc_retrieval_acc_random          0.024 us        0.024 us     29187915
bitwise_acc_random                1.42 us         1.42 us       493026
ecc_acc_random                    2.59 us         2.59 us       269872
execution_acc_random             0.524 us        0.524 us      1339762
instr_fetching_acc_random        0.024 us        0.024 us     29288136
range_check_acc_random            2.67 us         2.67 us       262513
sha256_acc_random                 6.25 us         6.25 us       111991

@jeanmon jeanmon force-pushed the jm/avm2-byte-decomposition branch from e09cc53 to d8b65a7 Compare February 12, 2025 15:52
@jeanmon jeanmon marked this pull request as ready for review February 12, 2025 20:30
@jeanmon jeanmon force-pushed the jm/avm2-byte-decomposition branch 2 times, most recently from b4bc442 to 80737dc Compare February 13, 2025 10:11
fcarreiro
fcarreiro reviewed Feb 13, 2025
View reviewed changes
Copy link
Contributor

@fcarreiro fcarreiro left a comment

Choose a reason for hiding this comment

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

Looks good from a VM2 PoV. Once you finish and Ilyas approves, I'm curious if you can run the benchmarks again to see the cost after the extra constraining.

@jeanmon jeanmon force-pushed the jm/avm2-byte-decomposition branch from 80737dc to 21fb996 Compare February 13, 2025 14:06
@jeanmon jeanmon force-pushed the jm/avm2-byte-decomposition branch from 21fb996 to 1bc03a1 Compare February 13, 2025 15:10
@jeanmon
Copy link
Contributor Author

jeanmon commented Feb 13, 2025 •
edited by fcarreiro
Loading

Looks good from a VM2 PoV. Once you finish and Ilyas approves, I'm curious if you can run the benchmarks again to see the cost after the extra constraining.

@fcarreiro

----------------------------------------------------------------------
Benchmark                            Time             CPU   Iterations
----------------------------------------------------------------------
alu_acc_random                   0.126 us        0.126 us      5553823
bc_decomposition_acc_random       8.73 us         8.73 us        80583
bc_retrieval_acc_random          0.024 us        0.024 us     29187915
bitwise_acc_random                1.42 us         1.42 us       493026
ecc_acc_random                    2.59 us         2.59 us       269872
execution_acc_random             0.524 us        0.524 us      1339762
instr_fetching_acc_random        0.024 us        0.024 us     29288136
range_check_acc_random            2.67 us         2.67 us       262513
sha256_acc_random                 6.25 us         6.25 us       111991

@jeanmon jeanmon merged commit 6c93058 into master Feb 13, 2025
53 checks passed
@jeanmon jeanmon deleted the jm/avm2-byte-decomposition branch February 13, 2025 16:35
@AztecBot AztecBot mentioned this pull request Feb 13, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fcarreiro fcarreiro fcarreiro left review comments

@IlyasRidhuan IlyasRidhuan IlyasRidhuan approved these changes

@Maddiaa0 Maddiaa0 Awaiting requested review from Maddiaa0 Maddiaa0 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jeanmon @fcarreiro @IlyasRidhuan