Skip to content

Conversation

daxpedda
Copy link
Contributor

@daxpedda daxpedda commented Jul 18, 2025

This PR includes some assorted fixes and improvements from #1291.
They are split into descriptive commits for your convenience.

@daxpedda
Copy link
Contributor Author

daxpedda commented Jul 18, 2025

I was wondering why this PR looks so big. But I just forgot to rebase it on top of #1300.
It should be fairly small now.

@daxpedda daxpedda changed the title Assorted fixed and improvements ed448-goldilocks: assorted fixed and improvements Jul 18, 2025
@daxpedda daxpedda changed the title ed448-goldilocks: assorted fixed and improvements ed448-goldilocks: assorted fixes and improvements Jul 18, 2025
@baloo baloo mentioned this pull request Jul 19, 2025
Comment on lines -279 to +278
fn square_n(&self, mut n: u32) -> FieldElement {
let mut result = self.square();
fn square_n<const N: u32>(&self) -> FieldElement {
Copy link
Member

Choose a reason for hiding this comment

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

Curious if this change actually impacts codegen in any way (i.e. for the better)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't check the codegen, but I benchmarked the inversion method and there were no changes.

Copy link
Member

Choose a reason for hiding this comment

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

Feels kind of six of one, half dozen of another in that case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made this change to prevent accidental variable time usage.
But let me know and I can just remove it.

@daxpedda daxpedda requested a review from tarcieri July 20, 2025 15:04
@tarcieri tarcieri merged commit e8b3a04 into RustCrypto:master Jul 20, 2025
20 checks passed
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