Skip to content

fix: Over-align public constants in moduli_init#2150

Merged
jonathanpwang merged 3 commits into
mainfrom
fix/ecc-align-constants
Sep 29, 2025
Merged

fix: Over-align public constants in moduli_init#2150
jonathanpwang merged 3 commits into
mainfrom
fix/ecc-align-constants

Conversation

@Avaneesh-axiom

@Avaneesh-axiom Avaneesh-axiom commented Sep 26, 2025

Copy link
Copy Markdown
Contributor

The two_modular_limbs_list constant declared by moduli_init! and used later in the complex setup macro was not guaranteed to be aligned to a large enough alignment. This caused an issue in some use-case.

This change aligns the public constant two_modular_limbs_list to the max block size of a modulus among all moduli declared to ensure no memory alignment issues arise.

Fixes INT-5135

@Avaneesh-axiom Avaneesh-axiom changed the title [fix] Over-align public constants in moduli_init fix: Over-align public constants in moduli_init Sep 26, 2025
@openvm-org openvm-org deleted a comment from github-actions Bot Sep 26, 2025
@github-actions

This comment has been minimized.

@Avaneesh-axiom Avaneesh-axiom marked this pull request as ready for review September 26, 2025 04:26

@jonathanpwang jonathanpwang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, but hold on merging.

limb_list_borders doesn't need alignment because (1) it's usize aligned, (2) we only access it's fields statically so it likely is compiled away already.

@jonathanpwang jonathanpwang requested a review from HrikB September 26, 2025 23:17
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@jonathanpwang jonathanpwang force-pushed the fix/ecc-align-constants branch from 1607d17 to 1ae4417 Compare September 29, 2025 03:45
@github-actions

Copy link
Copy Markdown
group app.proof_time_ms app.cycles app.cells_used leaf.proof_time_ms leaf.cycles leaf.cells_used
verify_fibair (+5 [+2.2%]) 228 322,610 2,058,654 - - -
fibonacci (-11 [-1.1%]) 1,000 1,500,210 2,107,962 - - -
regex (+72 [+3.0%]) 2,447 4,109,350 17,719,202 - - -
ecrecover (-26 [-3.6%]) 700 137,132 2,278,104 - - -
pairing (-14 [-1.0%]) 1,419 1,882,939 25,845,170 - - -

Commit: 1ae4417

Benchmark Workflow

@jonathanpwang jonathanpwang merged commit 11cedc2 into main Sep 29, 2025
34 checks passed
@jonathanpwang jonathanpwang deleted the fix/ecc-align-constants branch September 29, 2025 04:04
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