Refactor D3D12 renderer root signature creation (shader-slang#1779)

This change originated as an attempt to re-enable a test case, but it has ended up disabling more tests (for good reasons) than it re-enables.

The main change here is a significant overhaul of the way that the D3D12 render path extracts information from the Slang reflection API to produce a root signature.
There were also some supporting fixes in the reflection information to make sure it returns what the D3D12 back-end needed.

The big picture here is that the D3D12 path now uses the descriptor ranges stored in the reflection data more or less directly.
It still needs to use register/space offset information queried via the "old" reflection API, but it only does so at the top level now, for the program and entry points themselves.
All other layout information is derived directly from what Slang provides.

Smaller changes:

* The "flat" reflection API was expanded to include `getBindingRangeDescriptorRangeCount()` which was clearly missing.

* The "flat" reflection results for a constant buffer or parameter block that didn't contain any uniform data and was mapped to a plain constant buffer needed to be fixed up. That logic is still way to subtle to be trusted.

* Several additional tests were disabled that relied on static specialization, global/entry-point generi type parameters, structured buffers of interfaces or other features we don't officially support with shader objects right now. All of the affected tests were somehow passing by sheer luck and because they often passed in specialization arguments via explicit  `TEST_INPUT` lines.

* The `inteface-shader-param` test is re-enabled now that we can properly describe its input with the new `set` mode on `TEST_INPUT`

* `ShaderCursor::getElement()` can now be used on structure types (in addition to arrays) to support by-index access to fields

* The `TEST_INPUT` system was expanded to support both by-name and by-index setting of structure fields for aggregates

* The `TEST_INPUT` system was expanded to allow an `out` prefix to mark parts of an expression as outputs on a `set` lines

* The `TEST_INPUT` system was expanded so that anything that would be allowed on a `TEST_INPUT` line by itself (like `ubuffer(...)`) can now be used as a sub-expression on a `set` line

Co-authored-by: Yong He <yonghe@outlook.com>

Author: Tim Foley

slang.h

@@ -1993,6 +1993,7 @@ extern "C"
 
     SLANG_API SlangInt spReflectionTypeLayout_getBindingRangeDescriptorSetIndex(SlangReflectionTypeLayout* typeLayout, SlangInt index);
     SLANG_API SlangInt spReflectionTypeLayout_getBindingRangeFirstDescriptorRangeIndex(SlangReflectionTypeLayout* typeLayout, SlangInt index);
+    SLANG_API SlangInt spReflectionTypeLayout_getBindingRangeDescriptorRangeCount(SlangReflectionTypeLayout* typeLayout, SlangInt index);
 
     SLANG_API SlangInt spReflectionTypeLayout_getDescriptorSetCount(SlangReflectionTypeLayout* typeLayout);
     SLANG_API SlangInt spReflectionTypeLayout_getDescriptorSetSpaceOffset(SlangReflectionTypeLayout* typeLayout, SlangInt setIndex);
@@ -2608,6 +2609,12 @@ namespace slang
                 index);
         }
 
