address review comments

cryspen · Dec 11, 2024 · 9f92306 · 9f92306
1 parent 8836f78
commit 9f92306
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 13 deletions.
diff --git a/libcrux-ml-dsa/src/encoding/t1.rs b/libcrux-ml-dsa/src/encoding/t1.rs
@@ -27,8 +27,9 @@ pub(crate) fn deserialize<SIMDUnit: Operations>(
     serialized: &[u8],
     result: &mut PolynomialRingElement<SIMDUnit>,
 ) {
+    const WINDOW: usize = 10;
     for i in 0..result.simd_units.len() {
-        result.simd_units[i] = SIMDUnit::t1_deserialize(&serialized[i * 10..(i + 1) * 10]);
+        result.simd_units[i] = SIMDUnit::t1_deserialize(&serialized[i * WINDOW..(i + 1) * WINDOW]);
     }
     ()
 }

diff --git a/libcrux-ml-dsa/src/ml_dsa_generic/instantiations/avx2.rs b/libcrux-ml-dsa/src/ml_dsa_generic/instantiations/avx2.rs
@@ -25,7 +25,9 @@ mod avx2_feature {
             crate::simd::avx2::AVX2SIMDUnit,
             crate::hash_functions::simd256::Shake128x4,
             crate::hash_functions::simd256::Shake256,
-            crate::hash_functions::portable::Shake256Xof, // XXX: Use simd256
+            // We use the portable version here.
+            // It doesn' make sense to do these in parallel.
+            crate::hash_functions::portable::Shake256Xof,
             crate::hash_functions::simd256::Shake256x4,
             ROWS_IN_A,
             COLUMNS_IN_A,
@@ -64,7 +66,9 @@ mod avx2_feature {
             crate::simd::avx2::AVX2SIMDUnit,
             crate::hash_functions::simd256::Shake128x4,
             crate::hash_functions::simd256::Shake256,
-            crate::hash_functions::portable::Shake256Xof, // XXX: Use simd256
+            // We use the portable version here.
+            // It doesn' make sense to do these in parallel.
+            crate::hash_functions::portable::Shake256Xof,
             crate::hash_functions::simd256::Shake256x4,
             ROWS_IN_A,
             COLUMNS_IN_A,
@@ -111,7 +115,9 @@ mod avx2_feature {
             crate::simd::avx2::AVX2SIMDUnit,
             crate::hash_functions::simd256::Shake128x4,
             crate::hash_functions::simd256::Shake256,
-            crate::hash_functions::portable::Shake256Xof, // XXX: Use simd256
+            // We use the portable version here.
+            // It doesn' make sense to do these in parallel.
+            crate::hash_functions::portable::Shake256Xof,
             crate::hash_functions::simd256::Shake256x4,
             ROWS_IN_A,
             COLUMNS_IN_A,
@@ -156,10 +162,14 @@ mod avx2_feature {
     ) -> Result<MLDSASignature<SIGNATURE_SIZE>, SigningError> {
         crate::ml_dsa_generic::sign_pre_hashed::<
             crate::simd::avx2::AVX2SIMDUnit,
-            crate::hash_functions::portable::Shake128, // XXX: Use simd256
+            // We use the portable version here.
+            // It doesn' make sense to do these in parallel.
+            crate::hash_functions::portable::Shake128,
             crate::hash_functions::simd256::Shake128x4,
             crate::hash_functions::simd256::Shake256,
-            crate::hash_functions::portable::Shake256Xof, // XXX: Use simd256
+            // We use the portable version here.
+            // It doesn' make sense to do these in parallel.
+            crate::hash_functions::portable::Shake256Xof,
             crate::hash_functions::simd256::Shake256x4,
             SHAKE128_PH,
             256,
@@ -207,7 +217,9 @@ mod avx2_feature {
             crate::simd::avx2::AVX2SIMDUnit,
             crate::hash_functions::simd256::Shake128x4,
             crate::hash_functions::simd256::Shake256,
-            crate::hash_functions::portable::Shake256Xof, // XXX: Use simd256
+            // We use the portable version here.
+            // It doesn' make sense to do these in parallel.
+            crate::hash_functions::portable::Shake256Xof,
             ROWS_IN_A,
             COLUMNS_IN_A,
             SIGNATURE_SIZE,
@@ -251,7 +263,9 @@ mod avx2_feature {
             crate::simd::avx2::AVX2SIMDUnit,
             crate::hash_functions::simd256::Shake128x4,
             crate::hash_functions::simd256::Shake256,
-            crate::hash_functions::portable::Shake256Xof, // XXX: Use simd256
+            // We use the portable version here.
+            // It doesn' make sense to do these in parallel.
+            crate::hash_functions::portable::Shake256Xof,
             ROWS_IN_A,
             COLUMNS_IN_A,
             SIGNATURE_SIZE,
@@ -293,10 +307,14 @@ mod avx2_feature {
     ) -> Result<(), VerificationError> {
         crate::ml_dsa_generic::verify_pre_hashed::<
             crate::simd::avx2::AVX2SIMDUnit,
-            crate::hash_functions::portable::Shake128, // XXX: Use simd256
+            // We use the portable version here.
+            // It doesn' make sense to do these in parallel.
+            crate::hash_functions::portable::Shake128,
             crate::hash_functions::simd256::Shake128x4,
             crate::hash_functions::simd256::Shake256,
-            crate::hash_functions::portable::Shake256Xof, // XXX: Use simd256
+            // We use the portable version here.
+            // It doesn' make sense to do these in parallel.
+            crate::hash_functions::portable::Shake256Xof,
             SHAKE128_PH,
             256,
             ROWS_IN_A,

diff --git a/libcrux-sha3/src/lib.rs b/libcrux-sha3/src/lib.rs
@@ -270,17 +270,17 @@ pub mod portable {
         }
         use super::*;
 
-        /// SHAKE128 in absorb state
+        /// SHAKE128 Xof state
         pub struct Shake128Xof {
             state: KeccakXofState<1, 168, u64>,
         }
 
-        /// SHAKE256 in absorb state
+        /// SHAKE256 Xof state
         pub struct Shake256Xof {
             state: KeccakXofState<1, 136, u64>,
         }
 
-        /// An XOF in absorb state
+        /// An XOF
         pub trait Xof<const RATE: usize>: private::Sealed {
             /// Create new absorb state
             fn new() -> Self;