diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index 02b1755bf8..918cddb729 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -87,6 +87,12 @@ line upon naming the release. Refer to previous for appropriate section names. struct is flat-converted [#8526](https://github.com/microsoft/DirectXShaderCompiler/issues/8526). +#### SPIR-V + +- Added `-fspv-reserve-resource-array-bindings`, which reserves one + binding number per array element for arrays of resources without splitting + the array into separate SPIR-V variables/bindings. + ### Upcoming Preview Release These changes apply to experimental preview shader models only and will not be diff --git a/docs/SPIR-V.rst b/docs/SPIR-V.rst index ef0dd3a3a2..4c42e5a4a0 100644 --- a/docs/SPIR-V.rst +++ b/docs/SPIR-V.rst @@ -4423,6 +4423,14 @@ codegen for Vulkan: SPIR-V backend. Also note that this requires the optimizer to be able to resolve all array accesses with constant indeces. Therefore, all loops using the resource arrays must be marked with ``[unroll]``. +- ``-fspv-reserve-resource-array-bindings``: Reserves one binding number + per array element for arrays of resources, matching the binding numbers + that ``-fspv-flatten-resource-arrays`` would reserve, but keeps each array + as a single SPIR-V variable/binding (with ``descriptorCount`` equal to the + array size) instead of splitting it into one variable per element. Use this + when you want later-declared resources to avoid overlapping the numbers an + array would need under DX, without requiring the array itself to be + split up or unrolled. - ``-fspv-entrypoint-name=``: Specify the SPIR-V entry point name. Defaults to the HLSL entry point name. - ``-fspv-use-legacy-buffer-matrix-order``: Assumes the legacy matrix order (row diff --git a/include/dxc/Support/HLSLOptions.td b/include/dxc/Support/HLSLOptions.td index bfd986ad3e..e1c6b5aae2 100644 --- a/include/dxc/Support/HLSLOptions.td +++ b/include/dxc/Support/HLSLOptions.td @@ -405,6 +405,8 @@ def fspv_target_env_EQ : Joined<["-"], "fspv-target-env=">, Group, HelpText<"Specify the target environment: vulkan1.0 (default), vulkan1.1, vulkan1.1spirv1.4, vulkan1.2, vulkan1.3, or universal1.5">; def fspv_flatten_resource_arrays: Flag<["-"], "fspv-flatten-resource-arrays">, Group, Flags<[CoreOption, DriverOption]>, HelpText<"Flatten arrays of resources so each array element takes one binding number">; +def fspv_reserve_resource_array_bindings: Flag<["-"], "fspv-reserve-resource-array-bindings">, Group, Flags<[CoreOption, DriverOption]>, + HelpText<"Reserve one binding number per array element for arrays of resources, without splitting the array into separate variables">; def fspv_reduce_load_size: Flag<["-"], "fspv-reduce-load-size">, Group, Flags<[CoreOption, DriverOption]>, HelpText<"Replaces loads of composite objects to reduce memory pressure for the loads">; def fspv_fix_func_call_arguments: Flag<["-"], "fspv-fix-func-call-arguments">, Group, Flags<[CoreOption, DriverOption, HelpHidden]>, diff --git a/include/dxc/Support/SPIRVOptions.h b/include/dxc/Support/SPIRVOptions.h index 0253caba63..276be5cf4c 100644 --- a/include/dxc/Support/SPIRVOptions.h +++ b/include/dxc/Support/SPIRVOptions.h @@ -64,6 +64,7 @@ struct SpirvCodeGenOptions { bool useLegacyBufferMatrixOrder = false; bool useScalarLayout = false; bool flattenResourceArrays = false; + bool reserveResourceArrayBindings = false; bool reduceLoadSize = false; bool autoShiftBindings = false; bool supportNonzeroBaseInstance = false; diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp index 1630243aab..7e3f7ece7d 100644 --- a/lib/DxcSupport/HLSLOptions.cpp +++ b/lib/DxcSupport/HLSLOptions.cpp @@ -1120,6 +1120,8 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, Args.hasFlag(OPT_Wno_vk_emulated_features, OPT_INVALID, false); opts.SpirvOptions.flattenResourceArrays = Args.hasFlag(OPT_fspv_flatten_resource_arrays, OPT_INVALID, false); + opts.SpirvOptions.reserveResourceArrayBindings = Args.hasFlag( + OPT_fspv_reserve_resource_array_bindings, OPT_INVALID, false); opts.SpirvOptions.reduceLoadSize = Args.hasFlag(OPT_fspv_reduce_load_size, OPT_INVALID, false); opts.SpirvOptions.fixFuncCallArguments = @@ -1294,6 +1296,8 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, Args.hasFlag(OPT_fspv_use_legacy_buffer_matrix_order, OPT_INVALID, false) || Args.hasFlag(OPT_fspv_flatten_resource_arrays, OPT_INVALID, false) || + Args.hasFlag(OPT_fspv_reserve_resource_array_bindings, OPT_INVALID, + false) || Args.hasFlag(OPT_fspv_reduce_load_size, OPT_INVALID, false) || Args.hasFlag(OPT_fspv_reflect, OPT_INVALID, false) || Args.hasFlag(OPT_fspv_fix_func_call_arguments, OPT_INVALID, false) || diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp index 9aaa36c97b..1d61f69c11 100644 --- a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp +++ b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp @@ -140,6 +140,10 @@ uint32_t getNumBindingsUsedByResourceType(QualType type) { if (type.isNull()) return 1; + // Strip typedefs so that arrays declared through them (e.g. + // `typedef Texture2D Textures[5];`) are still seen as arrays below. + type = type.getCanonicalType(); + // For every array dimension, the number of bindings needed should be // multiplied by the array size. For example: an array of two Textures should // use 2 binding slots. @@ -150,9 +154,20 @@ uint32_t getNumBindingsUsedByResourceType(QualType type) { type = constArrayType->getElementType(); } - // Once we remove the arrayness, we expect the given type to be either a - // resource OR a structure that only contains resources. - assert(isResourceType(type) || isResourceOnlyStructure(type)); + // An unbounded (runtime-sized) array, e.g. Texture2D textures[], consumes + // the rest of its register space: HLSL requires it to be the last + // resource declared in that space, so no other resource can be placed at + // a higher binding number that a precise reservation would need to + // protect. Reserve a single slot for it instead of trying to compute a + // size that doesn't exist. + if (type->isIncompleteArrayType()) + return arrayFactor; + + // Not every entry in resourceVars is a resource: for example, globals with + // [[vk::ext_storage_class]] are also registered there. Those still only use + // one binding number. + if (!isResourceType(type) && !isResourceOnlyStructure(type)) + return 1; // In the case of a resource, each resource takes 1 binding slot, so in total // it consumes: 1 * arrayFactor. @@ -2664,7 +2679,9 @@ bool DeclResultIdMapper::decorateResourceBindings() { // resources (e.g. array of textures), DX uses one binding number per array // element. We can match this behavior via a command line option. uint32_t numBindingsToUse = 1; - if (spirvOptions.flattenResourceArrays || needsFlatteningCompositeResources) + if (spirvOptions.flattenResourceArrays || + spirvOptions.reserveResourceArrayBindings || + needsFlatteningCompositeResources) numBindingsToUse = getNumBindingsUsedByResourceType( var.getSpirvInstr()->getAstResultType()); @@ -2754,7 +2771,9 @@ bool DeclResultIdMapper::decorateResourceBindings() { // resources (e.g. array of textures), DX uses one binding number per array // element. We can match this behavior via a command line option. uint32_t numBindingsToUse = 1; - if (spirvOptions.flattenResourceArrays || needsFlatteningCompositeResources) + if (spirvOptions.flattenResourceArrays || + spirvOptions.reserveResourceArrayBindings || + needsFlatteningCompositeResources) numBindingsToUse = getNumBindingsUsedByResourceType( var.getSpirvInstr()->getAstResultType()); diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.cl.reserve-array-bindings.example1-optimized.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.reserve-array-bindings.example1-optimized.hlsl new file mode 100644 index 0000000000..c20dd0ea56 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.reserve-array-bindings.example1-optimized.hlsl @@ -0,0 +1,30 @@ +// RUN: %dxc -T ps_6_0 -E main -fspv-reserve-resource-array-bindings -O3 %s -spirv | FileCheck %s + +// The array is NOT split into individual variables (unlike +// -fspv-flatten-resource-arrays): MyTextures/MySamplers stay as single +// array variables, each with a single Binding decoration, but the binding +// numbers reserved for them still account for the full array size. +// CHECK-NOT: MyTextures_0_ +// CHECK-NOT: MySamplers_0_ +// CHECK: OpDecorate %MyTextures Binding 0 +// CHECK: OpDecorate %AnotherTexture Binding 5 +// CHECK: OpDecorate %NextTexture Binding 6 +// CHECK: OpDecorate %MySamplers Binding 7 +// CHECK: OpDecorate %LastTexture Binding 9 +Texture2D MyTextures[5] : register(t0); +Texture2D NextTexture; // This is supposed to be t6. +Texture2D AnotherTexture : register(t5); +SamplerState MySamplers[2]; +Texture2D LastTexture; // Right after MySamplers[2], so this is supposed to be t9. + +float4 main(float2 TexCoord : TexCoord) : SV_Target0 +{ + float4 result = 0; + [unroll] + for (uint i = 0; i < 5; ++i) + result += MyTextures[i].Sample(MySamplers[i < 3 ? 0 : 1], TexCoord); + result += AnotherTexture.Sample(MySamplers[1], TexCoord); + result += NextTexture.Sample(MySamplers[1], TexCoord); + result += LastTexture.Sample(MySamplers[1], TexCoord); + return result; +} diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.cl.reserve-array-bindings.example1.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.reserve-array-bindings.example1.hlsl new file mode 100644 index 0000000000..24d73d49ef --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.reserve-array-bindings.example1.hlsl @@ -0,0 +1,26 @@ +// RUN: %dxc -T ps_6_0 -E main -fspv-reserve-resource-array-bindings -fcgl %s -spirv | FileCheck %s + +// CHECK: OpDecorate %MyTextures Binding 0 +// CHECK: OpDecorate %AnotherTexture Binding 5 +// CHECK: OpDecorate %NextTexture Binding 6 +// CHECK: OpDecorate %MySamplers Binding 7 +// CHECK: OpDecorate %LastTexture Binding 9 +Texture2D MyTextures[5] : register(t0); +Texture2D NextTexture; // This is supposed to be t6. +Texture2D AnotherTexture : register(t5); +SamplerState MySamplers[2]; +Texture2D LastTexture; // Right after MySamplers[2], so this is supposed to be t9. + +float4 main(float2 TexCoord : TexCoord) : SV_Target0 +{ + float4 result = + MyTextures[0].Sample(MySamplers[0], TexCoord) + + MyTextures[1].Sample(MySamplers[0], TexCoord) + + MyTextures[2].Sample(MySamplers[0], TexCoord) + + MyTextures[3].Sample(MySamplers[1], TexCoord) + + MyTextures[4].Sample(MySamplers[1], TexCoord) + + AnotherTexture.Sample(MySamplers[1], TexCoord) + + NextTexture.Sample(MySamplers[1], TexCoord) + + LastTexture.Sample(MySamplers[1], TexCoord); + return result; +} diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.cl.reserve-array-bindings.non-resource.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.reserve-array-bindings.non-resource.hlsl new file mode 100644 index 0000000000..e84465d9ae --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.reserve-array-bindings.non-resource.hlsl @@ -0,0 +1,17 @@ +// RUN: %dxc -T ps_6_0 -E main -fspv-reserve-resource-array-bindings -Vd -fcgl %s -spirv | FileCheck %s + +// Regression test: globals that are not resources (here, one with an inline +// SPIR-V storage class) are also visited when reserving bindings. They must +// fall back to a single binding instead of tripping the resource-only +// assertion in getNumBindingsUsedByResourceType. + +[[vk::ext_extension("SPV_KHR_ray_tracing")]] +[[vk::ext_capability(/* RayTracingKHR */ 4479)]] +[[vk::ext_storage_class(/*RayPayloadKHR*/5338)]] +float4 payload; +// CHECK-DAG: [[ptr_payload_v4:%[a-zA-Z0-9_]+]] = OpTypePointer RayPayloadKHR %v4float +// CHECK-DAG: %payload = OpVariable [[ptr_payload_v4]] RayPayloadKHR + +float4 main() : SV_Target0 { + return payload; +} diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.cl.reserve-array-bindings.typedef.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.reserve-array-bindings.typedef.hlsl new file mode 100644 index 0000000000..ff892a83f4 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.reserve-array-bindings.typedef.hlsl @@ -0,0 +1,25 @@ +// RUN: %dxc -T ps_6_0 -E main -fspv-reserve-resource-array-bindings -fcgl %s -spirv | FileCheck %s + +// Regression test: arrays declared through a typedef must reserve one binding +// per element, just like arrays spelled out directly. Both the explicitly and +// the implicitly bound typedef'd arrays have to be counted in full, otherwise +// the resources declared after them overlap. +// CHECK: OpDecorate %ExplicitTextures Binding 0 +// CHECK: OpDecorate %ImplicitTextures Binding 3 +// CHECK: OpDecorate %NextTexture Binding 6 +typedef Texture2D Textures[3]; +Textures ExplicitTextures : register(t0); +Textures ImplicitTextures; +Texture2D NextTexture; +SamplerState MySampler; + +float4 main(float2 TexCoord : TexCoord) : SV_Target0 +{ + float4 result = + ExplicitTextures[0].Sample(MySampler, TexCoord) + + ExplicitTextures[2].Sample(MySampler, TexCoord) + + ImplicitTextures[0].Sample(MySampler, TexCoord) + + ImplicitTextures[2].Sample(MySampler, TexCoord) + + NextTexture.Sample(MySampler, TexCoord); + return result; +} diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.cl.reserve-array-bindings.unbounded.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.reserve-array-bindings.unbounded.hlsl new file mode 100644 index 0000000000..81948b7264 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.reserve-array-bindings.unbounded.hlsl @@ -0,0 +1,21 @@ +// RUN: %dxc -T ps_6_0 -E main -fspv-reserve-resource-array-bindings -fcgl %s -spirv | FileCheck %s + +// Regression test: an explicitly-bound unbounded (runtime-sized) resource +// array used to crash getNumBindingsUsedByResourceType, since its AST type +// (IncompleteArrayType) isn't a ConstantArrayType and isn't a resource type +// either, so the function fell through to its "unknown resource type" +// unreachable. An unbounded array must be the last resource declared in its +// register space, so nothing can collide with it regardless of how many +// slots are reserved for it -- it only reserves its own single binding +// number, same as without this flag. NextTexture (auto-assigned, no +// explicit register) is unaffected by MyTextures's position and lands on +// the first free slot, binding 0. +// CHECK: OpDecorate %MyTextures Binding 5 +// CHECK: OpDecorate %NextTexture Binding 0 +Texture2D MyTextures[] : register(t5); +Texture2D NextTexture; + +float4 main(float2 TexCoord : TexCoord, uint index : Index) : SV_Target0 +{ + return MyTextures[index].Load(int3(0, 0, 0)) + NextTexture.Load(int3(0, 0, 0)); +}