[SPIRV] Add -fspv-flatten-resource-array-bindings-only

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

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=<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

include/dxc/Support/HLSLOptions.td

@@ -405,6 +405,8 @@ def fspv_target_env_EQ : Joined<["-"], "fspv-target-env=">, Group<spirv_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<spirv_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<spirv_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<spirv_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<spirv_Group>, Flags<[CoreOption, DriverOption, HelpHidden]>,

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;

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) ||

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());
 

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;
+}

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;
+}

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;
+}

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;
+}

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));
+}