+        SlangInt getBindingRangeDescriptorRangeCount(SlangInt index)
+        {
+            return spReflectionTypeLayout_getBindingRangeDescriptorRangeCount(
+                (SlangReflectionTypeLayout*) this,
+                index);
+        }
 
         SlangInt getDescriptorSetCount()
         {

source/slang/slang-reflection-api.cpp

@@ -1326,38 +1326,53 @@ namespace Slang
                 Index bindingRangeIndex = m_extendedInfo->m_bindingRanges.getCount();
                 SlangBindingType bindingType = SLANG_BINDING_TYPE_CONSTANT_BUFFER;
                 Index spaceOffset = -1;
+                bool usesIndirectAllocation = false;
                 LayoutResourceKind kind = LayoutResourceKind::None;
 
                 // TODO: It is unclear if this should be looking at the resource
                 // usage of the parameter group, or of its "container" layout.
                 //
                 for(auto& resInfo : parameterGroupTypeLayout->resourceInfos)
                 {
+                    if( spaceOffset == -1 )
+                    {
+                        spaceOffset = _calcSpaceOffset(path, kind);
+                    }
+
                     kind = resInfo.kind;
                     switch(kind)
                     {
                     default:
                         continue;
 
+                    case LayoutResourceKind::ConstantBuffer:
+                    case LayoutResourceKind::PushConstantBuffer:
+                    case LayoutResourceKind::DescriptorTableSlot:
+                        break;
+
+                        // Certain cases indicate a parameter block that
+                        // actually involves indirection.
+                        //
                         // Note: the only case where a parameter group should
                         // reflect as consuming `Uniform` storage is on CPU/CUDA,
                         // where that will be the only resource it contains.
                         //
                         // TODO: If we ever support targets that don't have
                         // constant buffers at all, this logic would be questionable.
                         //
-                    case LayoutResourceKind::ConstantBuffer:
-                    case LayoutResourceKind::PushConstantBuffer:
                     case LayoutResourceKind::RegisterSpace:
-                    case LayoutResourceKind::DescriptorTableSlot:
                     case LayoutResourceKind::Uniform:
+                        usesIndirectAllocation = true;
                         break;
                     }
 
                     bindingType = _calcBindingType(typeLayout, kind);
-                    spaceOffset = _calcSpaceOffset(path, kind);
                     break;
                 }
+                if(spaceOffset == -1)
+                {
+                    spaceOffset = 0;
+                }
 
                 TypeLayout::ExtendedInfo::BindingRangeInfo bindingRange;
                 bindingRange.leafTypeLayout = typeLayout;
@@ -1419,7 +1434,7 @@ namespace Slang
                 // because the physical storage for `C.a` is provided by the
                 // memory allocation for `C` itself.
 
-                if( spaceOffset != -1 )
+                if( !usesIndirectAllocation )
                 {
                     // The logic here assumes that when a parameter group consumes
                     // resources that must "leak" into the outer scope (including
@@ -1848,6 +1863,19 @@ SLANG_API SlangInt spReflectionTypeLayout_getBindingRangeFirstDescriptorRangeInd
     return bindingRange.firstDescriptorRangeIndex;
 }
 
+SLANG_API SlangInt spReflectionTypeLayout_getBindingRangeDescriptorRangeCount(SlangReflectionTypeLayout* inTypeLayout, SlangInt index)
+{
+    auto typeLayout = convert(inTypeLayout);
+    if(!typeLayout) return 0;
+
+    auto extTypeLayout = Slang::getExtendedTypeLayout(typeLayout);
+    if(index < 0) return 0;
+    if(index >= extTypeLayout->m_bindingRanges.getCount()) return 0;
+    auto& bindingRange = extTypeLayout->m_bindingRanges[index];
+
+    return bindingRange.descriptorRangeCount;
+}
+
 SLANG_API SlangInt spReflectionTypeLayout_getDescriptorSetCount(SlangReflectionTypeLayout* inTypeLayout)
 {
     auto typeLayout = convert(inTypeLayout);

source/slang/slang.cpp

@@ -2832,7 +2832,9 @@ SLANG_NO_THROW SlangResult SLANG_MCALL ComponentType::specialize(
     auto specializationParamCount = getSpecializationParamCount();
     if( specializationArgCount != specializationParamCount )
     {
-        // TODO: diagnose
+        sink.diagnose(SourceLoc(), Diagnostics::mismatchSpecializationArguments,
+            specializationParamCount,
+            specializationArgCount);
         sink.getBlobIfNeeded(outDiagnostics);
         return SLANG_FAIL;
     }

tests/bugs/gh-357.slang

@@ -1,4 +1,4 @@
-//TEST(compute):COMPARE_COMPUTE: -shaderobj
+//DISABLED_TEST(compute):COMPARE_COMPUTE: -shaderobj
 
 //TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out,name outputBuffer
 

tests/compute/assoctype-generic-arg.slang

@@ -1,5 +1,5 @@
-//TEST(compute):COMPARE_COMPUTE:-cpu -shaderobj
-//TEST(compute):COMPARE_COMPUTE: -shaderobj
+//DISABLED_TEST(compute):COMPARE_COMPUTE:-cpu -shaderobj
+//DISABLED_TEST(compute):COMPARE_COMPUTE: -shaderobj
 
 //TEST_INPUT:type AssocImpl
 

tests/compute/global-generic-value-param.slang

@@ -1,6 +1,6 @@
 // global-generic-value-param.slang
 
-//TEST(compute):COMPARE_COMPUTE: -shaderobj
+//DISABLED_TEST(compute):COMPARE_COMPUTE: -shaderobj
 
 // This is a basic test of support for global generic
 // value parameters: explicit named parameters at global

tests/compute/global-type-param-in-entrypoint.slang

@@ -1,4 +1,4 @@
-//TEST(compute):COMPARE_RENDER_COMPUTE: -shaderobj
+//DISABLED_TEST(compute):COMPARE_RENDER_COMPUTE: -shaderobj
 
 //TEST_INPUT: cbuffer(data=[1.0 0.0 0.0 0.0 0.0 1.0 0.0 0.0 0.0 0.0 1.0 0.0 0.0 0.0 0.0 1.0]):name Uniforms
 //TEST_INPUT: ubuffer(data=[0 0 0 0], stride=4):out,name outputBuffer

tests/compute/global-type-param.slang

@@ -1,4 +1,4 @@
-//TEST(smoke,compute):COMPARE_COMPUTE: -shaderobj
+//DISABLED_TEST(smoke,compute):COMPARE_COMPUTE: -shaderobj
 
 //TEST_INPUT:type Wrapper<Impl>
 

tests/compute/int-generic.slang

@@ -1,4 +1,4 @@
-//TEST(compute):COMPARE_COMPUTE: -shaderobj
+//DISABLED_TEST(compute):COMPARE_COMPUTE: -shaderobj
 
 //TEST_INPUT:type Material<1,2>
 

tests/compute/interface-assoc-type-param.slang

@@ -1,7 +1,7 @@
 // Tests using associated types through an existential-struct-typed param.
 
-//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -cuda -shaderobj
-//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -cpu -shaderobj
+//DISABLED_TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -cuda -shaderobj
+//DISABLED_TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -cpu -shaderobj
 
 [anyValueSize(8)]
 interface IInterface

tests/compute/interface-func-param-in-struct.slang

@@ -1,7 +1,7 @@
 // Tests specializing a function with existential-struct-typed param.
 
-//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -cuda -shaderobj
-//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -cpu -shaderobj
+//DISABLED_TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -cuda -shaderobj
+//DISABLED_TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -cpu -shaderobj
 
 [anyValueSize(8)]
 interface IInterface
@@ -31,8 +31,6 @@ void compute(uint tid, Params p)
     gOutputBuffer[tid] = p.obj[0].eval();
 }
 
-//TEST_INPUT: entryPointExistentialType Impl
-
 [numthreads(4, 1, 1)]
 void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID,
 //TEST_INPUT:ubuffer(data=[0 0 0 0 1 0], stride=4):name=params.obj

tests/compute/interface-shader-param-in-struct.slang

@@ -4,7 +4,6 @@
 // inside of structure types to make sure that works
 
 //DISABLED_TEST(compute):COMPARE_COMPUTE_EX:-slang -compute
-
 //DISABLED_TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -dx12 -profile sm_6_0 -use-dxil
 //DISABLED_TEST(compute, vulkan):COMPARE_COMPUTE_EX:-vk -compute
 
@@ -42,7 +41,7 @@ int test(
 }
 
 
-//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out
+//TEST_INPUT:set gOutputBuffer = out ubuffer(data=[0 0 0 0], stride=4)
 RWStructuredBuffer<int> gOutputBuffer;
 
 // Note: even though `C` doesn't include any
@@ -52,7 +51,8 @@ RWStructuredBuffer<int> gOutputBuffer;
 // that it *will* contain uniform/ordinary data
 // after specialization.
 //
-//TEST_INPUT:cbuffer(data=[0 0 0 0 0 0 0 0]):
+//TEST_INPUT:set C = new{ new MyStrategy{ ubuffer(data=[1 2 4 8], stride=4) } }
+//TEST_INPUT: globalExistentialType MyStrategy
 cbuffer C
 {
    IRandomNumberGenerationStrategy gStrategy;
@@ -64,23 +64,10 @@ struct Stuff
     int extra;
 }
 
-// Note: the data for global-scope existential parameters
-// is being introduced *before* the entry point declaration,
-// because the default policy used by `slangc` (which the
-// render test also uses) is to specialize the parameters at
-// the global scope (producing a new layout) and then compose
-// that specialized global scope with the entry point.
-//
-// (The net result is that data related to global-scope
-// specialization always precedes the data for entry point
-// parameters in these tests today)
-//
-//TEST_INPUT: globalExistentialType MyStrategy
-//TEST_INPUT:ubuffer(data=[1 2 4 8], stride=4):
-
 [numthreads(4, 1, 1)]
 void computeMain(
-//TEST_INPUT:root_constants(data=[0 0 0 0 0 0 0 0 256]):
+//TEST_INPUT:set stuff = { new MyModifier{ ubuffer(data=[16 32 64 128], stride=4) }, 256 }
+//TEST_INPUT: entryPointExistentialType MyModifier
     uniform Stuff stuff,
 
     uint3       dispatchThreadID : SV_DispatchThreadID)
@@ -129,6 +116,3 @@ struct MyModifier : IModifier
         return val ^ localModifiers[val & 3];
     }
 }
