Skip to content

Internal SIMD layout specification is not flexible enough for SPIR-V #130405

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
fu5ha opened this issue Sep 15, 2024 · 4 comments
Open

Internal SIMD layout specification is not flexible enough for SPIR-V #130405

fu5ha opened this issue Sep 15, 2024 · 4 comments
Labels
A-align Area: alignment control (`repr(align(N))` and so on) A-repr Area: the `#[repr(stuff)]` attribute A-SIMD Area: SIMD (Single Instruction Multiple Data) PG-portable-simd Project group: Portable SIMD (https://github.com/rust-lang/project-portable-simd) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@fu5ha
Copy link
Contributor

fu5ha commented Sep 15, 2024

In rust-gpu we implement a codegen backend for SPIR-V.

SPIR-V supports vector types, which we currently model by analogue of Rust Abi::Vector types, i.e. #[repr(simd)]. However, SPIR-V supports vector types of the same size that have a different representation depending on their element, which is incompatible with how Rust currently handles vector type data layouts.

Base SPIR-V Vector Layout

The default representation of SPIR-V vectors is specified in section 2.2.2 and 2.18.1 of the SPIR-V spec. This default representation is effectively an align of the underlying element type and size of element_size * count. As specified in section 2.16, the count is limited to 2, 3, or 4 elements, unless certain capabilities are enabled which allow 8 and 16 element vectors as well.

Extra requirements

However, the actual layout requirements of vector types change and can get rather messy depending on where and how they are used. As far as the SPIR-V spec itself is concerned, the above are the only rules. In practice, you must run SPIR-V code through a client implementation of some graphics API. For Vulkan, this adds extra rules for how you can load vector types from different kinds of buffers. The rules are specified in the Vulkan specification here.

Basically, under so-called scalar alignment rules (the least restrictive), stuff in buffers behaves just like repr(C) layout, with vectors retaining the property of having the same alignment as their base element.

Under the more restrictive layout rules (i.e. older versions of the spec without extensions enabled), vectors start to behave more like on x86 or aarch64: two component vectors have an alignment of twice their base element, and 3 and 4 component vectors both have alignment of 4 times the element.

Something that is unclear to me is whether it's even sensical within Rust's type model to allow the same type to exhibit different alignment requirements in different locations. If not, it makes most sense to me to always follow the less restrictive model and force the programmer to respect the more restrictive rules manually where applicable.

Alternative solutions

We already have a set of #[spirv(X)] attributes. It would be possible, though more annoying and perhaps less flexible, for us to implement a #[spirv(vector)] attribute which handles things separately. Right now we are experimenting with basically doing this to patch internal simd vector layouting.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 15, 2024
@VorpalBlade
Copy link

If not, it makes most sense to me to always follow the less restrictive model and force the programmer to respect the more restrictive rules manually where applicable.

I don't know much about SPIRV, but what would the effects be of getting those rules wrong? Would it be unsound (cause UB) or just fail to compile or fail to run?

@workingjubilee workingjubilee added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-SIMD Area: SIMD (Single Instruction Multiple Data) labels Sep 15, 2024
@workingjubilee
Copy link
Member

I don't know much about SPIRV, but what would the effects be of getting those rules wrong? Would it be unsound (cause UB) or just fail to compile or fail to run?

In theory one of the latter should result. SPIRV does provide enough validation rules to declare this kind of problem incorrect at compile time.

In practice, we're talking about dynamic JITs that are assuming a valid output from a higher-level language's compilation, so... we should assume it would be UB.

@fu5ha
Copy link
Contributor Author

fu5ha commented Sep 16, 2024

In rust-gpu we run spirv-val and spirv-opt on emitted spir-v unless you specifically disable them, which detect the specific issues that arise for buffer block layout rules quite well

@workingjubilee
Copy link
Member

Oh good.

@saethlin saethlin added PG-portable-simd Project group: Portable SIMD (https://github.com/rust-lang/project-portable-simd) and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Sep 25, 2024
@workingjubilee workingjubilee added A-repr Area: the `#[repr(stuff)]` attribute A-align Area: alignment control (`repr(align(N))` and so on) labels Nov 1, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-align Area: alignment control (`repr(align(N))` and so on) A-repr Area: the `#[repr(stuff)]` attribute A-SIMD Area: SIMD (Single Instruction Multiple Data) PG-portable-simd Project group: Portable SIMD (https://github.com/rust-lang/project-portable-simd) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants