microsoft / DirectXShaderCompiler

This repo hosts the source for the DirectX Shader Compiler which is based on LLVM/Clang.
Other
3.07k stars 683 forks source link

[SPIR-V] OpConstantSampler #4137

Closed turanszkij closed 1 month ago

turanszkij commented 2 years ago

Hello, would it be possible to add support for declaring OpConstantSampler ( https://www.khronos.org/registry/SPIR-V/specs/1.0/SPIRV.html#OpConstantSampler ) somehow in hlsl shader code for SPIR-V compiler?

It is already possible to define static samplers for DX12 via the root signature syntax, but that is not available for SPIR-V. It would make it more convenient to declare static samplers in shaders only, instead of declaring them in shaders for DX12, but declaring them in cpp code for vulkan.

It is fine if the shader syntax for spirv is different to dx12, because it could be handled by macro defines.

jaebaek commented 2 years ago

Supporting the root signature is a complicated issue. If we need it, it must be a much larger discussion.

Without the root signature support, is there a way to denoting a static sampler in HLSL e.g., using a keyword?

turanszkij commented 2 years ago

There is the old way of declaring samplers in shader which is still accepted by the compiler:

SamplerState sam
{
    Filter = MIN_MAG_MIP_LINEAR;
    AddressU = Wrap;
    AddressV = Wrap;
};

This method was used by the Effects framework for DirectX ( https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-sampler ), which reflected the sampler properties and bound the appropriate sampler state from directX code behind the scenes. Now the HLSL compiler generates a warning for it:

warning: effect state block ignored - effect syntax is deprecated. To use braces as an initializer use them with equal signs. [-Weffects-syntax]

Maybe this could be repurposed, so that the SPIRV compiler accepts it and generates a constant sampler. Or if purely repurposing this is not an option, maybe setting a flag like would be ok:

[[vk::constant_sampler]]
SamplerState sam
{
    Filter = MIN_MAG_MIP_LINEAR;
    AddressU = Wrap;
    AddressV = Wrap;
};

(Samplers like this will be handled in HLSL just like normal samplers without the initializer list, so either application needs to bind them, or the root signature declare them as static samplers)

s-perron commented 1 month ago

This would be candidate to implement with inline spir-v, but it currently fails because there is no way to place the OpConstantSampler instruction in the correct place. See https://godbolt.org/z/YKbEa4o43.

We could use similar syntax, but make it a builtin function in the compiler.

In any case, a change like this now requires going through HLSL specs. I've opened an issue in that repo: https://github.com/microsoft/hlsl-specs/issues/305.