refactor(aes): machine-word generic fixslice#560
Conversation
|
This looks good to me as a general direction, though it may be some time before I can review it in detail. I'm a bit confused how this broke the build the way it did, it seems somehow with only changes to the soft backend it broke things in the NEON backend somehow? |
|
I haven't looked closely at the CI yet, but perhaps it is already broken on latest stable rustc prior to this PR? |
|
The CI failures were fixed in #562. |
Drop the cpubits-based selection between width-specific fixslice modules in preparation for unifying both into a single width-generic implementation.
Refactor the fixslice AES implementation so the algorithm body is generic over a `Word` trait, with a single `impl Word for u32` carrying all width-specific code (row width, packing routines, bitslice/inv_bitslice). The three-pass delta-swap pipeline used by `bitslice` and `inv_bitslice` is extracted into a shared width-generic `bitslice_swaps` helper, with mask constants derived from a `Word::byte_repeat` method. Each impl now only carries the bytes-in/bytes-out routine. This is a purely mechanical refactor: no logic changes, no behavior changes, the only word width supported is still `u32`.
Add the second `Word` impl, `impl Word for u64`, which batches 4 blocks per state versus 2 for `u32`. The concrete re-exports and the hazmat helpers are gated by `target_pointer_width` via `cpubits!`, selecting `u32` on 16/32-bit targets and `u64` on 64-bit.
d237e81 to
0f8f6b9
Compare
| type Blocks: ArraySize; | ||
|
|
||
| /// Width in bits of one row of the bitsliced state (8 for `u32`, 16 for `u64`). | ||
| const ROW_BITS: u32; |
There was a problem hiding this comment.
Seems like this could have a default? (could probably use an explanatory comment)
| const ROW_BITS: u32; | |
| const ROW_BITS: u32 = size_of::<Self>() * 2; |
...or if HALF_ROW were defined this way it gets rid of the * 2
There was a problem hiding this comment.
Huh, I am a bit surprised that it works. I will fix it as part of #575.
tarcieri
left a comment
tarcieri
left a comment
There was a problem hiding this comment.
I really like the amount of duplication this gets rid of and would like to see it landed
3 participants
This PR collapses the two fixslice impls into a single
fixslice.rsparameterized over aWordtrait that abstracts the machine-word width.I tried to structure the commits to make it clear to the reviewer that this introduces no changes to the existing implementation logic.
This PR was motivated by #559 to support larger machine-word (SIMD) implementations without introducing major duplication and maintenance burden.