Skip to content

Merge simd vector traits - #197

Merged
ejmahler merged 9 commits into
masterfrom
merge-simd-vector-traits
Sep 28, 2026
Merged

ejmahler merged 9 commits into
masterfrom
merge-simd-vector-traits

Conversation

@ejmahler

@ejmahler ejmahler commented Sep 28, 2026 •

Copy link
Copy Markdown
Owner

Closes #188

SseNum, NeonNum, etc are gone. SseVector, NeonVector, etc have been merged into SimdVector, so now it's just the one trait (with one minor exception).

SSE and Neon went smoothly. When I converted wasm simd and fcma, I found that they bounced too freely between the wrapped types and the bare metal types, and it made it difficult to apply simple find and replace type operations. I ended up converting them both to use the wrapper in all places. That was quite painful, but hopefully it will pay off in the form of reduced tech debt.

I also eliminated the per-architecture array traits, and replaced them with a generic trait called SimdComplexArray and SimdComplexArrayMut. I redid some of the naming conventions while I was in here, mainly to reduce wordiness of common operations.

I ended up keeping the FcmaVector trait, but it only has the rotate_and_add etc methods that are unique to fcma. There's one more thing to resolve here, which is that I put unimplemented!() on the fcma apply_rotate90 and nmadd methods. They're only used by butterflies and will never be used by generic code, so I'm questioning whether they belong in the SimdVector trait at all. In the case of fcma, the reason i paid attention to it is because if you call apply_rotate90 you're probably leaving performance on the table and should probably be calling something that leads to fcma instructions and applying the rotations that way.

It would be fine to just check it in how it is as a little tech debt thing, but I'll sleep on it and see if I come up with a decision for how to handle it.

@ejmahler

Copy link
Copy Markdown
Owner Author

One more thing that could be done here: We have macros in each of the architectures to make it easier for the butterflies to load big chunks of data, with different strides etc, and they're all identical, so they could be combined too.

@ejmahler

ejmahler commented Sep 28, 2026 •

Copy link
Copy Markdown
Owner Author

I decided not to do anything on either front for now.

For the macros, there's more refactoring I want to do here - for example, these could be methods on SimdComplexArrayMut, at least some of them. But until then, they're not doing any harm sitting where they are.

For the trait, the only math ops that need to go in the SimdVector traits are the ones needed by the generic algorithms. Mostly that's just mul_complex, but a hypothetical SimdRadersAlgorthim would need an add(), and a conjugate or multiply-and-conjugate. I'd argue for even removing make_rotation, apply_rotation, and the rotation associated type, and instead introduciong a butterfly4 opaque type.

The reason why I'm not doing this now is that I'm cooking up a longer term plan for how to arrange these non-generic math operations. The main benefit of having a math op in the trait is to make it generic over f32 and f64. If a math op is only used by butterflies, we're not using that feature at all, so why pay for it? With the status quo it doesn't matter either way, which is why they're inside of traits. But if we move math ops out of the trait, they can be marked with target_feature. So if we move the butterfly-only math ops out of traits, that plus const generic array operations could pave the way towards our simd butterflies being 100% safe.

That's obviously a completely unrelated task to this, so I'm just going to leave SimdVector how it is for now.

@ejmahler
ejmahler marked this pull request as ready for review September 28, 2026 22:31
@ejmahler
ejmahler merged commit 5ba6f53 into master Sep 28, 2026
21 checks passed
@ejmahler
ejmahler deleted the merge-simd-vector-traits branch September 28, 2026 22:39
@ejmahler
ejmahler restored the merge-simd-vector-traits branch September 29, 2026 02:27
HEnquist added a commit to HEnquist/RustFFT that referenced this pull request Oct 1, 2026
The merged SimdVector trait from ejmahler#197 already has load1_lo_complex and
store1_lo_complex, with load1_lo and store1_lo wrappers in simd_array.rs,
so the partial column tail uses those and this branch no longer adds any
trait methods or backend impls of its own.

The cross layer tail now runs after ejmahler#197's unroll / no-unroll split, so
both paths get it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consolidate the per-backend vector traits into SimdVector

1 participant