Skip to content

src/kem/kyber/.../*.S, src/sig/dilithium/.../*.S: Return using `ret'#1448

Closed
distorted-mdw wants to merge 1 commit intoopen-quantum-safe:mainfrom
distorted-mdw:mdw/fix-br-lr
Closed

src/kem/kyber/.../*.S, src/sig/dilithium/.../*.S: Return using `ret'#1448
distorted-mdw wants to merge 1 commit intoopen-quantum-safe:mainfrom
distorted-mdw:mdw/fix-br-lr

Conversation

@distorted-mdw
Copy link
Copy Markdown
Contributor

@distorted-mdw distorted-mdw commented May 2, 2023

Partly, lr isn't a register alias under older versions of GNU as. But, mostly, br doesn't mean the same thing to the branch predictor, so it will cause unnecessary misprediction stalls.

  • Does this PR change the input/output behaviour of a cryptographic algorithm (i.e., does it change known answer test values)? (If so, a version bump will be required from x.y.z to x.(y+1).0.)
  • Does this PR change the list of algorithms available -- either adding, removing, or renaming? Does this PR otherwise change an API? (If so, PRs in oqs-provider, OQS-OpenSSL, OQS-BoringSSL, and OQS-OpenSSH will also need to be ready for review and merge by the time this is merged.)

Partly, `lr' isn't a register alias under older versions of GNU `as'.
But, mostly, `br' doesn't mean the same thing to the branch predictor,
so it will cause unnecessary misprediction stalls.

Signed-off-by: Mark Wooding <mark.wooding@trustonic.com>
Copy link
Copy Markdown
Member

@baentsch baentsch left a comment

Choose a reason for hiding this comment

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

Just a comment regarding the placement of this PR: The code you are suggesting to change is automatically imported to liboqs from PQClean, so it would be more suitable to propose the code changes there instead of in this project.

@vincentvbh
Copy link
Copy Markdown

This should be integrated into PQClean/PQClean#488 since the code base is the same.

@distorted-mdw
Copy link
Copy Markdown
Contributor Author

Yes. The reason I hadn't marked it for review is that I haven't yet prepared the MR for PQClean.

@baentsch
Copy link
Copy Markdown
Member

@distorted-mdw Do you still plan on doing a PR to PQClean for this? Our latest update from PQClean (#1512) didn't pull this in....

@baentsch
Copy link
Copy Markdown
Member

baentsch commented Nov 5, 2023

Closing due to lack of feedback & stale-ness.

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.

3 participants