[spirv] Add option to flatten array of resources.

docs/SPIR-V.rst

@@ -3403,6 +3403,13 @@ codegen for Vulkan:
 - ``-fspv-target-env=<env>``: Specifies the target environment for this compilation.
   The current valid options are ``vulkan1.0`` and ``vulkan1.1``. If no target
   environment is provided, ``vulkan1.0`` is used as default.
+- ``-fspv-flatten-resource-arrays``: Flattens arrays of textures and samplers
+  into individual resources, each taking one binding number. For example, an
+  array of 3 textures will become 3 texture resources taking 3 binding numbers.
+  This makes the behavior similar to DX. Without this option, you would get 1
+  array object taking 1 binding number. Note that arrays of
+  {RW|Append|Consume}StructuredBuffers are currently not supported in the
+  SPIR-V backend.
 - ``-Wno-vk-ignored-features``: Does not emit warnings on ignored features
   resulting from no Vulkan support, e.g., cbuffer member initializer.
 

include/dxc/Support/HLSLOptions.td

@@ -279,6 +279,8 @@ def fspv_extension_EQ : Joined<["-"], "fspv-extension=">, Group<spirv_Group>, Fl
   HelpText<"Specify SPIR-V extension permitted to use">;
 def fspv_target_env_EQ : Joined<["-"], "fspv-target-env=">, Group<spirv_Group>, Flags<[CoreOption, DriverOption]>,
   HelpText<"Specify the target environment: vulkan1.0 (default) or vulkan1.1">;
+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 Wno_vk_ignored_features : Joined<["-"], "Wno-vk-ignored-features">, Group<spirv_Group>, Flags<[CoreOption, DriverOption, HelpHidden]>,
   HelpText<"Do not emit warnings for ingored features resulting from no Vulkan support">;
 def Wno_vk_emulated_features : Joined<["-"], "Wno-vk-emulated-features">, Group<spirv_Group>, Flags<[CoreOption, DriverOption, HelpHidden]>,

include/dxc/Support/SPIRVOptions.h

