Skip to content

Conversation

@laudiacay
Copy link

draft.....

@laudiacay
Copy link
Author

partially will address RustCrypto/asm-hashes#45 per #449 (comment)

@laudiacay
Copy link
Author

@newpavlov having a little trouble getting the q6 register back into state[0..4]. I have gotten everything else done for the aarch64 implementation (well, we'll find out for sure when I can run it).

Do you know the right syntax for this? Everything I've tried (state.as_mut_ptr(), state[0].as_mut_ptr(), state[0..4].as_mut_ptr().... several other iterations) says "cannot assign to this". I tried using the "v6.1s" syntax to individually assign u32s to indices and apparently those aren't valid registers for in/out.

problem line in question:

lateout("q6") state as *mut u32,

@newpavlov
Copy link
Member

I am not very familiar with ARM assembly, so I can not comment on that. But lateout("q6") state as *mut u32, places pointer to the register, while I would expect you to place value from the state directly into it. The compress function should look roughly like this:

pub fn compress(state: &mut [u32; 5], blocks: &[Block<Sha1Core>]) {
    let [mut s0, mut s1, mut s2, mut s3, mut s4] = *state;
    unsafe {
        // you can use different register names
        asm!(
            // assembly impl
            inout("q0") s0,
            inout("q1") s1,
            inout("q2") s2,
            inout("q3") s3,
            inout("q4") s4,
            in("q5") blocks.as_ptr(),
            in("q6") blocks.len(),
            // clobbers
            options(pure, nostack),
        );
    }
    *state = [s0, s1, s2, s3, s4];
}

Also note that the ARM assembly relies on instructions from the crypto extension. You should add runtime detection using the cpufeatures crate and use the software backend as a fallback.

i don't know why the "do not enable by library crates" warning is in the asm feature

The idea is that the choice should be made by binary crates. Otherwise it will be hard for binary crates to change it if a library unconditionally enables such feature.

@newpavlov
Copy link
Member

Closing as stale. Feel free to reopen it or create a new PR, if you would like to resume to work on this problem.

@newpavlov newpavlov closed this Jun 9, 2023
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