Repository navigation
Stable simd - #379
Stable simd#379
Conversation
253c2af to
4807921
Compare
|
Measured this stack (#379 through #383, head fec473a) on a Zen 5 and read the generated x86 code, in case it helps review. Short version: it builds and passes tests on stable 1.98 and 1.99 with NumbersEPYC 9845,
Workloads dominated by run and bitmap containers (chains of What the binary does on Zen 5From
Two codegen observations
Authored by Claude (claude-fable-5-1) at Evan Schwartz's request. |
|
I reordered the bitonic merge call in xor to match or: passing the newly loaded vector as the second argument (which gets reversed) rather than the vector carried from the previous step, this both keeps the loop carried dependency chain shorter, and seems to also avoid pulling out individual lanes in the loop as well. |
|
@Kerollmops I kinda think we should just always enable simd, make the |
There was a problem hiding this comment.
Hey @Dr-Emann and @emschwartz 👋
Thanks you very much for the work. It seems much better than depending on the standard SIMD unstable API, at least for now.
I was wondering about the compatibility with ARM machines and if we are testing this target in the CI. At Meilisearch we are running on Linux ARM instances and I would like to ensure that we are not breaking anything.
Should we enable simd by default?
I think we should enabled SIMD by default if and only if it doesn't impact compilation on any tier 1 targets. I suppose not but it would be preferable to ensure that in the CI. @emschwartz do you think you can review the generated instruction for Linux ARM for both version and run couple benchmarks on an ARMv8, ARMv8.2-a, ARMv9 CPUs, please?
I can see an Aarch64 NEON baseline support is available but I'm not sure about what it means for ARMv8+...
Should we even have a simd feature (or, still have the feature, but have it do nothing, and always enable simd)?
[..]
I kinda think we should just always enable simd, make the simd feature a no-op.
I'm wondering if it would make any sense to be able to completely disable SIMD instructions these days apart from compiling for small architectures like WASM but it seems that fearless_simd is already compatible with it, anyway. So, I'm in favor of removing the simd feature if it is doesn't even have an impact to disable this feature. It's breaking to remove a feature but I prefer that than hiding that the simd feature is a no-op.
@Dr-Emann Would it be possible to rebase the whole PR stack on top of main, please? There is a button for that but I prefer you to press it 😄
Thank you for the work @Dr-Emann and thanks for the little report @emschwartz 🙏
Have a nice day 🌵
CI builds with `-C target-cpu=native`, and without `--target`, Cargo applies `RUSTFLAGS` to proc macros too. The cached `fearless_simd_macros` proc macro was built on a runner with more CPU features than later runners which restored it from the cache, so rustc crashed with SIGILL when it loaded the macro. Cached dependencies could fail the same way at test time. Add a hash of the target features `-C target-cpu=native` enables (from `rustc --print cfg`) to the cache key, so a cache is only restored on runners with the same target features. Unlike hashing all of /proc/cpuinfo's flags, runners with different CPU models but the same target features still share a cache.
The `simd` feature now builds on stable Rust. The vectorized array store operations (or, and, xor, sub) are ported to fearless_simd with the same algorithms: fearless_simd needs either `std` or `libm`, so `libm` is enabled for no_std builds; with `std` enabled, fearless_simd uses std instead. Adds tests comparing each vectorized op against its scalar version at the baseline and detected SIMD levels. CI now checks and tests `simd` on stable, including no_std.
`255 - mask` and `mask ^ 0xFF` both invert a u8 bitmask; `!mask` says so directly.
|
Arm's naming is a bit confusing, but I'm pretty sure you're talking about aarch64: ARMv8 / v8.2-a / v9 are the 64-bit generations. ARMv8 does technically include an AArch32 mode, but nobody runs Linux servers that way. in practice "ARMv8+ server" means AArch64. The ARM tier-1 targets are all 64-bit (aarch64-unknown-linux-gnu, aarch64-apple-darwin, and so on). 32-bit ARM targets like armv7-unknown-linux-gnueabihf are tier 2. NEON is mandatory in AArch64, so the NEON baseline covers every 64-bit ARM CPU from v8.0 onward. v8.2 and v9 add optional extensions like dotprod, fp16, and SVE/SVE2. fearless_simd doesn't use those, and our u16x8 functions wouldn't need them anyway. |
|
Hey @Dr-Emann 👋 Thanks for the precision around the ARMv8 use. It's much clearer right now and I was indeed interested in NEON and AArch64 support. I would like to merge this PR as part of the PR stack but I would like to know if you plan to address the remaining comments I emitted. Like the one about bumping Have a nice day 🌵 |
fearless_simd works on stable, so there's no reason to keep it behind a flag anymore. Drops the scalar fallbacks in ArrayStore and the simd-only CI jobs. BREAKING CHANGE: the `simd` feature no longer exists.
This PR ports the experimental
simdfeature from nightly-onlycore::simdtofearless_simd.Most of the changes come from adapting the existing algorithms to the new API: vector types now carry a generic parameter that we have to carry through operations, helpers, and visitors.
The generics enable compiling implementations for multiple SIMD levels. With
std, runtime CPU detection selects the appropriate implementation on x86/x86-64, allowing a binary to use instructions beyond its compile-time target.no_stdbuilds continue to use compile-time target features.Also add tests comparing SIMD and scalar results, and updates CI to test SIMD on stable, with and without
std.BREAKING CHANGE: the
simdfeature no longer exists. Simd is now used unconditionally to try to make operations faster.Closes #373
Same as #378, but from a branch inside this repo so I can stack a little.
Open Questions
Should we enable simd by default?: Yes, we removed the simd feature entirely, and simd is now always enabled.