-
-//TEST_INPUT: entryPointExistentialType MyModifier
-//TEST_INPUT:ubuffer(data=[16 32 64 128], stride=4):

tests/compute/interface-shader-param.slang

@@ -3,11 +3,11 @@
 // Test using interface tops as top-level shader parameters
 // (whether global, or on an entry point).
 
-//DISABLED_TEST(compute):COMPARE_COMPUTE_EX:-slang -compute
+//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute
 
-//DISABLED_TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -dx12 -profile sm_6_0 -use-dxil
-//DISABLED_TEST(compute, vulkan):COMPARE_COMPUTE_EX:-vk -compute
-//DISABLE_TEST(compute):COMPARE_COMPUTE_EX:-cpu -compute
+//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -dx12 -profile sm_6_0 -use-dxil
+//TEST(compute, vulkan):COMPARE_COMPUTE_EX:-vk -compute
+//TEST(compute):COMPARE_COMPUTE_EX:-cpu -compute
 
 // First we will define some fake interfaces for testing.
 // Let's pretend we are doing some kind of random number
@@ -70,43 +70,27 @@ int test(
     return modifiedVal;
 }
 
-// The global-scope parameters for this example include
-// some `uniform` parameters, and that will trigger
-// the allocation of a global-scope constant buffer to
-// hold them. Slang's layout rules mean that the buffer
-// will be allocated before any other global-scope parameters.
-//
-// In this example, the buffer will not be needed after specialization,
-// but we need to declare/allocate it here so that the application
-// creates a descriptor table/set that matches what the shader
-// signature expects.
-//
-//TEST_INPUT:cbuffer(data=[0], stride=4):name=gStrategy
-
-
-
 // Now we'll define a shader entry point that will use
 // these interfaces to define its behavior.
 //
 // We'll start with the buffer for writing the test output.
 
