[Migrated] Move Output vars to return position (was: Storage class inference interface)

Question

[Migrated] Move Output vars to return position (was: Storage class inference interface)

Opened this issue a year ago · 8 comments

Issue automatically imported from old repo: EmbarkStudios/rust-gpu#416
Old labels: mcp: accepted
Originally creatd by khyperia on 2021-02-11T13:41:36Z

Storage class inference is now a thing as of #300/#414. This details the design for the interface system taking advantage of inference, pointing out all the rules of how storage classes are specified in entry points.

There are a handful of ways of specifying a storage class:

direct attribute: fn main(#[spirv(input)] x: &f32). This uses the current list of storage classes.
via builtin: fn main(#[spirv(position)] x: &Vector3) (inferred as input). This uses the current list of builtins (a table is needed to map builtin to storage class, I think some are input, some are output, and there may be others)
via image: fn main(x: &Image2d) (inferred as uniform_constant). I thiiink images are always uniform_constant and not uniform, but I'm not sure.
unspecified future inference rules that we haven't discovered yet (the spec is light on what goes in what storage class), suggestions are welcome

If exactly one of these rules matches, then use the storage class it specifies.

If more than one of these rules matches, and they all compute the same storage class, emit a warning (e.g. #[spirv(position, input)], warn on input and say it's redundant). If more than one of these rules matches, and they compute different storage classes, emit an error.

If none of these rules matches, then we have an open design question. The options here are a trade-off between catching user errors that may be difficult to diagnose/guiding users with explicit syntax suggestions, and not annoying people with overly explicit syntax that takes a while to type out and read.

fn main(x: &f32) -> is this an error, or does it default to input? (or something else)
fn main(x: &Struct) -> is this an error, does it default to input, or does it default to uniform? (or something else)
fn main(x: &mut f32) -> is this an error, or does it default to output? (or something else)

For 2, I don't know if it's valid to have a struct (or any other non-scalar) be an input/output variable, more research is needed.

An alternative is to keep the current system of Input<T> and friends. We would remove the .load() and .store() methods entirely, and implement Deref/DerefMut (when applicable) for them. I much prefer the readability, recognizability, and usability of using plain references, but I understand others don't feel the same way~

Answer 1 · 2024-11-13T14:53:31.000Z

Comment from eddyb (CONTRIBUTOR) on 2021-02-11T14:53:59Z

via builtin: fn main(#[spirv(position)] x: &Vector3) (inferred as input). This uses the current list of builtins (a table is needed to map builtin to storage class, I think some are input, some are output, and there may be others)

Some can be either, but I think the best default would be to picking input vs output based on the kind of reference, and use attributes to override that - we could also support outputs via returns, tho multiple/named outputs would require a struct.

Answer 2 · 2024-11-13T14:53:42.000Z

Comment from eddyb (CONTRIBUTOR) on 2021-02-11T15:53:29Z

For Vulkan 1.2, if anyone is curious, the valid Storage Classes for each builtin are listed here:
https://www.khronos.org/registry/vulkan/specs/1.2/html/vkspec.html#interfaces-builtin-variables

Not sure if the information is available in a machine-readable format (haven't checked yet), but Ctrl+F Storage Class can quickly highlight that each builtin varies with Execution Model, usually either Input or Output, but in some cases both are allowed.

Answer 3 · 2024-11-13T14:53:53.000Z

Comment from Hentropy (CONTRIBUTOR) on 2021-02-11T16:09:06Z

How would a function like this be handled? Compiler error I guess?

fn shader(#[spirv(input)] foo: &mut Vec4) {}

How does the SampleMask builtin work? It can be either Input or Output into a frag shader, so will it always need to be specified? Maybe Input is a sensible default, and it only needs to be specified if you want Output (or vice versa)?

Output is uninitialized, so reading before writing is bad. How will this be prevented?

(As you both know, I prefer a type based syntax for this because I think it's easier to read and write, more idiomatic, and will help with implementing safe CPU-GPU interop)

Answer 4 · 2024-11-13T14:54:04.000Z

Comment from eddyb (CONTRIBUTOR) on 2021-02-11T16:17:58Z

Output is uninitialized, so reading before writing is bad. How will this be prevented?

Oh right, that's where I was suggesting &mut MaybeUninit<T> on Discord for Output.

Alternatively, we could return the value from the entry-point - this gets more interesting with multiple outputs (which may be builtins so they would need a place to attach attributes to anyway) - probably be the best thing in that case is returning a struct.

Answer 5 · 2024-11-13T14:54:15.000Z

Comment from XAMPPRocky (MEMBER) on 2021-02-11T17:13:54Z

Oh right, that's where I was suggesting &mut MaybeUninit on Discord for Output.

Small note that, that is blocked on #415

Answer 6 · 2024-11-13T14:54:26.000Z

Comment from eddyb (CONTRIBUTOR) on 2021-02-11T17:16:47Z

Based on EmbarkStudios/rust-gpu#415 (comment) I think we're fine - I doubt ZST inputs/outputs would be common, if at all supported by SPIR-V and/or Vulkan.

Answer 7 · 2024-11-13T14:54:37.000Z

Comment from msiglreith (CONTRIBUTOR) on 2021-02-11T18:01:38Z

Alternatively, we could return the value from the entry-point - this gets more interesting with multiple outputs (which may be builtins so they would need a place to attach attributes to anyway) - probably be the best thing in that case is returning a struct.

This would be nice imo as it allows to default to input storage class for every parameter and output for the return type. (Feels also a bit nicer to write!)

Answer 8 · 2024-11-13T14:54:48.000Z

Comment from Hentropy (CONTRIBUTOR) on 2021-03-02T02:11:52Z

How would descriptor indexing work?

#[spirv(uniform, descriptor_set = 0, binding = 0)] way_1: [&T],
#[spirv(uniform, descriptor_set = 0, binding = 0)] way_2: &[T],
#[spirv(uniform_array, descriptor_set = 0, binding = 0)] way_3: &T,
#[spirv(uniform_array, descriptor_set = 0, binding = 0)] way_4: &[T],
#[spirv(uniform_array, descriptor_set = 0, binding = 0)] way_5: &[&T],

way_1 is not valid Rust, function parameters must be sized.
way_2 is ambiguous: is it a runtime descriptor array or a variably sized buffer?
way_3 does not require indexing before accessing the pointer.
way_4 requires T: Sized and makes the runtime descriptor array a fat pointer with an unknown size.
way_5 introduces an extra pointer that does not exist and makes the runtime descriptor array a fat pointer with an unknown size.

Descriptor array bindings are created in Vulkan by specifying a count > 1 in VkDescriptorSetLayoutBinding. Runtime descriptor array bindings additionally require the appropriate flag to be set in VkDescriptorSetLayoutBindingFlagsCreateInfo.