derive spirv vector on user structs

[Firestar99]

Requires #380

Since #380 already implements all the infrastructure to declare spirv vectors properly in glam, may as well expose it to our end users and allow them to declare their own vector types. The spirv_vector proc macro expands to the same rust_gpu::vector::v1 glam uses, but additionally implements spirv_std::vector::Vector and spirv_std::vector::VectorOrScalar on the type, so it can be used in eg. subgroup intrinsics.

Example:

rust-gpu/tests/compiletests/ui/glam/spirv_vector_macro.rs

Lines 13 to 33 in 6be1bff

	#[spirv_std::spirv_vector]
	#[derive(Copy, Clone, Default)]
	pub struct MyColor {
	pub r: f32,
	pub g: f32,
	pub b: f32,
	}
	#[spirv(compute(threads(32)))]
	pub fn main(
	#[spirv(storage_buffer, descriptor_set = 0, binding = 0)] input: &Vec3,
	#[spirv(storage_buffer, descriptor_set = 0, binding = 1)] output: &mut MyColor,
	) {
	let color = MyColor {
	r: input.x,
	g: input.y,
	b: input.z,
	};
	// any function that accepts a `VectorOrScalar` would do
	*output = subgroup_shuffle_up(color, 5);
	}

First two commits just move some code around, so best reviewed commit by commit.

close #410

[Firestar99]

Still a bit TBD around the proc macro name spirv_std::spirv_vector, should we just call it vector?

[nazar-pc]

Does it need to be a proc macro vs derive macro that would be more natural?

[Firestar99]

It's an attribute macro as we need to expand into another attribute macro that is placed on the struct directly, that our codegen backend can pick up.

[nazar-pc]

I see, what about requiring a user to specify it directly? Or maybe it is unnecessary in certain cases (like when I have a scalar new type rather than a vector)?

[Firestar99]