-//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out,name=gOutputBuffer
+//TEST_INPUT:set gOutputBuffer = out ubuffer(data=[0 0 0 0], stride=4)
 RWStructuredBuffer<int> gOutputBuffer;
 
 // Now we'll define a global shader parameter for the
 // random number generation strategy.
 //
-//__disabled__TEST_INPUT:object(type=MyStrategy):name=gStrategy
+//TEST_INPUT:set gStrategy = new MyStrategy{}
 uniform IRandomNumberGenerationStrategy gStrategy;
 
 // The other parameter (for the modifier) will be attached
 // the entry point instead, so that we are testing both
 // cases.
 //
-//__disabled__TEST_INPUT:object(type=MyModifier):name=modifier
 [numthreads(4, 1, 1)]
 void computeMain(
-//TEST_INPUT:root_constants(data=[0], stride=4):
+//TEST_INPUT:set modifier = new MyModifier{}
     uniform IModifier   modifier,
             uint3       dispatchThreadID : SV_DispatchThreadID)
 {
@@ -150,6 +134,3 @@ struct MyModifier : IModifier
         return val * 16;
     }
 }
-
-//TEST_INPUT: globalExistentialType MyStrategy
-//TEST_INPUT: entryPointExistentialType MyModifier

tests/compute/parameter-block.slang

