Skip to content

keccak: refactor to a closure-based API#113

Open
newpavlov wants to merge 4 commits intomasterfrom
keccak/closure
Open

keccak: refactor to a closure-based API#113
newpavlov wants to merge 4 commits intomasterfrom
keccak/closure

Conversation

@newpavlov
Copy link
Member

@newpavlov newpavlov commented Feb 28, 2026

The new API provides a better way for exposing support for parallel processing in implemented backends and resolves the issue with branching on each application of a Keccak permutation.

@newpavlov
Copy link
Member Author

newpavlov commented Mar 2, 2026

I am not sure how to deal with the SIMD "backend". It adds support for operating over u64x2/4/8 and allows user to select the width, while the trait expects to select optimal width using target feature detection and does not allow to keep the state in the interleaved form (i.e. [[u64; Self::PAR_SIZE]; PLEN]).

One potential option is to introduce keccak_backend="simd128/256/512" configuration options.

@aewag
Do you use the SIMD functions in practice? If you do, could you show us code which does it?

@newpavlov newpavlov force-pushed the keccak/closure branch 2 times, most recently from 0945c16 to fb99f23 Compare March 2, 2026 14:44
@tarcieri
Copy link
Member

tarcieri commented Mar 2, 2026

@newpavlov as a general rule SIMD backends are going to keep the state in SIMD registers between rounds, with an initial load step and at the end of a given number of rounds a store step which splits them back out into an array of states

@newpavlov newpavlov marked this pull request as ready for review March 2, 2026 15:54
@newpavlov
Copy link
Member Author

@tarcieri
The portable SIMD backend still needs to be handled somehow, but I think this PR is mostly ready for review.

/// Trait used to define a closure which operates over Keccak backends.
pub trait BackendClosure {
/// Execute closure with the provided backend.
fn call_once<B: Backend>(self);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could modify it to return a result (i.e. fn call_once<B: Backend>(self) -> Self::Output) and modify the Keccak methods accordingly, but I am not sure it's worth the trouble.

@tarcieri
Copy link
Member

tarcieri commented Mar 2, 2026

To me it would make more sense if the different state sizes and their backends were modeled as separate types, rather than listing them all out in a single trait/type. That's how we do things everywhere else that some algorithm/construction is implemented for different sizes.

I'm not sure it even makes sense to support this for any state size other than b=1600? XKCP doesn't have SIMD implementations for the other state sizes.

@newpavlov
Copy link
Member Author

To me it would make more sense if the different state sizes and their backends were modeled as separate types, rather than listing them all out in a single trait/type.

We could do it, but it would result in a fair amount of somewhat annoying boilerplate. On the other hand, it would resolve the issue with the software fallback, since backends will be selected independently for each function.

I'm not sure it even makes sense to support this for any state size other than b=1600?

I am not aware about potential parallel uses for those, so maybe not. I was trying to future-proof the API and adopting the portable SIMD implementation for it looks simple enough.

I guess we could start with parallel support only for b=1600 and add other functions later if needed.

#[inline]
fn as_mut(&mut self) -> &mut [u64; PLEN] {
&mut self.state
pub fn with_f800(&self, f: impl FnOnce(Fn800)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These methods are kept for consistency with the f/p1600 methods and to future-proof the API.

Copy link
Member

@tarcieri tarcieri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what's up with the commented out sections but having something in place for this seems good and I don't want to delay a release too much. If we get the API nailed down I can add e.g. SSSE3 and/or AVX2 later.

I'm also fine with keeping things infallible.

@newpavlov
Copy link
Member Author

newpavlov commented Mar 3, 2026

Not sure what's up with the commented out sections

It's mostly the portable SIMD stuff. I plan to introduce simd128/256/512 backends today or tomorrow as a replacement for those parts.

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