debris / tiny-keccak

An implementation of Keccak derived functions specified in FIPS-202, SP800-185 and KangarooTwelve
Creative Commons Zero v1.0 Universal
194 stars 50 forks source link

Bounds checking #9

Closed burdges closed 5 years ago

burdges commented 7 years ago

I believe there is a trick to avoid the bounds checking without unsafe code by consolodating the bound checks to the beginning and end of the loop. I forget exactly how it works though, maybe using a slice [..] or something.

debris commented 7 years ago

I believe that latest version of tiny-keccak does not perform bounds checking, cause it in benchmarks on my machine (macos) it's as fast as original C implementation.

rphmeier commented 7 years ago

Out of curiosity, I'm gonna experiment a bit with typenum and exporting type aliases for common variants and see how that affects the bounds checking.

rphmeier commented 7 years ago

@burdges I found a trick which may be the one you were referencing:

instead of fn xorin(&mut [u8], &[u8], usize) with unsafe indexing, just

fn xorin(dst: &mut [u8], src: &[u8]) {
    for (d, i) in dst.iter_mut().chain(src) {
        *d ^= *i
    }
}

and restrict the lengths of the slices at the call site.

burdges commented 7 years ago

That works? I found one in Rust commit 6a7bc47. I write stuff like this sometimes :

    let mut i = s.len();
    let s = &mut s[..i];  // elide bounds checks; see Rust commit 6a7bc47

I've used this to removed the unsafe from xorin in https://github.com/debris/tiny-keccak/pull/8/commits/ccf709b13ed1e9512068c624e707cdc7d971ed66

burdges commented 7 years ago

Oh I missed that you submitted https://github.com/debris/tiny-keccak/pull/11 which does this differently.

rphmeier commented 7 years ago

Seems like both tricks get me the same performance in benches, although I haven't looked at generated assembly yet.

burdges commented 7 years ago

You eliminated more unsafes than I did though. I'm not sure your iterator comment works though. If so, maybe only because you're using a fixed length array.