Skip to content
This repository was archived by the owner on Aug 18, 2025. It is now read-only.

Conversation

haraldh
Copy link
Contributor

@haraldh haraldh commented Mar 18, 2021

No description provided.

@tarcieri
Copy link
Member

Not sure I understand the purpose of this change?

@haraldh
Copy link
Contributor Author

haraldh commented Mar 18, 2021

Hmm, with the current master:

❯ cargo +nightly bench
   Compiling sha2-asm v0.6.0 (/home/harald/git/asm-hashes/sha2)
   Compiling whirlpool-asm v0.6.0 (/home/harald/git/asm-hashes/whirlpool)
   Compiling sha1-asm v0.5.0 (/home/harald/git/asm-hashes/sha1)
   Compiling md5-asm v0.5.0 (/home/harald/git/asm-hashes/md5)
error[E0308]: mismatched types
  --> whirlpool/benches/lib.rs:14:33
   |
14 |         whirlpool_asm::compress(&mut state, &data);
   |                                 ^^^^^^^^^^ expected `u64`, found `u8`
   |
   = note: expected mutable reference `&mut [u64; 8]`
              found mutable reference `&mut [u8; 64]`

error[E0308]: mismatched types
  --> md5/benches/lib.rs:14:39
   |
14 |         md5_asm::compress(&mut state, &data);
   |                                       ^^^^^ expected slice `[[u8; 64]]`, found array `[u8; 64]`
   |
   = note: expected reference `&[[u8; 64]]`
              found reference `&[u8; 64]`

error[E0308]: mismatched types
  --> sha1/benches/lib.rs:14:40
   |
14 |         sha1_asm::compress(&mut state, &data);
   |                                        ^^^^^ expected slice `[[u8; 64]]`, found array `[u8; 64]`
   |
   = note: expected reference `&[[u8; 64]]`
              found reference `&[u8; 64]`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
error: could not compile `sha1-asm`

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error[E0308]: mismatched types
  --> whirlpool/benches/lib.rs:14:45
   |
14 |         whirlpool_asm::compress(&mut state, &data);
   |                                             ^^^^^ expected slice `[[u8; 64]]`, found array `[u8; 64]`
   |
   = note: expected reference `&[[u8; 64]]`
              found reference `&[u8; 64]`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0308`.
error: build failed

@tarcieri
Copy link
Member

Huh strange. It builds for me with:

rustc 1.52.0-nightly (476acbf1e 2021-03-03)

and just to check there wasn't a recent regression, I also updated:

rustc 1.52.0-nightly (36f1f04f1 2021-03-17)

Seems to be working here:

   Compiling whirlpool v0.9.0 (/Users/bascule/src/RustCrypto/hashes/whirlpool)
    Finished bench [optimized] target(s) in 4.88s

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

running 4 tests
test bench1_10    ... bench:          94 ns/iter (+/- 27) = 106 MB/s
test bench2_100   ... bench:         820 ns/iter (+/- 154) = 121 MB/s
test bench3_1000  ... bench:       7,907 ns/iter (+/- 1,421) = 126 MB/s

What nightly are you using?

@haraldh
Copy link
Contributor Author

haraldh commented Mar 18, 2021

❯ rustc +nightly --version
rustc 1.52.0-nightly (8f349be27 2021-03-08)

@haraldh
Copy link
Contributor Author

haraldh commented Mar 18, 2021

still fails with:

❯ rustc +nightly --version
rustc 1.52.0-nightly (f5d8117c3 2021-03-16)

Are you sure the github master is in sync?

@tarcieri
Copy link
Member

Yep. Working for me here.

Maybe try updating to the latest nightly?

@newpavlov
Copy link
Member

newpavlov commented Mar 18, 2021

Yeah, probably @tarcieri has not updated the master branch. It looks I've forgot to update benchmarks in #30 (and we do not check them in CI). I think it could be worth to pass not a single-element slice, but with many elements (e.g. 1024).

@haraldh
Copy link
Contributor Author

haraldh commented Mar 18, 2021

@tarcieri you are in the wrong repo.. try asm-hashes :-)

@tarcieri
Copy link
Member

Oh whoops! My bad, heh

@tarcieri
Copy link
Member

Looks like this regressed due to #30 /cc @newpavlov

We should probably add checks the benchmarks build in CI.

That said, it looks like this may be needed after all.

@newpavlov
Copy link
Member

I guess we can merge it and update benchmarks further in a separate PR.

@newpavlov newpavlov merged commit 2c998b2 into RustCrypto:master Mar 18, 2021
@newpavlov newpavlov mentioned this pull request Mar 18, 2021
@haraldh haraldh deleted the fix_benches branch March 18, 2021 15:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants