From cea9fe7bef5bada6ef25cfec3aa12cea264ed697 Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Fri, 14 Aug 2026 11:58:09 +0100 Subject: [PATCH 1/3] fix(poly): drop wrong packing-width assert on the middle eval slice `base_eval_eq_packed_with_packed_output` asserted `log_packing_width <= eval_points.len()`, but `eval_points` is the *middle* slice produced by `par_eval_eq`, not the full point: the `log_packing_width` suffix is already folded into `eq_evals` and the `log_chunks` prefix into the packed scalar. Its length is therefore `n - log_packing_width - log_chunks` and has nothing to do with the packing width. The callers only guarantee it is at least 2, while the assert demanded at least `log_packing_width`, so every `n` in `[lpw + log_chunks + 2, 2*lpw + log_chunks)` panicked in debug builds. On a 32-thread AVX512 host that is log_chunks=7, lpw=4, so 13- and 14-variable polynomials aborted `test_packed_eval_eq` and `lean_prover`'s `test_small_memory`. The band is machine-dependent and non-empty on any target with a packing width above 4. The invariant is real, but it belongs to the callers, which already check it against the full point (`compute_eval_eq_base_packed` and `compute_eval_eq_base_packed_batched`). This restores 5cf504a, which removed the same assert for the same reason and was reverted by e45a0ed. It regressed because no test covered the band, so add one that computes the bounds from the runtime thread count and SIMD width rather than hardcoding them. Co-Authored-By: Claude Opus 5 (1M context) --- crates/backend/poly/src/eq_mle.rs | 41 ++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/crates/backend/poly/src/eq_mle.rs b/crates/backend/poly/src/eq_mle.rs index 3ab98c6e..214138b3 100644 --- a/crates/backend/poly/src/eq_mle.rs +++ b/crates/backend/poly/src/eq_mle.rs @@ -1023,10 +1023,13 @@ fn base_eval_eq_packed_with_packed_output( { // Ensure that the output buffer size is correct: // It should be of size `2^n`, where `n` is the number of variables. - let width = F::Packing::WIDTH; - let log_packing_width = log2_strict_usize(width); + // + // `eval_points` is the *middle* slice handed over by `par_eval_eq`, not the full point: + // the `log_packing_width` suffix is already folded into `eq_evals` and the `log_chunks` + // prefix into `packed_scalar`. Its length is therefore unrelated to the packing width, + // and asserting `log_packing_width <= eval_points.len()` here is wrong — that invariant + // belongs to the callers, which check it against the *full* point. debug_assert_eq!(out.len(), 1 << eval_points.len()); - debug_assert!(log_packing_width <= eval_points.len()); match eval_points.len() { 0 => { @@ -1320,6 +1323,38 @@ mod tests { } } + /// `base_eval_eq_packed_with_packed_output` receives the *middle* slice of the eval + /// points: `par_eval_eq` strips a `log_chunks` prefix and a `log_packing_width` suffix, + /// leaving `n - log_packing_width - log_chunks` variables. The packed path only requires + /// that to be at least 2, so the middle slice is routinely *shorter* than + /// `log_packing_width` and the kernel must not assume otherwise. + /// + /// This covers the narrow band of `n_vars` just above the packed-path threshold, where + /// that happens. Both the assertion and the band are machine-dependent (they move with + /// the thread count and SIMD width), so the bounds are computed rather than hardcoded. + #[test] + fn base_packed_handles_middle_slice_shorter_than_packing_width() { + let log_packing_width = log2_strict_usize(::Packing::WIDTH); + let (log_chunks, _) = parallel_split(); + let mut rng = StdRng::seed_from_u64(11); + + // Lower bound: first `n_vars` taking the packed path (see `compute_eval_eq_base_packed`). + // Upper bound: first `n_vars` whose middle slice reaches `log_packing_width`. + for n_vars in (log_packing_width + log_chunks + 2)..=(2 * log_packing_width + log_chunks) { + let eval: Vec = (0..n_vars).map(|_| rng.random()).collect(); + let scalar: EF = rng.random(); + + let mut expected = EF::zero_vec(1 << n_vars); + compute_eval_eq_base::(&eval, &mut expected, scalar); + + let mut packed = >::ExtensionPacking::zero_vec(1 << (n_vars - log_packing_width)); + compute_eval_eq_base_packed::(&eval, &mut packed, scalar); + + let unpacked: Vec = >::ExtensionPacking::to_ext_iter_vec(packed); + assert_eq!(expected, unpacked, "n_vars = {n_vars}"); + } + } + #[test] fn test_compute_eval_eq_packed_dual() { let packing_width = ::Packing::WIDTH; From adc445faf65f8f95f31955ac5c88816d9986487e Mon Sep 17 00:00:00 2001 From: Tom Wambsgans Date: Fri, 14 Aug 2026 22:11:40 +0200 Subject: [PATCH 2/3] test(poly): make the short-slice test bite on every target The test added in the previous commit derives its `n_vars` band from `parallel_split()` and the packing width. With packing width 4 (arm64 NEON) that band collapses to a single `n_vars` whose middle slice is exactly `log_packing_width`, which the old assert accepted, so the test passes with the assert restored. It does not cover the bug on the machines this repo is developed on. The band was not uncovered before either: `test_packed_eval_eq` already loops `n_vars` from `log_packing_width` to 20 through `compute_eval_eq_base_packed`, which is why the assert aborted it on a 32-thread AVX512 host. What let the assert come back in e45a0ed is that `cargo testall` and CI both run `--release`, where `debug_assert!` is compiled out. So call `base_eval_eq_packed_with_packed_output` directly on slices of length 1 to 3 and compare against its unpacked-output twin `base_eval_eq_packed`. Those are exactly the hardcoded arms the assert made unreachable, and the check no longer depends on the thread count or SIMD width, so it fails on arm64 too. It still only fires in a debug build; gating it in CI would need a debug-assertions job. Also trim the comment at the fix site. Co-Authored-By: Claude Opus 5 (1M context) --- crates/backend/poly/src/eq_mle.rs | 47 +++++++++++-------------------- 1 file changed, 16 insertions(+), 31 deletions(-) diff --git a/crates/backend/poly/src/eq_mle.rs b/crates/backend/poly/src/eq_mle.rs index 214138b3..67952b06 100644 --- a/crates/backend/poly/src/eq_mle.rs +++ b/crates/backend/poly/src/eq_mle.rs @@ -1021,14 +1021,8 @@ fn base_eval_eq_packed_with_packed_output( F: Field, EF: ExtensionField, { - // Ensure that the output buffer size is correct: - // It should be of size `2^n`, where `n` is the number of variables. - // - // `eval_points` is the *middle* slice handed over by `par_eval_eq`, not the full point: - // the `log_packing_width` suffix is already folded into `eq_evals` and the `log_chunks` - // prefix into `packed_scalar`. Its length is therefore unrelated to the packing width, - // and asserting `log_packing_width <= eval_points.len()` here is wrong — that invariant - // belongs to the callers, which check it against the *full* point. + // `eval_points` is the middle slice from `par_eval_eq`, so its length says nothing about the + // packing width (the callers assert that against the full point). debug_assert_eq!(out.len(), 1 << eval_points.len()); match eval_points.len() { @@ -1323,35 +1317,26 @@ mod tests { } } - /// `base_eval_eq_packed_with_packed_output` receives the *middle* slice of the eval - /// points: `par_eval_eq` strips a `log_chunks` prefix and a `log_packing_width` suffix, - /// leaving `n - log_packing_width - log_chunks` variables. The packed path only requires - /// that to be at least 2, so the middle slice is routinely *shorter* than - /// `log_packing_width` and the kernel must not assume otherwise. - /// - /// This covers the narrow band of `n_vars` just above the packed-path threshold, where - /// that happens. Both the assertion and the band are machine-dependent (they move with - /// the thread count and SIMD width), so the bounds are computed rather than hardcoded. + /// `par_eval_eq` hands the kernel a middle slice of any length >= 2, so the hardcoded arms + /// below `log_packing_width` must agree with the unpacked-output twin. Calling the kernel + /// directly keeps this independent of the SIMD width and thread count. #[test] - fn base_packed_handles_middle_slice_shorter_than_packing_width() { - let log_packing_width = log2_strict_usize(::Packing::WIDTH); - let (log_chunks, _) = parallel_split(); + fn base_packed_kernel_handles_short_slices() { let mut rng = StdRng::seed_from_u64(11); + let scalar: EF = rng.random(); + let eq_evals = ::Packing::from_fn(|_| rng.random()); - // Lower bound: first `n_vars` taking the packed path (see `compute_eval_eq_base_packed`). - // Upper bound: first `n_vars` whose middle slice reaches `log_packing_width`. - for n_vars in (log_packing_width + log_chunks + 2)..=(2 * log_packing_width + log_chunks) { - let eval: Vec = (0..n_vars).map(|_| rng.random()).collect(); - let scalar: EF = rng.random(); + for len in 1..=3 { + let points: Vec = (0..len).map(|_| rng.random()).collect(); - let mut expected = EF::zero_vec(1 << n_vars); - compute_eval_eq_base::(&eval, &mut expected, scalar); + let mut expected = EF::zero_vec(::Packing::WIDTH << len); + base_eval_eq_packed::(&points, &mut expected, eq_evals, scalar); - let mut packed = >::ExtensionPacking::zero_vec(1 << (n_vars - log_packing_width)); - compute_eval_eq_base_packed::(&eval, &mut packed, scalar); + let mut packed = EFPacking::::zero_vec(1 << len); + let packed_scalar = EFPacking::::from(scalar); + base_eval_eq_packed_with_packed_output::(&points, &mut packed, eq_evals, packed_scalar); - let unpacked: Vec = >::ExtensionPacking::to_ext_iter_vec(packed); - assert_eq!(expected, unpacked, "n_vars = {n_vars}"); + assert_eq!(expected, EFPacking::::to_ext_iter_vec(packed), "len = {len}"); } } From be41aaf113dd4c57cd8093142c35903c6b5b3712 Mon Sep 17 00:00:00 2001 From: Tom Wambsgans Date: Fri, 14 Aug 2026 22:19:39 +0200 Subject: [PATCH 3/3] fix(field): drop redundant `#[must_use]` on iterator-returning methods Nightly clippy (1.99.0-nightly, 2026-08-13) extended `clippy::double_must_use` to `impl Trait` return types, so `to_ext_lanes` and `packed_ext_powers_capped` now fail the CI lint job under `-Dwarnings`. `Iterator` is already `#[must_use]`, so the attribute never added anything and dropping it changes no behaviour. These are the only two occurrences in the workspace. The failure is independent of the branch it appears on: it breaks `main` and every open PR, and because `field` fails to lint, no downstream crate is reached at all. Co-Authored-By: Claude Opus 5 (1M context) --- crates/backend/field/src/packed/packed_traits.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/backend/field/src/packed/packed_traits.rs b/crates/backend/field/src/packed/packed_traits.rs index bd9a643b..95b601d4 100644 --- a/crates/backend/field/src/packed/packed_traits.rs +++ b/crates/backend/field/src/packed/packed_traits.rs @@ -337,7 +337,6 @@ pub trait PackedFieldExtension impl Iterator; /// Given a iterator of packed extension field elements, convert to an iterator of @@ -360,7 +359,6 @@ pub trait PackedFieldExtension impl Iterator { Self::packed_ext_powers(base).take(unpacked_len.div_ceil(BaseField::Packing::WIDTH)) }