-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
simd intrinsics with mask: accept unsigned integer masks, and fix some of the errors #137953
base: master
Are you sure you want to change the base?
Conversation
r? @Nadrieril rustbot has assigned @Nadrieril. Use |
The Miri subtree was changed cc @rust-lang/miri Some changes occurred in compiler/rustc_codegen_gcc Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr Some changes occurred to the platform-builtins intrinsics. Make sure the cc @antoyo, @GuillaumeGomez, @bjorn3, @calebzulawski, @programmerjake |
This comment has been minimized.
This comment has been minimized.
48bc564
to
26cb001
Compare
@bjorn3 anything we have to do here in cranelift? |
26cb001
to
fa396bc
Compare
r? codegen |
fa396bc
to
edadfc4
Compare
Some changes occurred in compiler/rustc_codegen_ssa |
It's not clear at all why the mask would have to be signed, it is anyway interpreted bitwise. The backend should just make sure that works no matter the surface-level type; our LLVM backend already does this correctly. The note of "the mask may be widened, which only has the correct behavior for signed integers" explains... nothing? Why can't the code do the widening correctly? If necessary, just cast to the signed type first...
Also while we are at it, fix the errors. For simd_masked_load/store, the errors talked about the "third argument" but they meant the first argument (the mask is the first argument there). They also used the wrong type for
expected_element
.I have extremely low confidence in the GCC part of this PR.
See discussion on Zulip