compiler: add simd_ctpop intrinsic #125266
Conversation
|
Some changes occurred to the platform-builtins intrinsics. Make sure the cc @antoyo, @GuillaumeGomez, @bjorn3, @calebzulawski, @programmerjake |
f8410db to
1914c72
Compare
Yes, that's the idea. |
|
r? @RalfJung looks good to me |
|
I presume it's implausible for this intrinsic to have a fallback body because we can't actually write the generic bound? Can we then add Miri support? I feel like it shouldn't be too hard, the boilerplate is already reusable in this function: rust/src/tools/miri/src/intrinsics/simd.rs Lines 18 to 20 in bfa3635 |
I can add Miri support yeah. I'm not sure what adding a fallback body would look like here, it might be possible to twist the code such that it actually works, tbh. |
|
@saethlin hmm so wait I looked around and didn't see a magic wand to wave... am I supposed to just, for a fallback body, literally write a body for the intrinsic? lemme think about that, it might work. |
|
hm. I know how to add the appropriate generic bounds, they just might make someone mad. |
|
Happy to review a Miri implementation, but LLVM codegen is not something I can confidently review. We haven't yet used a fallback body for any SIMD intrinsic so maybe this is not the right time and place for that experiment. :) (It could delay the PR quite a bit, trying to get that to work.) |
|
Would you mind adding cg_clif support too at ? Cranelift calls the instruction that needs to be used here popcnt: |
They will leak into the |
Ehn, we already have the relevant bounds or approximately so. As Ralf said, it would delay things significantly, that's the real reason. Yeah, I noticed that I'll add the followups. |
|
Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 The Miri subtree was changed cc @rust-lang/miri |
|
I briefly glanced at cg_gcc but I do not understand the relevant code for the popcount implementation, which appears to have many edge-cases, and does not seem to dispatch to the fast vpopcount builtins anyways, only doing that for pattern-matching LLVMIR intrinsics. I hope it won't be too hard to add, but my apologies. |
|
Neat, thanks for the Miri implementation. |
|
r=me on the Miri implementation and test, thanks. :) |
| let fn_ty = bx.type_func(&[vec_ty, bx.type_i1()], vec_ty); | ||
| let dont_poison_on_zero = bx.const_int(bx.type_i1(), 0); |
There was a problem hiding this comment.
Shouldn't this be called poison_on_zero, which we are then setting to false?
There was a problem hiding this comment.
hmmm, plausible. I was going for a "nah nah" instead of a "yeah nah".
|
@bors r=RalfJung,nikic |
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#124570 (Miscellaneous cleanups) - rust-lang#124772 (Refactor documentation for Apple targets) - rust-lang#125011 (Add opt-for-size core lib feature flag) - rust-lang#125218 (Migrate `run-make/no-intermediate-extras` to new `rmake.rs`) - rust-lang#125225 (Use functions from `crt_externs.h` on iOS/tvOS/watchOS/visionOS) - rust-lang#125266 (compiler: add simd_ctpop intrinsic) - rust-lang#125348 (Small fixes to `std::path::absolute` docs) Failed merges: - rust-lang#125296 (Fix `unexpected_cfgs` lint on std) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#125266 - workingjubilee:stream-plastic-love, r=RalfJung,nikic compiler: add simd_ctpop intrinsic Fairly straightforward addition. cc `@rust-lang/opsem` new (extremely boring) intrinsic
…, r=RalfJung,nikic compiler: add simd_ctpop intrinsic Fairly straightforward addition. cc `@rust-lang/opsem` new (extremely boring) intrinsic
Fairly straightforward addition.
cc @rust-lang/opsem new (extremely boring) intrinsic