@@ -53,6 +53,7 @@ struct SpirvCodeGenOptions {
   bool useDxLayout;
   bool useGlLayout;
   bool useScalarLayout;
+  bool flattenResourceArrays;
   SpirvLayoutRule cBufferLayoutRule;
   SpirvLayoutRule sBufferLayoutRule;
   SpirvLayoutRule tBufferLayoutRule;

lib/DxcSupport/HLSLOptions.cpp

@@ -667,6 +667,8 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
   opts.SpirvOptions.enableReflect = Args.hasFlag(OPT_fspv_reflect, OPT_INVALID, false);
   opts.SpirvOptions.noWarnIgnoredFeatures = Args.hasFlag(OPT_Wno_vk_ignored_features, OPT_INVALID, false);
   opts.SpirvOptions.noWarnEmulatedFeatures = Args.hasFlag(OPT_Wno_vk_emulated_features, OPT_INVALID, false);
+  opts.SpirvOptions.flattenResourceArrays =
+      Args.hasFlag(OPT_fspv_flatten_resource_arrays, OPT_INVALID, false);
 
   if (!handleVkShiftArgs(Args, OPT_fvk_b_shift, "b", &opts.SpirvOptions.bShift, errors) ||
       !handleVkShiftArgs(Args, OPT_fvk_t_shift, "t", &opts.SpirvOptions.tShift, errors) ||
@@ -741,6 +743,7 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
       Args.hasFlag(OPT_fvk_use_gl_layout, OPT_INVALID, false) ||
       Args.hasFlag(OPT_fvk_use_dx_layout, OPT_INVALID, false) ||
       Args.hasFlag(OPT_fvk_use_scalar_layout, OPT_INVALID, false) ||
+      Args.hasFlag(OPT_fspv_flatten_resource_arrays, OPT_INVALID, false) ||
       Args.hasFlag(OPT_fspv_reflect, OPT_INVALID, false) ||
       Args.hasFlag(OPT_Wno_vk_ignored_features, OPT_INVALID, false) ||
       Args.hasFlag(OPT_Wno_vk_emulated_features, OPT_INVALID, false) ||

tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

@@ -716,7 +716,7 @@ SpirvVariable *DeclResultIdMapper::createExternVar(const VarDecl *var) {
   const auto *bindingAttr = var->getAttr<VKBindingAttr>();
   const auto *counterBindingAttr = var->getAttr<VKCounterBindingAttr>();
 
-  resourceVars.emplace_back(varInstr, loc, regAttr, bindingAttr,
+  resourceVars.emplace_back(varInstr, var, loc, regAttr, bindingAttr,
                             counterBindingAttr);
 
   if (const auto *inputAttachment = var->getAttr<VKInputAttachmentIndexAttr>())
@@ -846,7 +846,7 @@ SpirvVariable *DeclResultIdMapper::createCTBuffer(const HLSLBufferDecl *decl) {
     astDecls[varDecl] = DeclSpirvInfo(bufferVar, index++);
   }
   resourceVars.emplace_back(
-      bufferVar, decl->getLocation(), getResourceBinding(decl),
+      bufferVar, decl, decl->getLocation(), getResourceBinding(decl),
       decl->getAttr<VKBindingAttr>(), decl->getAttr<VKCounterBindingAttr>());
 
   return bufferVar;
@@ -890,7 +890,7 @@ SpirvVariable *DeclResultIdMapper::createCTBuffer(const VarDecl *decl) {
   // We register the VarDecl here.
   astDecls[decl] = DeclSpirvInfo(bufferVar);
   resourceVars.emplace_back(
-      bufferVar, decl->getLocation(), getResourceBinding(context),
+      bufferVar, decl, decl->getLocation(), getResourceBinding(context),
       decl->getAttr<VKBindingAttr>(), decl->getAttr<VKCounterBindingAttr>());
 
   return bufferVar;
@@ -970,8 +970,8 @@ void DeclResultIdMapper::createGlobalsCBuffer(const VarDecl *var) {
       context, /*arraySize*/ 0, ContextUsageKind::Globals, "type.$Globals",
       "$Globals");
 
-  resourceVars.emplace_back(globals, SourceLocation(), nullptr, nullptr,
-                            nullptr, /*isCounterVar*/ false,
+  resourceVars.emplace_back(globals, /*decl*/ nullptr, SourceLocation(),
+                            nullptr, nullptr, nullptr, /*isCounterVar*/ false,
                             /*isGlobalsCBuffer*/ true);
 
   uint32_t index = 0;
@@ -1089,7 +1089,7 @@ void DeclResultIdMapper::createCounterVar(
   if (!isAlias) {
     // Non-alias counter variables should be put in to resourceVars so that
     // descriptors can be allocated for them.
-    resourceVars.emplace_back(counterInstr, decl->getLocation(),
+    resourceVars.emplace_back(counterInstr, decl, decl->getLocation(),
                               getResourceBinding(decl),
                               decl->getAttr<VKBindingAttr>(),
                               decl->getAttr<VKCounterBindingAttr>(), true);
@@ -1213,26 +1213,63 @@ class LocationSet {
 /// set and binding number.
 class BindingSet {
 public:
-  /// Uses the given set and binding number.
-  void useBinding(uint32_t binding, uint32_t set) {
-    usedBindings[set].insert(binding);
+  /// Uses the given set and binding number. Returns false if the binding number
+  /// was already occupied in the set, and returns true otherwise.
+  bool useBinding(uint32_t binding, uint32_t set) {
+    bool inserted = false;
+    std::tie(std::ignore, inserted) = usedBindings[set].insert(binding);
+    return inserted;
+  }
+
+  /// Uses the next avaiable binding number in |set|. If more than one binding
+  /// number is to be occupied, it finds the next available chunk that can fit
+  /// |numBindingsToUse| in the |set|.
+  uint32_t useNextBinding(uint32_t set, uint32_t numBindingsToUse = 1) {
+    uint32_t bindingNoStart = getNextBindingChunk(set, numBindingsToUse);
+    auto &binding = usedBindings[set];
+    for (uint32_t i = 0; i < numBindingsToUse; ++i)
+      binding.insert(bindingNoStart + i);
+    return bindingNoStart;
   }
 
-  /// Uses the next avaiable binding number in set 0.
-  uint32_t useNextBinding(uint32_t set) {
-    auto &binding = usedBindings[set];
-    auto &next = nextBindings[set];
-    while (binding.count(next))
-      ++next;
-    binding.insert(next);
-    return next++;
+  /// Returns the first available binding number in the |set| for which |n|
+  /// consecutive binding numbers are unused.
+  uint32_t getNextBindingChunk(uint32_t set, uint32_t n) {
+    auto &existingBindings = usedBindings[set];
+
+    // There were no bindings in this set. Can start at binding zero.
+    if (existingBindings.empty())
+      return 0;
+
+    // Check whether the chunk of |n| binding numbers can be fitted at the
+    // very beginning of the list (start at binding 0 in the current set).
+    uint32_t curBinding = *existingBindings.begin();
+    if (curBinding >= n)
+      return 0;
+
+    auto iter = std::next(existingBindings.begin());
+    while (iter != existingBindings.end()) {
+      // There exists a next binding number that is used. Check to see if the
+      // gap between current binding number and next binding number is large
+      // enough to accommodate |n|.
+      uint32_t nextBinding = *iter;
+      if (n <= nextBinding - curBinding - 1)
+        return curBinding + 1;
+
+      curBinding = nextBinding;
+
+      // Peek at the next binding that has already been used (if any).
+      ++iter;
+    }
+
+    // |curBinding| was the last binding that was used in this set. The next
+    // chunk of |n| bindings can start at |curBinding|+1.
+    return curBinding + 1;
   }
 
 private:
   ///< set number -> set of used binding number
-  llvm::DenseMap<uint32_t, llvm::DenseSet<uint32_t>> usedBindings;
-  ///< set number -> next available binding number
-  llvm::DenseMap<uint32_t, uint32_t> nextBindings;
+  llvm::DenseMap<uint32_t, std::set<uint32_t>> usedBindings;
 };
 } // namespace
 
@@ -1553,11 +1590,30 @@ bool DeclResultIdMapper::decorateResourceBindings() {
 
   // Decorates the given varId of the given category with set number
   // setNo, binding number bindingNo. Ignores overlaps.
-  const auto tryToDecorate = [this, &bindingSet](SpirvVariable *var,
+  const auto tryToDecorate = [this, &bindingSet](const ResourceVar &var,
                                                  const uint32_t setNo,
                                                  const uint32_t bindingNo) {
-    bindingSet.useBinding(bindingNo, setNo);
-    spvBuilder.decorateDSetBinding(var, setNo, bindingNo);
+    // By default we use one binding number per resource, and an array of
+    // resources also gets only one binding number. However, for array of
+    // 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)
+      numBindingsToUse = var.getArraySize();
+
+    for (uint32_t i = 0; i < numBindingsToUse; ++i) {
+      bool success = bindingSet.useBinding(bindingNo + i, setNo);
+      if (!success && spirvOptions.flattenResourceArrays) {
+        emitError("ran into binding number conflict when assigning binding "
+                  "number %0 in set %1",
+                  {})
+            << bindingNo << setNo;
+      }
+    }
+
+    // No need to decorate multiple binding numbers for arrays. It will be done
+    // by legalization/optimization.
+    spvBuilder.decorateDSetBinding(var.getSpirvInstr(), setNo, bindingNo);
   };
 
   for (const auto &var : resourceVars) {
@@ -1570,13 +1626,12 @@ bool DeclResultIdMapper::decorateResourceBindings() {
         else if (const auto *reg = var.getRegister())
           set = reg->RegisterSpace.getValueOr(defaultSpace);
 
-        tryToDecorate(var.getSpirvInstr(), set, vkCBinding->getBinding());
+        tryToDecorate(var, set, vkCBinding->getBinding());
       }
     } else {
       if (const auto *vkBinding = var.getBinding()) {
         // Process m1
-        tryToDecorate(var.getSpirvInstr(),
-                      getVkBindingAttrSet(vkBinding, defaultSpace),
+        tryToDecorate(var, getVkBindingAttrSet(vkBinding, defaultSpace),
                       vkBinding->getBinding());
       }
     }
@@ -1617,10 +1672,18 @@ bool DeclResultIdMapper::decorateResourceBindings() {
           llvm_unreachable("unknown register type found");
         }
 
-        tryToDecorate(var.getSpirvInstr(), set, binding);
+        tryToDecorate(var, set, binding);
       }
 
   for (const auto &var : resourceVars) {
+    // By default we use one binding number per resource, and an array of
+    // resources also gets only one binding number. However, for array of
+    // 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)
+      numBindingsToUse = var.getArraySize();
+
     if (var.isCounter()) {
       if (!var.getCounterBinding()) {
         // Process mX * c2
@@ -1630,15 +1693,17 @@ bool DeclResultIdMapper::decorateResourceBindings() {
         else if (const auto *reg = var.getRegister())
           set = reg->RegisterSpace.getValueOr(defaultSpace);
 
-        spvBuilder.decorateDSetBinding(var.getSpirvInstr(), set,
-                                       bindingSet.useNextBinding(set));
+        spvBuilder.decorateDSetBinding(
+            var.getSpirvInstr(), set,
+            bindingSet.useNextBinding(set, numBindingsToUse));
       }
     } else if (!var.getBinding()) {
       const auto *reg = var.getRegister();
       if (reg && reg->isSpaceOnly()) {
         const uint32_t set = reg->RegisterSpace.getValueOr(defaultSpace);
-        spvBuilder.decorateDSetBinding(var.getSpirvInstr(), set,
-                                       bindingSet.useNextBinding(set));
+        spvBuilder.decorateDSetBinding(
+            var.getSpirvInstr(), set,
+            bindingSet.useNextBinding(set, numBindingsToUse));
       } else if (!reg) {
         // Process m3 (no 'vk::binding' and no ':register' assignment)
 
@@ -1653,7 +1718,7 @@ bool DeclResultIdMapper::decorateResourceBindings() {
         else {
           spvBuilder.decorateDSetBinding(
               var.getSpirvInstr(), defaultSpace,
-              bindingSet.useNextBinding(defaultSpace));
+              bindingSet.useNextBinding(defaultSpace, numBindingsToUse));
         }
       }
     }

tools/clang/lib/SPIRV/DeclResultIdMapper.h

@@ -111,12 +111,24 @@ class StageVar {
 
 class ResourceVar {
 public:
-  ResourceVar(SpirvVariable *var, SourceLocation loc,
+  ResourceVar(SpirvVariable *var, const Decl *decl, SourceLocation loc,
               const hlsl::RegisterAssignment *r, const VKBindingAttr *b,
               const VKCounterBindingAttr *cb, bool counter = false,
               bool globalsBuffer = false)
       : variable(var), srcLoc(loc), reg(r), binding(b), counterBinding(cb),
-        isCounterVar(counter), isGlobalsCBuffer(globalsBuffer) {}
+        isCounterVar(counter), isGlobalsCBuffer(globalsBuffer), arraySize(1) {
+    if (decl) {
+      if (const ValueDecl *valueDecl = dyn_cast<ValueDecl>(decl)) {
+        const QualType type = valueDecl->getType();
+        if (!type.isNull() && type->isConstantArrayType()) {
+          if (auto constArrayType = dyn_cast<ConstantArrayType>(type)) {
+            arraySize =
+                static_cast<uint32_t>(constArrayType->getSize().getZExtValue());
+          }
+        }
+      }
+    }
+  }
 
   SpirvVariable *getSpirvInstr() const { return variable; }
   SourceLocation getSourceLocation() const { return srcLoc; }
@@ -127,6 +139,7 @@ class ResourceVar {
   const VKCounterBindingAttr *getCounterBinding() const {
     return counterBinding;
   }
+  uint32_t getArraySize() const { return arraySize; }
 
 private:
   SpirvVariable *variable;                    ///< The variable
@@ -136,6 +149,7 @@ class ResourceVar {
   const VKCounterBindingAttr *counterBinding; ///< Vulkan counter binding
   bool isCounterVar;                          ///< Couter variable or not
   bool isGlobalsCBuffer;                      ///< $Globals cbuffer or not
+  uint32_t arraySize;                         ///< Size if resource is an array
 };
 
 /// A (instruction-pointer, is-alias-or-not) pair for counter variables

tools/clang/lib/SPIRV/SpirvEmitter.cpp

@@ -164,7 +164,7 @@ bool spirvToolsLegalize(spv_target_env env, std::vector<uint32_t> *module,
 }
 
 bool spirvToolsOptimize(spv_target_env env, std::vector<uint32_t> *module,
-                        const llvm::SmallVector<llvm::StringRef, 4> &flags,
+                        clang::spirv::SpirvCodeGenOptions &spirvOptions,
                         std::string *messages) {
   spvtools::Optimizer optimizer(env);
 
@@ -176,14 +176,16 @@ bool spirvToolsOptimize(spv_target_env env, std::vector<uint32_t> *module,
   spvtools::OptimizerOptions options;
   options.set_run_validator(false);
 
-  if (flags.empty()) {
+  if (spirvOptions.optConfig.empty()) {
     optimizer.RegisterPerformancePasses();
+    if (spirvOptions.flattenResourceArrays)
+      optimizer.RegisterPass(spvtools::CreateDescriptorScalarReplacementPass());
     optimizer.RegisterPass(spvtools::CreateCompactIdsPass());
   } else {
     // Command line options use llvm::SmallVector and llvm::StringRef, whereas
     // SPIR-V optimizer uses std::vector and std::string.
     std::vector<std::string> stdFlags;
-    for (const auto &f : flags)
+    for (const auto &f : spirvOptions.optConfig)
       stdFlags.push_back(f.str());
     if (!optimizer.RegisterPassesFromFlags(stdFlags))
       return false;
@@ -662,8 +664,7 @@ void SpirvEmitter::HandleTranslationUnit(ASTContext &context) {
     // Run optimization passes
     if (theCompilerInstance.getCodeGenOpts().OptimizationLevel > 0) {
       std::string messages;
-      if (!spirvToolsOptimize(targetEnv, &m, spirvOptions.optConfig,
-                              &messages)) {
+      if (!spirvToolsOptimize(targetEnv, &m, spirvOptions, &messages)) {
         emitFatalError("failed to optimize SPIR-V: %0", {}) << messages;
         emitNote("please file a bug report on "
                  "https://github.com/Microsoft/DirectXShaderCompiler/issues "

tools/clang/test/CodeGenSPIRV/vk.binding.cl.flatten-arrays.error.hlsl

@@ -0,0 +1,19 @@
+// Run: %dxc -T ps_6_0 -E main -fspv-flatten-resource-arrays
+
+// CHECK: error: ran into binding number conflict when assigning binding number 3 in set 0
+
+Texture2D    MyTextures[5] : register(t0); // Forced use of binding numbers 0, 1, 2, 3, 4.
+Texture2D    AnotherTexture : register(t3); // Error: Forced use of binding number 3.
+SamplerState MySampler;
+
+float4 main(float2 TexCoord : TexCoord) : SV_Target0 {
+  float4 result =
+    MyTextures[0].Sample(MySampler, TexCoord) +
+    MyTextures[1].Sample(MySampler, TexCoord) +
+    MyTextures[2].Sample(MySampler, TexCoord) +
+    MyTextures[3].Sample(MySampler, TexCoord) +
+    MyTextures[4].Sample(MySampler, TexCoord) +
+    AnotherTexture.Sample(MySampler, TexCoord);
+  return result;
+}
+