Provide shader inputs as arguments to entry point functions

[kvark]

This is a proposal to address #1155 .
Aligns well with #1171.

We should consider:

• putting the "in" variables into the arguments of functions
• making the "out" variables to be result values of the functions
• leaving the resources to be in variables

Here is how it may look like:

[[group(0), binding(0), uniform]] params : SimParams;

struct VertexInput {
  [[location(0)]] texCoords : vec2<f32>;
};
struct VertexOutput {
  [[builtin(position)]] position: vec4<f32>;
  [[location(0)]] colour : vec4<f32>;
};

[[stage(vertex)]]
fn vertex(input: VertexInput) -> VertexOutput {
 ...
}

[[stage(fragment)]]
fn fragment(
    input: VertexOutput,
) -> [[location(0)]] vec4<f32> {
}

[[stage(compute)]]
fn comp_main(
  [[builtin(invocation_id)]] gl_GlobalInvocationId : vec3<u32>,
) {}

This is how we can return more than one value:

struct FragmentOutput {
  [[location(0)]] mrt0 : vec4<f32>;
  [[location(0)]] mrt1 : vec4<f32>;
  [[builtin(sample_mask)]] sampleMask : u32;
};

[[stage(fragment)]]
fn fragMRT(
  [[builtin(position)]] pos : vec4<f32>
) -> FragmentOutput {}

The in/out variables and builtins can be specified either directly as argument (or return values), or put into a struct. This largely follows the HLSL (see example) and MSL (see example) shaders.

In the specification, we can say that struct layouts can be one of the 3 different kinds:

