DirectXShaderCompiler icon indicating copy to clipboard operation
DirectXShaderCompiler copied to clipboard

[SPIR-V] Allow built-in object types as arguments to SpirvType

Open cassiebeckley opened this issue 1 year ago • 2 comments

There has been a validation error for passing in a built-in type to a template since the first commit to DXC. This check exists to prevent instances like this:

Texture2D<SamplerState> image;

When DXC was first created, users could not create their own templates, and therefore having a generic check for this did not cause any problems. However, now that HLSL 2021 allows users to create their own templates, and inline SPIR-V lets users pass types in to the SpirvType and SpirvOpaqueType templates in order to reference them, this is causing problems.

This PR allows the SpirvType and SpirvOpaqueType templates to take HLSL built-in types as arguments.

Fixes #6498

cassiebeckley avatar Oct 08 '24 19:10 cassiebeckley

For me it would just be enough to allow SpirvType to take another SpirvType AND be able to obtain the SpirvType (or some link) to a compound object like groupshared T or RWStructuredBuffer<T>

Stuffing SpirTypes into HLSL templates is probably a road to many bugs, and the most important feature is being able to build SpirvTypes from Native and SpirV Types because that way we can build any type we want/need.

Texture types should really only take scalar or vectors. The fact that they ever allowed anything else is a bug.

This should be be allowing more type as templates into the standard Texture types. If it does, that is unintentional.

We want to allow more types as templates to the vk::Spirv*Type. We were thinking we had a use case for allowing textures as a template to be able to define combined texture samplers, but that will not work. We have other issues trying to to implement combined texture samplers as inline spir-v.

However, we should still be able to use a SpirvType as a template to a SpirvType: https://godbolt.org/z/Pqh6nYchf.

s-perron avatar Oct 15 '24 18:10 s-perron