Port to fearless_simd

[Shnatsel]

No description provided.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.88%. Comparing base (0f47ea1) to head (b8ebaa5).

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #58      +/-   ##
==========================================
+ Coverage   99.82%   99.88%   +0.06%     
==========================================
  Files          13       12       -1     
  Lines        2261     2711     +450     
==========================================
+ Hits         2257     2708     +451     
+ Misses          4        3       -1     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Shnatsel]

On Zen4 This gives up to 7% penalty due to not utilizing AVX-512, but otherwise looks normal. We don't need explicit mul_neg_add on x86 it seems, this is lowered into the correct instruction automatically.

On Apple M4 this is a large regression. The hottest instructions are loads/stores to/from the stack for f32x16, so it might be due to register pressure or some such (LLVM isn't great at dealing with that). I'll need to investigate how wide lowers this kind of thing to NEON, its approach is apparently better than that of fearless_simd. Or we could rewrite the function to operate on native vectors but then we might give up some ILP?

[valadaptive]

On Apple M4 this is a large regression. The hottest instructions are loads/stores to/from the stack for f32x16, so it might be due to register pressure or some such

This is a wild guess (I don't have Apple Silicon hardware, so I can't benchmark any of this), but the way you're loading from a slice looks a bit convoluted. Instead of e.g.

let in0_re = f32x4::simd_from(simd, <[f32; 4]>::try_from(&reals_s0[0..4]).unwrap());

have you tried simply:

let in0_re = f32x4::from_slice(simd, &reals_s0[0..4]));

Also just to confirm, you ran this with the latest fearless_simd from Git, correct? linebender/fearless_simd#159 aimed to improve codegen around SIMD loads, and linebender/fearless_simd#181 just landed a couple days ago and adds (potentially) faster methods for SIMD stores.

[Shnatsel]

Yep, this is on latest fearless_simd from git. I'll see if from_slice does anything, it's certainly more readable.

I've also tried swapping vector repr from arrays to structs to mimic wide internal representation but it didn't make a difference.

[Shnatsel]

CI is broken in a really interesting way: it complains about mul_neg_add which doesn't appear anywhere in the code on the latest commit. It's either running on an old commit or on a different branch; either way that could be exploitable if it can be reproduced.

[Shnatsel]

Nope, no difference in performance from changing loads/stores. Looks like a readability win to me though.

[Shnatsel]

I had Claude analyze the generated assembly from fft_dit_chunk_32_simd_f32, and it seems that the regression is due to over-eager inlining: https://claude.ai/share/920afa1e-8e39-48a1-b00f-1a55d6c25d1c

Load/store instructions are hot on the profiler (samply), so this makes sense as a regression due to stack spills. This also explains why all the kernels are gone from cargo asm view - they've all been aggressively inlined.

This is likely an artifact of fearless_simd's #[inline(always)]. We go though vectorize() to add a function call that isn't force-inlined but apparently that's not enough.

[valadaptive]

This is likely an artifact of fearless_simd's #[inline(always)]. We go though vectorize() to add a function call that isn't force-inlined but apparently that's not enough.

I wonder why it's making a different inlining decision. Was the function marked #[inline(never)] before?

I think we should probably just make vectorize not be inline over in fearless_simd.

[Shnatsel]

Nope, preventing inlining didn't help performance. It seems functions that operate on 512-bit vectors like f32x16 are still much slower than with wide.

[Shnatsel]

Now that both aren't inlined, I can directly compare the generated assembly. This is almost the first time I see ARM assembly but what jumps out at me is that wide doesn't use the fmls instruction, it always does fmla followed by a fneg which is two instructions to do the same thing, and it's still much faster somehow.

Assembly of an affected function fft_dit_chunk_64_simd_f32 lifted from samply assembly view:

wide: wide_asm.txt

fearless_simd: fearless_asm.txt

Profiling data

Profile with wide (on main, exact commit): https://share.firefox.dev/3LNEZto