What exactly do you mean? Manually implementing Vector and VectorOrScalar on your type, instead of using the attribute macro? I also wrote entirely new docs for vector (mostly in #380 so it isn't obvious diff):

rust-gpu/crates/spirv-std/src/vector.rs

Lines 19 to 61 in 9ed3407

	/// Abstract trait representing a SPIR-V vector type.
	///
	/// To derive this trait, mark your struct with:
	/// ```no_run
	/// #[spirv_std::spirv_vector]
	/// # #[derive(Copy, Clone, Default)]
	/// # struct Bla(f32, f32);
	/// ```
	///
	/// This places these additional constraints on your type, checked by the spirv codegen:
	/// * must be a struct
	/// * members must be a spirv [`Scalar`] type, which includes:
	/// * Floating-point type: f32, f64
	/// * Integer type: u8, u16, u32, u64, i8, i16, i32, i64
	/// * Boolean type: bool
	/// * all members must be of the same primitive type
	/// * must have 2, 3 or 4 vector components / members
	/// * type must derive Copy, Clone, Default
	///
	/// The spirv codegen backend will then emit your type as an `OpTypeVector` instead of an `OpTypeStruct`. The layout of
	/// your type is unaffected, the size, alignment and member offsets will follow standard rustc layout rules. This hint
	/// does nothing on other target platforms.
	///
	/// See the SPIRV spec on [Types](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_types) and the
	/// "Data rules" in the [Universal Validation Rules](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_universal_validation_rules).
	///
	/// # Example
	/// ```no_run
	/// #[spirv_std::spirv_vector]
	/// #[derive(Copy, Clone, Default)]
	/// struct MyColor {
	/// r: f32,
	/// b: f32,
	/// g: f32,
	/// }
	/// ```
	///
	///
	/// # Safety
	/// Must only be implemented on types that the spirv codegen emits as valid `OpTypeVector`. This includes all structs
	/// marked with `#[rust_gpu::vector::v1]`, which `#[spirv_std::spirv_vector]` expands into or [`glam`]'s non-SIMD
	/// "scalar" vector types use directly.
	pub unsafe trait Vector<T: Scalar, const N: usize>: VectorOrScalar<Scalar = T> {}

[nazar-pc]

I guess there is a reason it works that way right now, but that is (generally) not the only way.

First of all, I not only may want to derive an impl of a trait for vectors, but also for single-field structs, in which case I would not call it a vector.

Second, I'd expect it to look something like this for the case when multiple fields are present instead:

#[derive(VectorOrScalar)]
#[repr(C)]
pub struct New(u32, u32);

If #[repr(C)] is not appropriate, Rust has #[repr(simd)], which I know very little about beyond the fact that it exists. Maybe #[repr(vector)] can be introduced if semantic meaning is different here.

Then macro would fail to compile if the struct or one of its fields doesn't have the necessary #[repr()] defined.

[Firestar99]

On single-field structs: I've recently noticed that structs with a single scalar member and #[transparent] do compile down to simply that scalar member. So you could just impl Scalar on such a struct and it should work, or fail validation. (I would not expect any actual UB to stem from this, validation should easily catch that)
So we could also offer a spirv_scalar (derive / attribute) macro that checks all the preconditions and impl Scalar and VectorOrScalar for it. But first, I would want to double check why we emit the scalars for these in the first place, in case there is some special requirement or such.

#[repr(simd)]: That has bitten us before, I'd rather not touch repr SIMD again and instead use attribute macros we define instead. Since repr SIMD is still unstable, rustc has changed what this repr means between versions, so we had to hack our way out of it before. #380 removes those hacks and replaces all usages of #[repr(simd)] with the new attribute macro.

attribute vs derive macro: It's mainly a technical reason it's an attribute macro. If we wanted to have the impl Vector as a derive macro, you'd still have to add the attribute macro for our codegen backend. So why duplicate things, when we could instead have one attribute macro for both?

[nazar-pc]

If we wanted to have the impl Vector as a derive macro, you'd still have to add the attribute macro for our codegen backend. So why duplicate things, when we could instead have one attribute macro for both?

My point is that the information for codegen should not be a proc macro either. If #[repr(simd)] doesn't work, make it #[repr(vector)] then or something like that that makes more sense. This is how idiomatically we do it in other cases in Rust.

As for the derive, it is also idiomatic to have the name of the trait and derive macro be the same and re-exported together. Having a separate proc macro looks more unusual, implicitly indicating complexity and is not as clear about what does it actually do. While derive macros look absolutely normal since everyone uses them all the time.

The derive macro could be called VectorOrScalar since that is what APIs require, internally it'd check that the struct either has one field with #[repr(transparent)] or #[repr(C)] (if/when supported) or it has #[repr(vector)] and multiple same type fields inside. It'll then implement both Scalar/Vector and ScalarOrVector for the type.

[Firestar99]

I can see how #[repr(vector)] seems to make a lot of sense from the user's perspective. But from the codegen perspective, it very much does not:

• Adding support for a custom repr is not just really hard, you'd have to change some of the fundamental structs deep within rustc.
• But also really error prone, as we have seen before #380 with glam vector having different representations on CPU and GPU. I really don't want to mod in a custom representation and have to deal with this, if a simpler alternative like an attribute macro is available.
• It also doesn't map to SPIRV at all, since the decision of emitting an OpTypeStruct or an OpTypeVector is completely orthogonal to the abi layout of a type. This may be confusing at first from a CPU perspective, but SPIRV doesn't actually require structs or vectors to have an abi layout, which defines the size, alignment and member offsets. Since GPUs are largely a register-based architecture, most of them simply don't support pointers pointing to registers, so having an abi layout for register-only types that can't be pointed to doesn't make much sense to begin with. It only requires an abi layout if said struct is read from or written out to a buffer, since other shaders or the CPU must agree on a layout there. The only requirement for the abi layout is that it should be sane (eg. no overlapping members, no member oob wrt. the struct size), otherwise, it does not care for how you layout your types.

So types really should be #[repr(C)] both on the CPU and GPU side, to denote their abi layout. And since the decision of whether a struct is a standard OpTypeStruct or wants to opt into OpTypeVector is completely orthogonal to it's abi layout, I think it's best to just have it be a separate optional attribute macro. And it can't be a derive macro, since our codegen backend needs it to be an attribute macro on the struct itself.

[nazar-pc]

Thanks for explaining!

The reason I imagined it as derive macro is because APIs require ScalarOrVector implementation and it is counter-intuitive/non-idiomatic when implementation of it is done using something other than derive macro. But I also understand that there are other limitations that might dictate the design here.

I'm now wondering if there is an API to do something like parsing the file with syn and replacing #[repr(vector)] with #[repr(C)] #[spirv_std::spirv_vector] for example, so that it looks more natural to the user and is easier to support for the codegen backend at the same time. I have no idea if such API exists, but it'd be neat if it did.

As for derive macro, I still don't fully get why it can't be a thing. Derive macro should literally just implement a trait properly. If there are extra requirements for that trait to be derivable (like #[repr] attribute or #[spirv_std::spirv_vector] attribute) then it should be checked by the macro and generate compilation error if something doesn't match expectations, but it is still orthogonal to trait implementation as such. In fact it is possible that no extra attributes are needed for non-vector new types like struct S(u32).

[Firestar99]

I've been thinking about this PR a bit more while waiting for #380 to be reviewed, and I have to say I may prefer the derive macro alternative. Specifically, I like that we can #[derive(Scalar)], which can check that the struct is #[repr(transparant)] and unsafe impl Scalar for T where Primitive: Scalar. So why not mirror this for vectors as well, and have a derive macro that checks for #[spirv::vector::v1] and impl Vector for T.

I think this can coexist with the "subgroup trait + macro" I mentioned at #410 (comment), since there are subgroup APIs that specifically require a float or int Scalar or Vector. Whereas the other would be better for shuffles, boardcasts and the like.

[Firestar99]

This PR needs a rethink on how it relates to #441.

• Does 441 supersede this PR?
• Or if it could be an extension of it, to allow using vectors in places where ScalarOrVectorComposite is unsuited for?