• interface layout: for the input/output structs. Every field has to have either [[location(xxx)]] or [[builtin(xxx)].
• block layout: for host-shareable structs. Some of the fields can have [[span(xx)]
• internal structs: no decorations are expected on the fields (but can still be on the types, etc).

Note: returning tuples may improve some of the cases, but it's not required in this proposal, can be considered after MVP.

[dj2]

What's the benefit of not moving the resources into the struct? Having everything passed to the function means they need to be passed to all call functions. This would be nice as we can stop doing the analysis of what's used by the shader, we just take the param / return values. Passing some, and referencing others means we still have to do the analysis for the resources.

[kvark]

I'm fine either way. My worry is that with resources specifically there is going to be a whole lot of repetition in the code.
It's not obvious to me also that gathering used resources brings any extra work. We still need to gather all the texture-sampler pairs used in sampling, statically, for example, to know if the binding layout is compatible.

[dneto0]

About resources: SPIR-V/Vulkan base functionality doesn't let you pass pointers to buffers (storage or uniform) into helper functions. So that argues in favour of leaving them outside.

[dneto0]

@kainino0x noted

When designing this it's worth noting that sometimes a vertex shader will be paired with a fragment shader that consumes only a subset of the vertex outputs.

Doesn't putting things in structs entangle one field with another?

[dneto0]

How does this affect certain builtin variables with special behaviour:

• sample_mask_out: writing to this affects how the fragments are handled or discarded.
  - > The value in the variable is significant only if the sample_mask_out variable is statically accessed by the fragment shader.
• frag_depth - when this is written Vulkan/SPIR-V requires the "DepthReplacing" execution mode. And it's undefined behaviour if we have the flag but the shader doesn't write it and vice versa.

Putting the variables in the parameter list / return struct makes it much less straightforward to track modifications when they can be across function boundaries (via pointer passing).

[kvark]

How does this affect certain builtin variables with special behaviour:
Putting the variables in the parameter list / return struct makes it much less straightforward to track modifications when they can be across function boundaries (via pointer passing).

Actually, I'd argue for the exact opposite 😝
When your function has to return something with a "sample_mask_out" value in it, you no longer need to analyze the code looking for all the branches and ensuring that all of them write to it. Instead, it's handled within the more general machinery of "function has to return a value of this type". In addition, you don't need a scan over the function call graph to see which builtins it touches, and how: everything is explicit in the signature. Note: you'd still need that scan for resources.

So overall the validation becomes easier, WGSL rules become simpler (no need to specify that all control flow writes to an output), and writing this code is easier (no repeating of varyings). It's a win all around.

[kdashg]

WGSL meeting minutes 2021-01-26

• DM: Background, when we write WGSL shaders there is a common problem that you have to repeat the varying between stages because of in vs output storage classes. Can’t use the same name as WGSL disallows that. Becomes tedious and unnecessary. Looking at target languages, HLSL has input/outputs as arguments and resources as globals. SPIR-V is only globals, and MSL is all arguments. Full spectrum is available so we can pick what we want. Think following HLSL for in/out as arguments and resources outside is the correct middle ground.
• MM: So, vertex shader outputs as strut and frag shader accepts struct as param?
• DM: Yup.
• MM: And there are global variables, one in and one out both as struct type?
• DM: Yup.
• JG: Like the idea of sharing the struct between the two.
• MM: One question, are we going to punch through nested structs?
• DM: I think the nesting 1 level is for MVP don’t think we need more? Meaning you can pass structs around but not structs in structs.
• MM. If vertex shader outputs struct with struct in it, legal or not?
• DM: Don't think it needs to be legal for MVP. Depth of 1 is fine for solving the case and being forward compatible.
• DN: You said in/out variables, I thought this was marking as arguments to entry points and return values. And the location attributes would be on the members of the struct right?
• DM: Yup.
• DN: If you re-use then the same locations for both input/output
• MM: You don’t have to do this though, I could have 2 distinct structs it would match using rules today. Do the in/out storage classes disappear?
• DN: Output probably has to stay but not input? Output is read write?
• DM: Seems like they could disappear.
• MM: That’s what metal does. You don’t have read/write outputs. Outputs are return values. If you want read/write use local var and copy into output
• DN: What happens on discard? Maybe that’s too detailed and should discuss on the bug.
• JG: Let's come back to this next week.
• MM: Seems like a good direction, like it in general.

[dneto0]

For handling pipeline inputs as entry point arguments, and pipeline outputs as entry point return, I thought about the translation path from SPIR-V (Vulkan) and to SPIR-V (Vulkan).

The translation scheme is fairly straightforward in both directions. There is no blowup in complexity of the code itself, so I think there will be negligible performance cost. For details, see https://dawn-review.googlesource.com/c/tint/+/38961 [KN: Preview link]

I have not thought about other impact on the WGSL spec.

[dneto0]

(Nitpick on the example at the top. The locations must overlap in the fragment output.)

[kdashg]

WGSL meeting minutes 2021-02-02

• DM: Don’t think there has been any good argument against and addresses the issue identified.
• DN: Did a design about the translation and was less complicated than thought. Would generate code DS doesn’t like but it’s correct and readable. Much better than I thought.
  • https://dawn.googlesource.com/tint/+/afb7b30f606f9c2cc35bb44c87bff9bec1f28ee9/docs/spirv-reader-input-output-variables.md
• MM: So for SPIR-V you’d just promote to global variables
• DM: Yes, in private address space and could see being inverted on the way back out.
• MM: In src that uses these entry point arguments going to want to pass to helper functions, so helpers would now take extra params. In the spirv code would the helper determine if it’s passed and then skip the param and use the global?
• DN: Enhancement you don’t need to get too. It’s ok to pass the ptr to inputs and outputs as function pointers and if that doesn’t work do inlining. You can detect code patterns that it’s another variable and short circuit in the helper. Find heuristically but doesn’t always work.
• MM: Question is, you’re cool with params that get passed around but never used? Oh they are used?
• DN: Yup. Did analysis for pipeline input and outputs. Resources are not cool as pointer semantics don’t work.
• DM: Wanted to know if the thing with privates passed into the function is something we have to do in the frontends regardless. Today when calling HLSL you have to have in the arguments. You put that into privates or pass into functions. Do the same quirks as spir-v frontend. But, I don't have to do it for the HLSL backend anymore. So justifying if we do this for SPIR-V frontend, but not HLSL backend but shaders are nicer to write we should do the former.
• DN: Don’t think anyone is arguing against this.
• MM: Downside is asymmetry between resources and in/out. Benefits of parameters outweighs the asymmetry.
• DM: Do we follow up by removing input/output storage classes?
• DN: Let’s have a draft of the spec change and see where it goes?
• DM: Should I do it?
• MM: Does this come with the idea that the inputs/outputs can be in a struct and can be shared between vert and frag shaders?
• DN: Yes, same thing. Don’t like a single level of struct.
• MM: So we’re going to punch through arbitrarily nested structs? Did that for WHLSL and found it was a pain and had little benefit. If you think it’s valuable we can go for it but don’t think it’s worth it.
• DN: It’s an orthogonally question. Wrapped up in counting of struct parameters. Why have this corner if we don't need to. Don’t think you need to but not sure where heading into.
• DM: Can discuss when we have the basics. I’ll draft this.