microsoft / DirectXShaderCompiler

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

[SPIR-V] Descriptor bindings assigned before dead code elimination #3189

Open nickyc95 opened 3 years ago

nickyc95 commented 3 years ago

Hi,

I am running into an issue where the binding numbers that are being assigned are not quite what I expect.

If you have a shader with 2 unused Cbuffers, then a CBuffer that is used you will end up with Set 0, Binding 2. I would expect it to be Set 0, Binding 0.

(I am using the shift functionality to move the textures and samplers so that buffer start at 0)

In the resulting Spirv I can see that the two unused Cbuffers have been eliminated but the binding index doesn't reflect that.

Texture2D g_texture2D;
sampler g_sampler;

cbuffer a //unused
{
    float4x4 g_a;
};
cbuffer b //unused
{
    float4x4 g_b;
};
cbuffer c //used
{
    float4x4 g_localToClip;
    float4 g_randomOffset;
    float4  g_colorAdd;
};

float4 mainPS( float3 vTexCoord : TEXCOORD, float4 vColor : COLOR ) : SV_Target
{
    return (g_texture2D.Sample( g_sampler, vTexCoord.xy ) * vColor) + g_colorAdd;
}

Expected: c [Set: 0, Binding: 0] Actual: c [Set: 0, Binding: 2]

Is it possible to get this kind of behaviour?

Thanks! Nick

LuciferSweety commented 2 years ago

I had the same issue. Do you fix it?

damyanp commented 1 week ago

@s-perron - this behaves differently to how bindings are allocated in DXIL mode. See https://godbolt.org/z/KMqqb5faE

s-perron commented 1 week ago

Resource binding numbers will never match what is done in DXIL because the two are fundamentally different. In Vulkan, textures and samples are in the same set, arrays of resources use a single binding in Vulkan, etc.. So we make minimal effort to try to match the space and binding in DXIL.

I can see users requesting different methods of assigning the bindings in SPIR-V. I do not want to change behaviour like that unless there is an overwhelmingly good reason. If we change the default behaviour we could break many people who rely on the current behaviour. I am aware of users who rely on unused resource "using" a binding to make the resource bindings in their vertex and fragment shaders match.

If you want the resources to be numbered from 0 to N, when there are N+1 resources used after optimization, then I would recommend writing a spirv-opt pass that renumbers binding. Then we could include it in DXC under an option.

However, the Spir-V maintainers will not have the resources to do this, but we can review the code.