Incorporate fft in AVX backend

[spapinistarkware]

This change is 

[spapinistarkware]

• ColumnOps #459
• Use precomputed twiddles in avx #443
• Precompute twiddles #401
• Remove FRI PolyOps dependency #399
• avx slow eval #398
• avx poly extend #397
• AVX fft eval on larger domains #396
• Reverse order of evaluations #395
• Incorporate fft in AVX backend #394 👈
• AVX regular fft #390
• Separate ifft #389
• Support ifft for itnermediate sizes #383
• AVX fft benchmark #382
• ifft_full #381
• Save twiddles #380
• ifft_lower #379
• avx ifft3 #378
• avx vecwise butterflies #377
• avx vecwise ibutterfly #368
• avx ibutterfly #367
• avx butterfly #366
• avx poly #365
• dev

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

Join @spapinistarkware and the rest of your teammates on Graphite

[shaharsamocha7]

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @spapinistarkware)

---

src/core/backend/avx512/circle.rs line 30 at r1 (raw file):

        AVX512Backend::bit_reverse_column(&mut values);
        // TODO(spapini): Handle small cases.
        let log_size = values.length.ilog2();

Move to the beginning of the function

I guess that if this is too small then we can just do trivial interpolation without twiddles or anything

Code quote:

        // TODO(spapini): Handle small cases.
        let log_size = values.length.ilog2();

---

src/core/backend/avx512/circle.rs line 38 at r1 (raw file):

                log_size as usize,
            );
        }

Should this be implemented also for AVX512?

Code quote:

        unsafe {
            ifft::ifft(
                std::mem::transmute(values.data.as_mut_ptr()),
                &twiddles[1..],
                log_size as usize,
            );
        }

---

src/core/backend/avx512/circle.rs line 54 at r1 (raw file):

        _point: crate::core::circle::CirclePoint<E>,
    ) -> E {
        todo!()

Why did you change it?

Code quote:

    fn eval_at_point<E: crate::core::fields::ExtensionOf<BaseField>>(
        _poly: &CirclePoly<Self, BaseField>,
        _point: crate::core::circle::CirclePoint<E>,
    ) -> E {
        todo!()

---

src/core/backend/avx512/circle.rs line 64 at r1 (raw file):

        // TODO(spapini): Precompute twiddles.
        let twiddles = rfft::get_twiddle_dbls(domain);

Why this file is called rfft? (and not fft?)

Code quote:

rfft

[spapinistarkware]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @shaharsamocha7)

---

src/core/backend/avx512/circle.rs line 38 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Should this be implemented also for AVX512?

This is the avx512 impl

---

src/core/backend/avx512/circle.rs line 54 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Why did you change it?

It does not bring the right results now. Next few PRs fix this.
The orde of the coefficients is different with the new fft impl.

---

src/core/backend/avx512/circle.rs line 64 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Why this file is called rfft? (and not fft?)

Because rust doesn;t let me have fft module inside fft module.

[spapinistarkware]

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

---

src/core/backend/avx512/circle.rs line 30 at r1 (raw file):

Previously, shaharsamocha7 wrote…

Move to the beginning of the function

I guess that if this is too small then we can just do trivial interpolation without twiddles or anything

Done.

Yes, I'm adding this in the next few PRs.

[shaharsamocha7]

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

---

src/core/backend/avx512/circle.rs line 38 at r1 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

This is the avx512 impl

My bad

---

src/core/backend/avx512/circle.rs line 64 at r1 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

Because rust doesn;t let me have fft module inside fft module.

so what rfft stands for?
maybe we should change the dir name to NTT?

[spapinistarkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @shaharsamocha7)

---

src/core/backend/avx512/circle.rs line 64 at r1 (raw file):

Previously, shaharsamocha7 wrote…

so what rfft stands for?
maybe we should change the dir name to NTT?

regular. it says so at the top of the module.
I can change it in another PR. Let's discuss a good name tomorrow:)

[graphite-app]

Merge activity

• Mar 6, 2:09 PM EST: spapinistarkware added this pull request to the Graphite merge queue.
• Mar 6, 2:15 PM EST: spapinistarkware merged this pull request with the Graphite merge queue.