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.

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

Add cmovz/cmovnz support for aarch64. #744

Merged
merged 2 commits into from
Mar 2, 2022
Merged

Add cmovz/cmovnz support for aarch64. #744

merged 2 commits into from
Mar 2, 2022

Conversation

codahale
Copy link
Contributor

@codahale codahale commented Mar 1, 2022

This passes tests and I believe it works, but I also have the assembly programming experience of your average kindergartner, so a second opinion would be worth getting.

Running cargo rustc --release -- --emit asm on the portable cmovz yields this:

        neg     w8, w0
        orr     w8, w8, w0
        ubfx    x8, x8, #7, #1
        ldr     x9, [x2]
        neg     x10, x8
        and     x9, x9, x10
        sub     x8, x8, #1
        and     x8, x8, x1
        orr     x8, x9, x8
        str     x8, [x2]
        ret

And a naive, branching version of cmovz yields this:

        cbz     x0, LBB1_2
        ret
LBB1_2:
        str     x1, [x2]
        ret

Which makes me think cmp/csel is the right choice.

@tarcieri
Copy link
Member

tarcieri commented Mar 2, 2022

Looks good, thank you!

Can you update the documentation to reflect AArch64 assembly is available as well?

@codahale
Copy link
Contributor Author

codahale commented Mar 2, 2022

I updated README.md and Cargo.toml.

@tarcieri tarcieri merged commit 4086539 into RustCrypto:master Mar 2, 2022
@tarcieri
Copy link
Member

tarcieri commented Mar 2, 2022

Thank you!

@tarcieri tarcieri mentioned this pull request Mar 2, 2022
@codahale codahale deleted the cmov-aarch64 branch March 2, 2022 15:34
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.

None yet

2 participants