[spirv] Tweak binding number assignment scheme (#675)

Now we essentially assign binding numbers to all resources in
three passes: [[vk::binding(...)]], register(...), and then
no annotation.

docs/SPIR-V.rst

@@ -642,8 +642,7 @@ In shaders for DirectX, resources are accessed via registers; while in shaders
 for Vulkan, it is done via descriptor set and binding numbers. The developer
 can explicitly annotate variables in HLSL to specify descriptor set and binding
 numbers, or leave it to the compiler to derive implicitly from registers.
-The explicit way has precedence over the implicit way. However, a mix of both
-way is not allowed (yet).
+The explicit way has precedence over the implicit way.
 
 Explicit binding number assignment
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -667,6 +666,15 @@ If there is no register specification, the corresponding resource will be
 assigned to the next available binding number, starting from 0, in descriptor
 set #0.
 
+In summary, the compiler essentially assigns binding numbers in three passes.
+
+- Firstly it handles all declarations with explicit ``[[vk::binding(X[, Y])]]``
+  annotation.
+- Then the compiler processes all remaining declarations with
+  ``:register(xX, spaceY)`` annotation.
+- Finally, the compiler assigns next available binding numbers to the rest in
+  the declaration order.
+
 HLSL Expressions
 ================
 

tools/clang/lib/SPIRV/DeclResultIdMapper.cpp

@@ -464,62 +464,47 @@ bool DeclResultIdMapper::finalizeStageIOLocations(bool forInput) {
 }
 
 bool DeclResultIdMapper::decorateResourceBindings() {
-  // Returns true if the given ResourceVar has explicit descriptor set and
-  // binding specified.
-  const auto bindingAssigned = [](const ResourceVar &var) {
-    return var.getBinding() != nullptr;
-  };
-
   BindingSet bindingSet;
   bool noError = true;
 
-  if (std::all_of(resourceVars.begin(), resourceVars.end(), bindingAssigned)) {
-    for (const auto &var : resourceVars) {
-      const auto attrLoc = var.getBinding()->getLocation();
-      const auto set = var.getBinding()->getSet();
-      const auto binding = var.getBinding()->getBinding();
+  // Process variables with [[vk::binding(...)]] binding assignment
+  for (const auto &var : resourceVars)
+    if (const auto *vkBinding = var.getBinding()) {
+      const auto set = vkBinding->getSet();
+      const auto binding = vkBinding->getBinding();
 
       if (bindingSet.isBindingUsed(binding, set)) {
         emitError("resource binding #%0 in descriptor set #%1 already assigned",
-                  attrLoc)
+                  vkBinding->getLocation())
             << binding << set;
         noError = false;
+      } else {
+        theBuilder.decorateDSetBinding(var.getSpirvId(), set, binding);
+        bindingSet.useBinding(binding, set);
       }
-
-      theBuilder.decorateDSetBinding(var.getSpirvId(),
-                                     var.getBinding()->getSet(),
-                                     var.getBinding()->getBinding());
-
-      bindingSet.useBinding(binding, set);
     }
-    return noError;
-  }
 
-  if (std::any_of(resourceVars.begin(), resourceVars.end(), bindingAssigned)) {
-    // We have checked that not all of the stage variables have explicit
-    // set and binding assignment.
-    emitError("partial explicit resource binding assignment via "
-              "[[vk::binding(X[, Y])]] unsupported");
-    return false;
-  }
-
-  for (const auto &var : resourceVars) {
-    // TODO: we can have duplicated set and binding number because of there are
-    // multiple resource types in the following. E.g., :register(s0) and
-    // :register(t0) will both map to set #0 and binding #0.
-    uint32_t set = 0, binding = 0;
-    if (const auto *reg = var.getRegister()) {
-      set = reg->RegisterSpace;
-      binding = reg->RegisterNumber;
-      bindingSet.useBinding(binding, set);
-    } else {
-      binding = bindingSet.useNextBinding();
-    }
+  // Process variables with register(...) binding assignment
+  for (const auto &var : resourceVars)
+    if (const auto *reg = var.getRegister())
+      if (!var.getBinding()) {
+        const uint32_t set = reg->RegisterSpace;
+        const uint32_t binding = reg->RegisterNumber;
+
+        // TODO: we can have duplicated set and binding number because of there
+        // are multiple resource types in the following. E.g., :register(s0) and
+        // :register(t0) will both map to set #0 and binding #0.
+        theBuilder.decorateDSetBinding(var.getSpirvId(), set, binding);
+        bindingSet.useBinding(binding, set);
+      }
 
-    theBuilder.decorateDSetBinding(var.getSpirvId(), set, binding);
-  }
+  // Process variables with no binding assignment
+  for (const auto &var : resourceVars)
+    if (!var.getBinding() && !var.getRegister())
+      theBuilder.decorateDSetBinding(var.getSpirvId(), 0,
+                                     bindingSet.useNextBinding());
 
-  return true;
+  return noError;
 }
 
 QualType

tools/clang/test/CodeGenSPIRV/vk.binding.explicit.hlsl

@@ -11,8 +11,8 @@ SamplerState sampler1      : register(s1, space1);
 SamplerState sampler2      : register(s2);
 
 // CHECK:      OpDecorate %texture1 DescriptorSet 0
-// CHECK-NEXT: OpDecorate %texture1 Binding 5
-[[vk::binding(5)]]
+// CHECK-NEXT: OpDecorate %texture1 Binding 2
+[[vk::binding(2)]]
 Texture2D<float4> texture1;
 
 // CHECK:      OpDecorate %texture2 DescriptorSet 2
@@ -46,8 +46,20 @@ struct S {
 [[vk::binding(2, 3)]]
 RWStructuredBuffer<S> sbuffer2 : register(u6);
 
-// TODO: support [[vk::binding()]] on AppendStructuredBuffer
-// TODO: support [[vk::binding()]] on ConsumeStructuredBuffer
+// CHECK:      OpDecorate %asbuffer DescriptorSet 1
+// CHECK-NEXT: OpDecorate %asbuffer Binding 20
+// CHECK-NEXT: OpDecorate %csbuffer DescriptorSet 1
+// CHECK-NEXT: OpDecorate %csbuffer Binding 21
+// CHECK-NEXT: OpDecorate %counter_var_asbuffer DescriptorSet 0
+// CHECK-NEXT: OpDecorate %counter_var_asbuffer Binding 1
+// CHECK-NEXT: OpDecorate %counter_var_csbuffer DescriptorSet 0
+// CHECK-NEXT: OpDecorate %counter_var_csbuffer Binding 4
+[[vk::binding(20, 1)]]
+AppendStructuredBuffer<S> asbuffer : register(u10);
+// Next available "hole": binding #1 in set #0
+[[vk::binding(21, 1)]]
+ConsumeStructuredBuffer<S> csbuffer : register(u11);
+// Next available "hole": binding #4 in set #0
 
 float4 main() : SV_Target {
     return 1.0;

tools/clang/test/CodeGenSPIRV/vk.binding.register.hlsl

@@ -18,14 +18,8 @@ Texture2D<float4> texture1: register(t2, space1);
 // CHECK-NEXT: OpDecorate %texture2 Binding 1
 Texture3D<float4> texture2: register(t1);
 
-// Note: using the next available binding #
-// CHECK:      OpDecorate %sampler3 DescriptorSet 0
-// CHECK-NEXT: OpDecorate %sampler3 Binding 0
 SamplerState sampler3;
 
-// Note: using the next available binding #
-// CHECK:      OpDecorate %sampler4 DescriptorSet 0
-// CHECK-NEXT: OpDecorate %sampler4 Binding 2
 SamplerState sampler4;
 
 // CHECK:      OpDecorate %var_myCbuffer DescriptorSet 3
@@ -64,16 +58,26 @@ RWStructuredBuffer<S> sbuffer2 : register(u6, space1);
     // The counter variable will use the next unassigned number
 // CHECK:      OpDecorate %abuffer DescriptorSet 0
 // CHECK-NEXT: OpDecorate %abuffer Binding 5
-// CHECK-NEXT: OpDecorate %counter_var_abuffer DescriptorSet 0
-// CHECK-NEXT: OpDecorate %counter_var_abuffer Binding 4
 AppendStructuredBuffer<S> abuffer : register(u5);
 
     // The counter variable will use the next unassigned number
 // CHECK:      OpDecorate %csbuffer DescriptorSet 0
 // CHECK-NEXT: OpDecorate %csbuffer Binding 7
+ConsumeStructuredBuffer<S> csbuffer : register(u7);
+
+// Note: The following are using the next available binding #
+
+// CHECK:      OpDecorate %sampler3 DescriptorSet 0
+// CHECK-NEXT: OpDecorate %sampler3 Binding 0
+
+// CHECK:      OpDecorate %sampler4 DescriptorSet 0
+// CHECK-NEXT: OpDecorate %sampler4 Binding 2
+
+// CHECK-NEXT: OpDecorate %counter_var_abuffer DescriptorSet 0
+// CHECK-NEXT: OpDecorate %counter_var_abuffer Binding 4
+
 // CHECK-NEXT: OpDecorate %counter_var_csbuffer DescriptorSet 0
 // CHECK-NEXT: OpDecorate %counter_var_csbuffer Binding 6
-ConsumeStructuredBuffer<S> csbuffer : register(u7);
 
 float4 main() : SV_Target {
     return 1.0;