shader-slang / slang

Making it easier to work with shaders
http://shader-slang.com
MIT License
2.18k stars 187 forks source link

Revise support for unsized arrays #2187

Open tangent-vector opened 2 years ago

tangent-vector commented 2 years ago

The Basics

The Slang compiler supports unsized arrays as shader parameter declarations at global scope:

Texture2D gAllMyTextures[];

That scenario seems to be working well enough for our current users targeting D3D12 and Vulkan, especially given that they often opt to use explicit register and binding specifications on such parameters.

Other Cases

We also intend to support unsized arrays for shader parameter data in other contexts, but the current layout rules for these cases are not quite what we want, and we do not appear to always generate working GLSL/SPIR-V. To illustrate the other kinds of scenarios we'd like to support:

// structure type with a "trailing" unsized array:
struct SceneTextures
{
    TextureCube envMap;
    Texture2D surfaceMaps[];
}

ParameterBlock<SceneTextures> gSceneTextures;

// shader entry point with unsized array parameters:
[shader("fragment")];
float4 myFragmentShader(
    uniform Texture2D extraTextures[],
    VertexData v) : SV_Target
{ ... }

The layout rules need work, even in the global-scope case

The original layout rules we implemented for unsized arrays were probably too simplistic. For D3D12 and Vulkan, they more or less amount to:

That rule doesn't really work in an ideal fashion for Vulkan, though. Even though the GLSL/SPIR-V rules allow any binding to apply to an unbounded-size array, any descriptor-set layout created via the Vulkan API may only have one unbounded-size descriptor range, and it must come at the end of the list of bindings.

We also use those same basic rules when an unsized array appears as a struct field, even though we really don't want to be doing that.

Proposal

The basic idea is to distinguish the global-scope and struct cases a little bit more. The struct case can be described with relatively reasonable rules, based on intuition from C, where allowing a struct to end with an unsized array is a common language extension supported by compilers.

Structures

To sketch the rules we'd want to apply to structs:

These rules should allow a ParameterBlock<X> to intuitively map to a single set or space in the cases where a user would be inclined to expect it, even when X ends with an unsized array of resources:

struct SceneTextures
{
    TextureCube envMap;
    Texture2D surfaceMaps[];
}
// the following should map to a single `space`/`set`
ParameterBlock<SceneTextures> gSceneTextures;

In cases where a user wants to have multiple unbounded types nested under a single struct, they can simply use a ParameterBlock<...> to introduce an explicit indirection where they need it:

struct LotsOfTextures
{
    ParameterBlock<Texture2D[]> a;
    ParameterBlock<TextureCube[]> b;
    ...
}

Because explicit ParameterBlocks are being used in that case, it is clear that the user would intend for a and b to each get their own space/set during layout. The intuition of using parameter blocks also works for targets like CPU and CUDA, where a parameter block maps to an ordinary pointer, making the indirection explicit just like it would be if we tried to implement LotsOfTextures in C/C++.

Globals and Entry Points

Ideally we could just apply the same intuition at the global scope, but we cannot tell users that they are only allowed to have a single unsized array in the global scope, and that it must come "last" (what does "last" even mean in a multi-file compilation scenario?). Thus, at the global scope we realistically need to treat every unsized array declaration as if it was wrapped in its own ParameterBlock<...>. The same basic intuition needs to apply to entry-point parameter lists.

Wrinkles and Open Questions

There's a certain amount of hand-waving above just because the notion of "sized" vs. "unbounded" types is not something that the current semantic checking steps can see, and thus we cannot enforce the rules being described here in the presence of generics.

We also can't correctly map certain interesting cases to either HLSL/DXIL or GLSL/SPIR-V:

struct TexureSamplerPair { Texture2D t; SamplerState s; }
struct MyStuff
{
    TextureCube envMap;
    TextureSamplerPair pairs[]; // What to do?!
}

The pairs field above cannot map to a single binding in SPIR-V or a single range of registers in HLSL/DXIL. The work we have to do to "legalize" nested resource types means that the MyStuff type actually contains two unbounded-size arrays when compiling for D3D/VK, even though the programmer who wrote it might only see one. It isn't clear where it is appropriate to signal an error like this.

There is a larger issue here that certain types in Slang may be "sized" according to the rules I've outlined above, but are still not allowed to be used as the element type of arrays (and sometimes they are allowed for sized arrays but not unsized...). Structures that contain multiple resource-type fields are one example, but we also know that we cannot support arrays of ParameterBlocks on most of our graphics-API targets.

The rules I described above where a ParameterBlock<X> is always sized cannot work on D3D11 or OpenGL targets, where parameter blocks are actually just aliases for ConstantBuffer<X>. This is probably a non-issue in practice, since those targets also cannot support unbounded-size arrays.

natduca commented 11 months ago

Q3 language core, probably low priority

csyonghe commented 11 months ago

Related issue:

void f( int b[] )
{ ... }
...
int in[3] = {..};
f(in);  // <== compiler error

When passing in an explicitly sized array to a function that expects a runtime sized array, I get a compiler error on the form: error 30019: expected an expression of type 'float[]', got 'float[8]

FWIW the function I'm testing with also has a runtime sized out parameter: void test(float i[], out float o[])

csyonghe commented 2 months ago

Update: slang now has notion of sized and unbound types in the type system, and calling a function that takes int[] with an argument of type int[3] now works.

We still don't have layout rules overhauled for trailing unsized arrays in a struct to put it in the same space as the parent.

csyonghe commented 2 weeks ago

Update: we now support pointers to a struct with trailing unsized arrays when generating spirv.