shader-slang / slang

Making it easier to work with shaders
MIT License
1.79k stars 160 forks source link

Add compile flag to allow register() for Vulkan #4320

Closed chaoticbob closed 3 weeks ago

chaoticbob commented 4 weeks ago

I don't think this compiler option exists, I wasn't able to find anything in the help for it. If it does exist please let me know what the option is.

Currently, when compiling for to SPIR-V for Vulkan, Slang does not support resource declaration using register(X, [Y]) where X is the binding number and Y is the set number. Slang requires the usage of [[vk::binding(X, [Y])]] to specify the binding and set numbers. If [[vk::binding()]] is absent and register() is present, the behavior I've observed is that register() is ignored (with a warning) and resource binding numbers are auto assigned based on declaration order.

The current behavior might unexpected for some users. It's understandable that Slang wants users to be deliberate with the usage of [[vk::binding()]] but it's really inconvenient having to add that decoration to every shader resource in shader source that' shared between Vulkan and other graphics APIs. In cases where the same values are used for register() and [[vk::binding()]]` it's just the same information that's duplicated...and if incorrectly duplicated could lead to some really annoying bugs to track down.

Additionally, for some users it may be non-obvious that having only register() won't do the thing they're expecting and instead binding numbers will be auto assigned. This can easily produce a mismatch between the shader and the graphics API that results in a device lost. For example the snippet below where the register binding numbers are all over the place and auto assignment would just assigns based on declaration order, see spirv-reflect output below.

I think having something like -fvk-allow-register-bindings that allows register(X, [Y]) for binding and set if [[vk::binding(X, [Y])]] isn't present would make shader source sharing much easier and reduce the barrier to entry for on-ramping.

Shader snippet:

ConstantBuffer<SceneParameters> SceneParams : register(b5); // Scene params

// -----------------------------------------------------------------------------
// Ray Tracing Resources
// -----------------------------------------------------------------------------

RaytracingAccelerationStructure  Scene         : register(t0); // Acceleration structure
RWTexture2D<float4>              RenderTarget  : register(u1); // Output texture
RWTexture2D<float4>              AccumTarget   : register(u2); // Accumulation texture
RWStructuredBuffer<uint>         RayGenSamples : register(u3); // Ray generation samples

struct Triangle {
    uint vIdx0;
    uint vIdx1;
    uint vIdx2;
};

StructuredBuffer<Triangle> Triangles[25] : register(t20); // Index buffers
StructuredBuffer<float3>   Positions[25] : register(t45); // Position buffers
StructuredBuffer<float3>   Normals[25]   : register(t70); // Normal buffers

// -----------------------------------------------------------------------------
// PBR Resources
// -----------------------------------------------------------------------------
struct MaterialParameters 
{
    float3 baseColor;
    float  roughness;
    float  metallic;
    float  specularReflectance;
    float  ior;
    float3 emissionColor;
};

StructuredBuffer<MaterialParameters> MaterialParams : register(t9); // Material params

Texture2D    IBLEnvironmentMap[100] : register(t100);
SamplerState IBLMapSampler          : register(s10);

SPIRV-Reflect of resources of above shader snippet - binding numbers are auto assigned.

  Descriptor bindings: 10

    Binding 0.0
      set      : 0
      binding  : 0
      type     : VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER (CBV)
      count    : 1
      accessed : true
      name     : SceneParams (SceneParameters_natural) 

    Binding 0.1
      set      : 0
      binding  : 1
      type     : VK_DESCRIPTOR_TYPE_ACCELERATION_STRUCTURE_KHR (SRV)
      count    : 1
      accessed : true
      name     : Scene

    Binding 0.2
      set      : 0
      binding  : 2
      type     : VK_DESCRIPTOR_TYPE_STORAGE_IMAGE (UAV)
      count    : 1
      accessed : true
      name     : RenderTarget

    Binding 0.3
      set      : 0
      binding  : 3
      type     : VK_DESCRIPTOR_TYPE_STORAGE_IMAGE (UAV)
      count    : 1
      accessed : true
      name     : AccumTarget

    Binding 0.4
      set      : 0
      binding  : 4
      type     : VK_DESCRIPTOR_TYPE_STORAGE_BUFFER (UAV)
      count    : 1
      accessed : true
      name     : RayGenSamples (RWStructuredBuffer)

    Binding 0.5
      set      : 0
      binding  : 5
      type     : VK_DESCRIPTOR_TYPE_STORAGE_BUFFER (UAV)
      count    : 25
      array    : [25]
      accessed : true
      name     : Triangles (StructuredBuffer)

    Binding 0.7
      set      : 0
      binding  : 7
      type     : VK_DESCRIPTOR_TYPE_STORAGE_BUFFER (UAV)
      count    : 25
      array    : [25]
      accessed : true
      name     : Normals (StructuredBuffer)

    Binding 0.8
      set      : 0
      binding  : 8
      type     : VK_DESCRIPTOR_TYPE_STORAGE_BUFFER (UAV)
      count    : 1
      accessed : true
      name     : MaterialParams (StructuredBuffer)

    Binding 0.9
      set      : 0
      binding  : 9
      type     : VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE (SRV)
      count    : 100
      array    : [100]
      accessed : true
      name     : IBLEnvironmentMap

    Binding 0.10
      set      : 0
      binding  : 10
      type     : VK_DESCRIPTOR_TYPE_SAMPLER (SAMPLER)
      count    : 1
      accessed : true
      name     : IBLMapSampler
ArielG-NV commented 4 weeks ago

I have a question about the suggestion: I am assuming register syntax means only: register(slot, space), example: Texture2D<float4> v : register(slot, space); and not including: register(profile, slot), example: Texture2D<float4> v : register(cs_6_0, t0) : register( ps_6_0, t0) : register(gs_5_0, t99);?

(Texture2D<float4> v : register(cs_6_0, t0) : register(ps_6_0, t0) : register(gs_5_0, t99); is 'supported' but is non functional in DXC)

chaoticbob commented 4 weeks ago

Yes, that's right: register(slot, space) where slot is come combination of the resource type and register index and space is just spaceN where N is the space index.

I've never used the register(profile, slot) variation. I think I've only seen it in older shader code.