formosa-crypto / formosa-mlkem

Other
11 stars 7 forks source link

replace keccakf1600 - with significant performance improvements for ref #24

Closed tfaoliveira closed 8 months ago

tfaoliveira commented 9 months ago

comparison basis (left side): https://github.com/formosa-crypto/libjade/tree/bb8569c01597338148800210de5aa2b512f783c6

new cycles for ref: https://github.com/formosa-crypto/libjade/commit/b53d83793fee8303cf2cfbd62171911741f327b0 new cycles avx2: https://github.com/formosa-crypto/libjade/commit/7046ee95c5f924ed11343c9babadbc2358a16752

ref

keypair, 302134 -> 220474
enc    , 351866 -> 267840
dec    , 424384 -> 339360

avx2

keypair, 85284 -> 83438
enc    , 82856 -> 80916
dec    , 83982 -> 82122
vbgl commented 9 months ago

If you split this PR in two halves, I think I can manage to do the proof for the REF implementation (while postponing the proof for the AVX2 version).

vbgl commented 9 months ago

Well, no longer sure about my previous statement…

vbgl commented 9 months ago

I’m done fixing the proofs. I’ve removed a few redundant instructions.

tfaoliveira commented 8 months ago

Hi Vincent, thank you for pushing on this. Do you think it is ready to be merged?

vbgl commented 8 months ago

Yes it is, if you don’t complain about the changes I’ve made to the code.

tfaoliveira commented 8 months ago

I don't complain. At some point, maybe during the next retreat, I would like to discuss compiler warnings with you and the compiler team.

vbgl commented 8 months ago

Thanks. There was no need to squash the three changes into a single commit.