ifft_lower

[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

[ilyalesokhin-starkware]

src/core/backend/avx512/fft.rs line 88 at r1 (raw file):

        ifft_vecwise_loop(values, twiddle_dbl, fft_layers - VECWISE_FFT_BITS, index_h);
        for layer in (VECWISE_FFT_BITS..fft_layers).step_by(3) {
            match fft_layers - layer {

do you need a match at every layer?

why not
while fft_layers - layer > 3 {
..
}
match fft_layers - layer

Code quote:

        for layer in (VECWISE_FFT_BITS..fft_layers).step_by(3) {
            match fft_layers - layer {

[ilyalesokhin-starkware]

src/core/backend/avx512/fft.rs line 109 at r1 (raw file):

}

/// Runs the 5 first ifft layers across the entire array.

Suggestion:

Runs the first 5 ifft layers across the entire array.

[ilyalesokhin-starkware]

src/core/backend/avx512/fft.rs line 112 at r1 (raw file):

/// Parameters:
///   values - Pointer to the entire value array, aligned to 64 bytes.
///   twiddle_dbl - The doubles of the twiddle factors for each of the 5 ifft layers.

don't you have some alignement requirements for the twiddle factors?

Code quote:

twiddle_dbl - The doubles of the twiddle factors for each of the 5 ifft layers

[ilyalesokhin-starkware]

src/core/backend/avx512/fft.rs line 161 at r1 (raw file):

) {
    for m in 0..(1 << loop_bits) {
        let index = (index_h << loop_bits) + m;

Make this consistent with ifft_vecwise_loop

Suggestion:

 let index = (index_h << loop_bits) + index_l;

[ilyalesokhin-starkware]

src/core/backend/avx512/fft.rs line 248 at r1 (raw file):

    let r1 = sub_mod_p(val0, val1);

    // Extract the even and odd parts of r1 and twiddle_m_e_dbldbl, and spread as 8 64bit values.

?

Code quote:

twiddle_m_e_dbldb

[ilyalesokhin-starkware]

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

[spapinistarkware]

Reviewable status: 1 of 4 files reviewed, 5 unresolved discussions (waiting on @ilyalesokhin-starkware)

---

src/core/backend/avx512/fft.rs line 112 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

don't you have some alignement requirements for the twiddle factors?

No. Perhaps this code could be a bit more efficient with the first and second twiddle layers, but I load them as values, withouth the efficient pointer loading (i.e. let rust deal with it).

Most fo the times I just ened to load a single M31, and broadcast it.

[ilyalesokhin-starkware]

Reviewed 2 of 4 files at r1.
Reviewable status: 3 of 4 files reviewed, 5 unresolved discussions

[spapinistarkware]

Reviewable status: 3 of 4 files reviewed, 4 unresolved discussions (waiting on @ilyalesokhin-starkware)

---

src/core/backend/avx512/fft.rs line 88 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

do you need a match at every layer?

why not
while fft_layers - layer > 3 {
..
}
match fft_layers - layer

Why is this better?

---

src/core/backend/avx512/fft.rs line 109 at r1 (raw file):

}

/// Runs the 5 first ifft layers across the entire array.

Done.

---

src/core/backend/avx512/fft.rs line 161 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

Make this consistent with ifft_vecwise_loop

Done.

---

src/core/backend/avx512/fft.rs line 248 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

?

Done.

[ilyalesokhin-starkware]

src/core/backend/avx512/fft.rs line 88 at r1 (raw file):

Previously, spapinistarkware (Shahar Papini) wrote…

Why is this better?

it makes it clear that match happens only at the last layer both for the reader and the compiler

[spapinistarkware]

Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @ilyalesokhin-starkware)

---

src/core/backend/avx512/fft.rs line 88 at r1 (raw file):

Previously, ilyalesokhin-starkware wrote…

it makes it clear that match happens only at the last layer both for the reader and the compiler

Done.

[ilyalesokhin-starkware]

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

[graphite-app]

Merge activity

• Mar 4, 4:52 AM EST: spapinistarkware added this pull request to the Graphite merge queue.
• Mar 4, 4:57 AM EST: spapinistarkware merged this pull request with the Graphite merge queue.