@@ -3,10 +3,6 @@
 //TEST(compute):COMPARE_COMPUTE:-vk -shaderobj
 //TEST(compute):COMPARE_COMPUTE:-shaderobj
 
-
-//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out,name=block0.buffer
-//TEST_INPUT:ubuffer(data=[0 1 2 3], stride=4):name=block1.buffer
-
 // Ensure that Slang `ParameterBlock` type is lowered
 // to HLSL in the fashion that we expect.
 
@@ -15,7 +11,10 @@ struct P
 	RWStructuredBuffer<int> buffer;
 };
 
+//TEST_INPUT: set block0 = new{ out ubuffer(data=[0 0 0 0], stride=4) }
 ParameterBlock<P> block0;
+
+//TEST_INPUT: set block1 = new{ ubuffer(data=[0 1 2 3], stride=4) }
 ParameterBlock<P> block1;
 
 [numthreads(4, 1, 1)]

tests/disabled-tests.txt

@@ -9,15 +9,53 @@ Test that don't work with shader objects in render-test
 The following tests were disabled because they had been running on non `-shaderobj` code paths that have since been removed.
 These tests will need to be re-enabled together with changes to the shader object implementation, or removed entirely if they no longer test useful functionality.
 
+### `ConstantBuffer<ISomething>`
+
+These tests rely on details of how a `ConstantBuffer<ISomething>` or `ParameterBlock<ISomething>` gets laid out.
+We need to fix the compiler and shader object implementation to agree that a `ConstantBuffer<ISomething>`
+should compile as `exists T : ISomething . ConstantBuffer<T>` and *not* `ConstantBuffer<exists T : ISomething . T>`
+like it currently does.
+(The reason for this choice is that we want a shader object created from `SomeConcreteType`, where `SomeConcreteType : ISomething`, to be directly usable for a `ConstantBuffer<ISomething>` parameter with its existing buffer allocation.)
+
 * compute/dynamic-dispatch-12.slang
+
+### `StructuredBuffer<ISomething>`
+
+These tests require support for structured buffers where the element type either is an interface type or transitively contains one.
+
 * compute/dynamic-dispatch-13.slang
 * compute/dynamic-dispatch-14.slang
 * compute/dynamic-dispatch-bindless-texture.slang
-* compute/global-type-param2.slang
+* compute/interface-func-param-in-struct.slang
+* compute/interface-assoc-type-param.slang
+
+### Generic Specialization Parameters
+
+These tests make use of generic specialization parameters in ways that don't easily align with the implementation approach that is more focused on existential parameters.
+They should either be ported to use existentials directly (at which point we potentially get rid of support for generic specialization at global or entry-point scope?) or we should refine the implementation of generic specialization to be consistent with existential specialization.
+
 * compute/global-type-param-array.slang
 * compute/global-type-param1.slang
+* compute/global-type-param2.slang
+* bugs/gh-357.slang
+* compute/assoctype-generic-arg.slang
+* compute/global-generic-value-param.slang
+* compute/global-type-param-in-entrypoint.slang
+* compute/global-type-param.slang
+* compute/int-generic.slang
+
+### Static Specialization
+
+These tests rely on the ability of the static specialization path to provide locations for data that doesn't "fit" into the fixed-size payload of an existential-type field/value.
+They will need to wait until the shader object implementation(s) are updated to support that case.
+
 * compute/interface-shader-param-in-struct.slang
 * compute/interface-shader-param-legalization.slang
+
+### Uncategorized
+
+These tests need to be binned according to what features they need.
+
 * compute/interface-shader-param.slang
 * compute/performance-profile.slang
 * compute/rewriter-parameter-block-complex.hlsl
@@ -40,4 +78,3 @@ These tests will need to be re-enabled together with changes to the shader objec
 * render/tess.hlsl
 * render/unused-discard.hlsl
 * compute/interface-param-partial-specialize.slang
-* compute/array-existential-parameter.slang