Improve logic for when a "default space" is needed (shader-slang#925)

If the user declares global shader parameters for D3D SM5.1+ or Vulkan, then they need to go into an appropriate `space` or `set`:

```hlsl
Texture2D    t; // should go in space/set 0
SamplerState s; // same here...
```

This also applies to allocation of spaces/sets to parameter blocks:

```hlsl
ParameterBlock<X> x; // should get space/set 0
ParameterBlock<Y> y; // should get space/set 1
```

In cases where there are a combination of explicitly and implicitly bound parameters, anything left implicitly bound goes into a "default" space/set:

```
ParameterBlock<X> x : register(space0); // this has claimed space/set 0

Texture2D    t; // this needs a space, so a "default" space/set of 1 will be claimed
SamplerState s; // this also needs a space/set, and will use the default
```

The logic for deciding when a default space/set was needing was, more or less, looking at all the global shader parameters and seeing if any of them needed a `register`/`binding`, and if so determining that a default space /set would be needed.

There was a bug in that logic, though, because of cases like the following:

```hlsl
ParameterBlock<X> x;
Texture2D         t : register(t0, space99);
```

In this case, the parameter `t` already has an explicit binding, so it doesn't actually need a default space to be allocated. If we allocate a default space/set of 0 on the basis of `t`, then `x` will end up being shifted to space/set 1.

The fix is to only consider global parameters that need `register`s/`binding`s *if* they don't have an explicit binding already (which is luckily something we are tracking during parameter binding).

Note: just to clarify the behavior here, the "do we need a default space/set?" logic is done *before* automatic binding of parameters, so in a shader with any global texture/buffer/sampler parameters, those will all end up in space/set zero (in the absence of explicit bindings), and explicit blocks will start at space/set one, independent of the order of declaration. This behavior is maybe too subtle, and we might decide we need to change it, but it will have to do for now.

Author: Tim Foley

source/slang/parameter-binding.cpp

@@ -2298,23 +2298,43 @@ RefPtr<ProgramLayout> generateParameterBindings(
     // As a starting point, we will definitely need a "default" space if
     // we are creating a default constant buffer, since it should get
     // a binding in that "default" space.
+    //
     bool needDefaultSpace = needDefaultConstantBuffer;
     if (!needDefaultSpace)
     {
+        // Next we will look at the global-scope parameters and see if
+        // any of them requires a `register` or `binding` that will
+        // thus need to land in a default space.
+        //
         for (auto& parameterInfo : sharedContext.parameters)
         {
             SLANG_RELEASE_ASSERT(parameterInfo->varLayouts.Count() != 0);
             auto firstVarLayout = parameterInfo->varLayouts.First();
 
-            // Does the parameter have any resource usage that isn't just
-            // allocating a whole register space?
+            // For each parameter, we will look at each resource it consumes.
+            //
             for (auto resInfo : firstVarLayout->typeLayout->resourceInfos)
             {
-                if (resInfo.kind != LayoutResourceKind::RegisterSpace)
-                {
-                    needDefaultSpace = true;
-                    break;
-                }
+                // We don't care about whole register spaces/sets, since
+                // we don't need to allocate a default space/set for a parameter
+                // that itself consumes a whole space/set.
+                //
+                if( resInfo.kind == LayoutResourceKind::RegisterSpace )
+                    continue;
+
+                // We also don't want to consider resource kinds for which
+                // the variable already has an (explicit) binding, since
+                // the space from the explicit binding will be used, so
+                // that a default space isn't needed.
+                //
+                if( parameterInfo->bindingInfo[resInfo.kind].count != 0 )
+                    continue;
+
+                // Otherwise, we have a shader parameter that will need
+                // a default space or set to live in.
+                //
+                needDefaultSpace = true;
+                break;
             }
         }
     }

tests/reflection/default-space.slang

@@ -0,0 +1,23 @@
+//TEST:REFLECTION:-profile sm_5_1 -stage fragment -target hlsl
+
+// This test is to confirm that we do not allocate a "default"
+// space/set for global shader parameters unless it is
+// really required. In particular, if there are global-scope
+// resource parameters *but* they are all explicitly bound,
+// then a default space isn't needed.
+
+
+// An explicitly-bound global texture.
+Texture2D a : register(t0, space99);
+
+// An implicitly-bound global parameter block.
+//
+// This parameter should be given `space0`, because
+// it is the first available space after all explicitly-bound
+// parameters have claimed their registers/spaces.
+//
+struct B { Texture2D b; }
+ParameterBlock<B> b;
+
+float4 main() : SV_Target
+{ return 0.0; }

tests/reflection/default-space.slang.expected

@@ -0,0 +1,44 @@
+result code = 0
+standard error = {
+}
+standard output = {
+{
+    "parameters": [
+        {
+            "name": "a",
+            "binding": {"kind": "shaderResource", "space": 99, "index": 0},
+            "type": {
+                "kind": "resource",
+                "baseShape": "texture2D"
+            }
+        },
+        {
+            "name": "b",
+            "binding": {"kind": "registerSpace", "index": 0},
+            "type": {
+                "kind": "parameterBlock",
+                "elementType": {
+                    "kind": "struct",
+                    "name": "B",
+                    "fields": [
+                        {
+                            "name": "b",
+                            "type": {
+                                "kind": "resource",
+                                "baseShape": "texture2D"
+                            },
+                            "binding": {"kind": "shaderResource", "index": 0}
+                        }
+                    ]
+                }
+            }
+        }
+    ],
+    "entryPoints": [
+        {
+            "name": "main",
+            "stage:": "fragment"
+        }
+    ]
+}
+}