Modify bit_reverse for SIMD backend

[andrewmilson]

This change is 

[andrewmilson]

• Add benchmarks for SIMD fft #609
• Add benchmarks for SIMD quotients #608
• Add benchmarks for SIMD eval_at_point #607
• Add benchmarks for SIMD bit_reverse #606
• Implement QuotientOps for SIMD backend #605
• Implement AccumulationOps for SIMD backend #604
• Implement FriOps for SIMD backend #603
• Implement PolyOps for SIMD backend #599
• Modify FFT implementation for SIMD backend #596
• Add FFT implementation for SIMD backend #602
• Modify blake2s implementation for SIMD backend #595
• Add blake2s implementation for SIMD backend #601
• Modify bit_reverse for SIMD backend #594 👈
• Add bit_reverse for SIMD backend #600
• Implement FieldOps for SIMD backend #593
• Add SIMD backend arithmetic #592
• Enable 'rng.gen()'ing field types #591
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @andrewmilson and the rest of your teammates on Graphite

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 91.37931% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 91.33%. Comparing base (15e93af) to head (2719965).

Files	Patch %	Lines
crates/prover/src/core/backend/simd/bit_reverse.rs	91.37%	5 Missing ⚠️

Additional details and impacted files

@@                            Coverage Diff                             @@
##           05-02-Add_bit_reverse_for_SIMD_backend     #594      +/-   ##
==========================================================================
+ Coverage                                   91.24%   91.33%   +0.09%     
==========================================================================
  Files                                          76       76              
  Lines                                        9529     9560      +31     
  Branches                                     9529     9560      +31     
==========================================================================
+ Hits                                         8695     8732      +37     
+ Misses                                        763      757       -6     
  Partials                                       71       71              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[spapinistarkware]

Reviewed all commit messages.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @andrewmilson)

a discussion (no related file):
Same here, do a first commit with just copying

---

[shaharsamocha7]

Reviewed all commit messages.
Reviewable status: 0 of 2 files reviewed, 5 unresolved discussions (waiting on @andrewmilson and @spapinistarkware)

---

crates/prover/src/core/backend/simd/bit_reverse.rs line 124 at r2 (raw file):

    }

    unsafe { transmute(data) }

Suggestion:
Have from_simd func?

Code quote:

    unsafe { transmute(data) }

---

crates/prover/src/core/backend/simd/bit_reverse.rs line 139 at r2 (raw file):

    use crate::core::backend::Column;
    use crate::core::fields::m31::BaseField;
    use crate::core::utils::bit_reverse as ground_truth_bit_reverse;

rename?

Suggestion:

cpu_bit_reverse

---

crates/prover/src/core/backend/simd/bit_reverse.rs line 144 at r2 (raw file):

    fn test_bit_reverse16() {
        let data: Aligned<A64, [u32; 256]> = Aligned(array::from_fn(|i| i as u32));
        let mut expected: Aligned<A64, [u32; 256]> = data;

Can you explain/document why we need this Aligned?

Code quote:

        let data: Aligned<A64, [u32; 256]> = Aligned(array::from_fn(|i| i as u32));
        let mut expected: Aligned<A64, [u32; 256]> = data;

---

crates/prover/src/core/backend/simd/mod.rs line 45 at r2 (raw file):

    fn bit_reverse_column(_column: &mut Self::Column) {
        todo!()

Don't you implement also this one?

Code quote:

todo!()

[andrewmilson]

Reviewable status: 0 of 2 files reviewed, 4 unresolved discussions (waiting on @shaharsamocha7 and @spapinistarkware)

---

crates/prover/src/core/backend/simd/bit_reverse.rs line 139 at r2 (raw file):

Previously, shaharsamocha7 wrote…

rename?

Done.

---

crates/prover/src/core/backend/simd/bit_reverse.rs line 144 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Can you explain/document why we need this Aligned?

Removed, updated.

---

crates/prover/src/core/backend/simd/mod.rs line 45 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Don't you implement also this one?

Not for &mut [PackedSecureField] only for &mut [PackedBaseField]

[shaharsamocha7]

Reviewed 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @spapinistarkware)

---

crates/prover/src/core/backend/simd/mod.rs line 45 at r2 (raw file):

Previously, andrewmilson (Andrew Milson) wrote…

Not for &mut [PackedSecureField] only for &mut [PackedBaseField]

Why not? in another pr?

[andrewmilson]

Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @shaharsamocha7 and @spapinistarkware)

---

crates/prover/src/core/backend/simd/mod.rs line 45 at r2 (raw file):

Previously, shaharsamocha7 wrote…

Why not? in another pr?

Sure, it's not used anywhere at the moment AFAIK and it's not trivial. AVX backend has a bogus implementation of it too

[spapinistarkware]

Reviewed 1 of 3 files at r4, 1 of 1 files at r5.
Reviewable status: all files reviewed (commit messages unreviewed), 2 unresolved discussions (waiting on @andrewmilson)

---

crates/prover/src/core/backend/simd/bit_reverse.rs line 119 at r5 (raw file):

    for _ in 0..4 {
        // Apply the abcd:0123 => 0abc:123d permutation.
        // `LoLoInterleaveHiLo` allows us to interleave the first half of 2 words.

I don't see any reference for this (before as well). Maybe skip it, and just keep the interleave.

[spapinistarkware]

Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @andrewmilson)

[spapinistarkware]

with one comment

Reviewed 1 of 3 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @andrewmilson)