Profile with fearless_simd (#58, exact commit): https://share.firefox.dev/3LVrB6x

Both recorded with samply record -r 5000 cargo bench --profile=profiling --bench=bench 'Forward f32/PhastFT DIT/64'

[Shnatsel]

I can't really read Aarch64 assembly but Claude can, and Claude has an idea on what's going on: https://claude.ai/share/4fa3af4f-3c34-4d57-b11d-3611385f4f1c

That explains a lot if it's true

[valadaptive]

Try linebender/fearless_simd#184.

[Shnatsel]

linebender/fearless_simd#184 is a +20% boost on Apple M4! Thanks a lot!

This still isn't as fast as wide but that's a big improvement!

[Shnatsel]

New profile for fearless_simd: https://share.firefox.dev/3LOGBDc

New assembly for fft_dit_chunk_64_simd_f32: asm_64_f32_fixed_loads_asm.txt

[Shnatsel]

It looks like the load-store-load pattern for load_array is present even when loading from actual arrays that are just constants: https://claude.ai/share/4fa3af4f-3c34-4d57-b11d-3611385f4f1c

@valadaptive you'll probably want to drop load_array entirely and always use the implementation of load_array_ref on Aarch64 even for on-stack arrays because rustc doesn't align stack arrays with this load in mind

[Shnatsel]

I was wrong about wide being less efficient at computing FMA. The math instructions are equally efficient - there is no single-instruction version of mul_sub() in NEON, you need to do either vfmaq_f32(vnegq_f32(a), b, c) or vnegq_f32(vfmsq_f32(a, b, c)) but there's no single-instruction version.

[valadaptive]

It looks like the load-store-load pattern for load_array is present even when loading from actual arrays that are just constants: https://claude.ai/share/4fa3af4f-3c34-4d57-b11d-3611385f4f1c

@valadaptive you'll probably want to drop load_array entirely and always use the implementation of load_array_ref on Aarch64 even for on-stack arrays because rustc doesn't align stack arrays with this load in mind

Can you share the LLVM assembly? I think if you include RUSTFLAGS="--emit=llvm-ir", it should show up somewhere in target/release/deps.

The constant array loads are inlined, and I find it very strange that LLVM is incapable of eliminating a redundant load/store of a simple constant.

It's hard to find any information about this online, but I cannot find any reference to the ld1 instruction requiring the address to be aligned, and the information I can find seems to indicate that NEON can do unaligned loads and stores just fine. I suspect Claude may be making this up.

I did notice that the f32x16 array load function maps to vld1q_f32_x4, which Rust lowers to the LLVM intrinsic llvm.aarch64.neon.ld1x4.v4f32.p0. The issue may be that LLVM lowers that particular intrinsic poorly, or does so very late in the pipeline. I'll have to check; it may be better to just use four 128-bit loads.

[valadaptive]

Try it with linebender/fearless_simd#185.

[Shnatsel]

After switching over to linebender/fearless_simd#185 I had to double-check my results, re-measure the baseline and run tests, because the benchmarks look too good to be true!

fearless_simd is now 10% to 40% faster than wide!

It's hard to find any information about this online, but I cannot find any reference to the ld1 instruction requiring the address to be aligned, and the information I can find seems to indicate that NEON can do unaligned loads and stores just fine. I suspect Claude may be making this up.

Yep, I agree that's probably not the cause of the spill. Looking at the code that implements it, I suspect it's Aligned512 that's to blame: crate::support::Aligned512(vld1q_f32_x4(val.as_ptr() as *const _)) }

I'll prepare a branch without the aligned wrappers and measure that for comparison.

[Shnatsel]

I'll prepare a branch without the aligned wrappers and measure that for comparison.

Branch up: https://github.com/Shnatsel/fearless_simd/tree/no-align-wrapper-in-loads

But nope, no improvement from that. It seems transmute_copy really is just faster than the intrinsics.

[Shnatsel]

Here's the LLVM IR you asked for:
phastft-fearless-main-commit-744661d.ll.gz
phastft-transmute-copy-loads.ll.gz

[Shnatsel]

FWIW I showed this to an LLVM developer and he said

that 'save to stack and immediately load back to registers' thing could be an escape hatch for when the compiler needs to bitcast from one type to another but doesn't know how

[Shnatsel]

The performance regression is resolved upstream in linebender/fearless_simd#184 and linebender/fearless_simd#185, this should be ready to merge