refactor: use more defines in assembly fine to make files less verbose#789
Merged
refactor: use more defines in assembly fine to make files less verbose#789
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the code generation for assembly files to use macros/defines, significantly reducing verbosity in generated assembly code. The change converts repetitive Montgomery reduction loops into reusable macro definitions without altering any underlying algorithms.
Key Changes
- Introduced
FROMMONT_STEP()macros in amd64 assembly generation to replace hundreds of lines of repetitive Montgomery reduction code - Refactored ARM64 Poseidon2 code generator to use
f.Define()for modular arithmetic operations (ADD_MOD, SUB_MOD, DOUBLE_MOD, etc.) - Updated hash values across all elliptic curve implementations to trigger recompilation
Reviewed changes
Copilot reviewed 25 out of 27 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
internal/generator/field/asm/amd64/element_frommont.go |
Wraps Montgomery reduction step in a Define macro, called NbWords times instead of inlining |
internal/generator/field/asm/arm64/element_vec_F31_poseidon2.go |
Converts inline functions to Define macros for modular operations, matrix multiplication, improving code generation |
field/asm/element_*w/element_*w_amd64.s |
Generated assembly now uses FROMMONT_STEP() macro repeated N times instead of inlined code blocks |
ecc/*/fr/element_amd64.s and ecc/*/fp/element_amd64.s |
Updated hash values to force Go compiler recompilation after assembly changes |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No algorithmic changes, just refactor code generation to produce more defines and not output thousands of repetitive assembly.
Note
Refactors assembly to reduce repetition without changing algorithms or behavior.
FROMMONT_STEPmacro to implement the repeated MontgomeryfromMontinner loop infield/asm/element_4w/element_4w_amd64.s,element_5w,element_6w,element_10w, andelement_12wFROMMONT_STEP()calls) in each backendmul/reducelogic; only structural DRYing offromMontpathsecc/*/*/element_amd64.sto trigger recompilation; includes remain the sameWritten by Cursor Bugbot for commit d6b3b4c. This will update automatically on new commits. Configure here.