Correct the simd_masked_{load,store} intrinsic docs #119203
Conversation
|
r? @thomcc (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
| @@ -243,12 +243,13 @@ extern "platform-intrinsic" { | |||
| /// | |||
| /// `T` must be a vector. | |||
| /// | |||
| /// `U` must be a vector of pointers to the element type of `T`, with the same length as `T`. | |||
| /// `U` must be a pointer to the element type of `T` | |||
There was a problem hiding this comment.
Why not make the argument of type *const U then? It's not necessary to make it so generic.
Same for the store operation.
There was a problem hiding this comment.
I think this is a good idea. I'll explore this
There was a problem hiding this comment.
So this is left to a future PR?
There was a problem hiding this comment.
Yeah, I think we'll need to change how we validate these monomorphizations and there will be an opportunity when we move these checks out of the codegen crate.
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
@rustbot ready I will squash the commits when it's approved |
rustbot
added
S-waiting-on-review
|
LGTM, r=me after squashing. @bors delegate+ |
thomcc
added
S-waiting-on-author
|
I'm going to be away for a few months, so I'm rerolling my PRs so that folks don't have to wait for me. Sorry/thanks. r? libs |
farnoy
force-pushed
the
simd-masked-intrinsic-docfix
branch
from
77c3d65 to
14a4551
Compare
|
Some changes occurred to the platform-builtins intrinsics. Make sure the cc @antoyo, @GuillaumeGomez, @bjorn3, @calebzulawski, @programmerjake |
bors
removed
the
S-waiting-on-author
bors
added
the
S-waiting-on-bors
|
@bors rollup=always (docs-only change) |
| /// | ||
| /// `V` must be a vector of integers with the same length as `T` (but any element size). | ||
| /// | ||
| /// For each element, if the corresponding value in `mask` is `!0`, read the corresponding | ||
| /// pointer from `ptr`. | ||
| /// pointer offset from `ptr`. | ||
| /// The first element is loaded from `ptr`, the second from `ptr.wrapping_offset(1)` and so on. |
There was a problem hiding this comment.
i'd expect it to use offset, not wrapping_offset
There was a problem hiding this comment.
The entire point of this is to be able to mask off loads that would be out-of-bounds. If it used offset, that would defeat the purpose.
There was a problem hiding this comment.
The entire point of this is to be able to mask off loads that would be out-of-bounds. If it used
offset, that would defeat the purpose.
no, because offset is only called for those indexes that are not masked off -- sort-of. LLVM IR semantics are quite lacking here:
The ‘llvm.masked.load’ intrinsic is designed for conditional reading of selected vector elements in a single IR operation. It is useful for targets that support vector masked loads and allows vectorizing predicated basic blocks on these targets. Other targets may support this intrinsic differently, for example by lowering it into a sequence of branches that guard scalar load operations. The result of this operation is equivalent to a regular vector load instruction followed by a ‘select’ between the loaded and the passthru values, predicated on the same mask. However, using this intrinsic prevents exceptions on memory access to masked-off lanes.
There was a problem hiding this comment.
no, because offset is only called for those indexes that are not masked off
Even then I couldn't use this to do loads where the first elements are out of bounds.
The LLVM docs are unfortunately unclear on whether the pointer arithmetic has "inbounds" semantics or not, but the note about suppressing exceptions seems to indicate that out-of-bounds should be allowed.
There was a problem hiding this comment.
I'm proposing to clarify the LangRef: llvm/llvm-project#82469.
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#119203 (Correct the simd_masked_{load,store} intrinsic docs) - rust-lang#121277 (Refactor trait implementations in `core::convert::num`.) - rust-lang#121322 (Don't ICE when hitting overflow limit in fulfillment loop in next solver) - rust-lang#121323 (Don't use raw parameter types in `find_builder_fn`) - rust-lang#121344 (Expand weak alias types before collecting constrained/referenced late bound regions + refactorings) - rust-lang#121350 (Fix stray trait mismatch in `resolve_associated_item` for `AsyncFn`) - rust-lang#121352 (docs: add missing "the" to `str::strip_prefix` doc) Failed merges: - rust-lang#121340 (bootstrap: apply most of clippy's suggestions) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#119203 - farnoy:simd-masked-intrinsic-docfix, r=RalfJung Correct the simd_masked_{load,store} intrinsic docs Explains the uniform pointer being used for these two operations and how elements are offset from it.
Explains the uniform pointer being used for these two operations and how elements are offset from it.