summaryrefslogtreecommitdiffstats
path: root/tests
diff options
context:
space:
mode:
authorTim Foley <tfoleyNV@users.noreply.github.com>2019-03-26 12:49:02 -0700
committerGitHub <noreply@github.com>2019-03-26 12:49:02 -0700
commit88859d413e53e4228ae3b832d8bbd2711ccce84a (patch)
treee9a3547dbdae359b6a448af4bdaff6170ff9aec6 /tests
parentbedcb920045dd68f522e29d90fc3804f84bc08d0 (diff)
Allow plugging in types with resources for interface parameters (#913)
* Allow plugging in types with resources for interface parameters The key feature enabled by this change is that you can take a shader declared with interface-type parameters: ```hlsl ConstantBuffer<ILight> gLight; float4 myShader(IMaterial material, ...) { ... } ``` and specialize its interface-type parameters to concrete type that can contain resources like textures, samplers, etc. The hard part of doing this layout is that we need to support signatures that include a mix of interface and non-interface types. Imagine this contrived example: ```hlsl float4 myShader( Texture2D diffuseMap, ILight light, Texture2D specularMap) { ... } ``` We end up wanting `diffuseMap` to get `register(t0)` and `specularMap` to get `register(t1)`, so that they have the same location no matter what we plug in for `light`. But if we plug in a concrete type for `light` that needs a texture register, we need to allocate it *somewhere*. We handle this by having the `TypeLayout` for `light` come back with a "primary" type layout that doesn't have any texture registers, but with a "pending" type layout that includes the texture register requirements of whatever concrete type we plug in. This split between "primary" and "pending" layout then needs to work its way up the hierarchy, so that an aggregate `struct` type with a mix of interface and non-interface fields (recursively), needs to compute an aggregate "primary type layout" and an aggregate "pending type layout," and then each field needs to be able to compute its offset in the primary/pending layout of the aggregate. A large chunk of the work in this PR is then just implementing the split between primary and pending data, and ensuring that layouts are computed appropriately. The next catch is that when a "parameter group" (either a parameter block or constant buffer) contains one or more values of interface type, then we can allow the parameter group to "mask" some of the resource usage of the concrete types we plug in, but others "bleed through." For example, if we have: ```hlsl struct MyStuff { float3 color; ILight light; } ConstantBuffer<MyStuff> myStuff; struct SpotLight { float3 position; Texture2D shadowMap; } `` If we plug in the `SpotLight` type for `myStuff.light`, then the `float3` data for the light can be "masked" by the fact that we have a constant buffer (we can just allocate the `float3` `position` right after `color`), but the `Texture2D` needed for `shadowMap` needs to "bleed through" and become "pending" data for the `myStuff` shader parameter. Adding support for that detail more or less required a full rewrite of the logic for allocating parameter group type layouts. The next detail is that when we go to legalize a declaration like the `myStuff` buffer, we will end up with something like: ```hlsl struct MyStuff_stripped { float3 color; } struct Wrapped { MyStuff_stripped primary; SpotLight pending; } ConstantBuffer<Wrapped> myStuff; ``` This "wrapped" version of the buffer type more accurately reflects the layout we need/want for the uniform/ordinary data, but in order to further legalize it and pull out the resource-type fields like `shadowMap` we need to have accurate layout information, and the problem is that layout information for the original buffer can't apply to this new "wrapped" buffer. The last major piece of this change is logic that runs during existential type legalization to compute new layouts for "wrapped" buffers like these that embeds correct offset/binding/register information for any resources nested inside them. A key challenge in that code is that existential legalization needs to erase any "pending" data from the program entirely, so that offset information that used to be relatie to the "pending" part of a surrounding type now needs to be relative to the primary part. The work here may not be 100% complete for all scenarios, but it does well enough on the new and existing tests that I want to checkpoint it. Note that a few other tests have had their output changed, but in all cases I've reviewed the diffs and determined that the change in observable behavior is consistent with what we intened Slang's behavior to be. Note that there is still one major piece of support for interface-type parameters that is missing here, and which might force us to revisit some of the decisions in this code: we don't properly support user-defined `struct` types with interface-type fields. * fixup: typos
Diffstat (limited to 'tests')
-rw-r--r--tests/bugs/vk-structured-buffer-load.hlsl.glsl2
-rw-r--r--tests/compute/interface-shader-param3.slang37
-rw-r--r--tests/compute/interface-shader-param4.slang132
-rw-r--r--tests/compute/interface-shader-param4.slang.expected.txt4
-rw-r--r--tests/reflection/parameter-block-explicit-space.slang.expected10
5 files changed, 166 insertions, 19 deletions
diff --git a/tests/bugs/vk-structured-buffer-load.hlsl.glsl b/tests/bugs/vk-structured-buffer-load.hlsl.glsl
index 1cf0b00a7..206a25ef0 100644
--- a/tests/bugs/vk-structured-buffer-load.hlsl.glsl
+++ b/tests/bugs/vk-structured-buffer-load.hlsl.glsl
@@ -6,7 +6,7 @@ layout(row_major) uniform;
layout(row_major) buffer;
#extension GL_NV_ray_tracing : require
-layout(std430, binding = 2) readonly buffer _S1 {
+layout(std430, binding = 1) readonly buffer _S1 {
float _data[];
} gParamBlock_sbuf_0;
diff --git a/tests/compute/interface-shader-param3.slang b/tests/compute/interface-shader-param3.slang
index 3a9debaa0..3c8b24be1 100644
--- a/tests/compute/interface-shader-param3.slang
+++ b/tests/compute/interface-shader-param3.slang
@@ -4,20 +4,6 @@
// interface types at more complicated places in the overall layout.
//
-// NOTE TO SELF:
-//
-// First issue is that the constant buffer layouts aren't being
-// computed correctly for `gStrategy` (even in the previous test),
-// so that it doesn't get a `b` register bound.
-//
-// Second issue is that the type legalization logic is now overzealous,
-// and moves the `modifier` member of the entry-point constant buffer
-// out to global scope, when it should allocate it inside the constant
-// buffer.
-//
-
-
-
//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute
//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -dx12
//TEST(compute, vulkan):COMPARE_COMPUTE_EX:-vk -compute
@@ -59,7 +45,15 @@ int test(
//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):dxbinding(0),glbinding(0),out
RWStructuredBuffer<int> gOutputBuffer;
-//TEST_INPUT:cbuffer(data=[1 0 0 0], stride=4):dxbinding(0),glbinding(1)
+// Note: while we declare `gStrategy` here, there is no matching
+// line providing input data at this point. That is partly to
+// work around an apparent bug in the test runner, but it is also
+// to reflect the fact that this declaration does not cause a
+// constant buffer binding to be allocated, because just from
+// the declaration of `gStrategy` we cannot tell whether a
+// constant buffer binding is even needed (there might be no
+// uniform/ordinary data).
+//
ConstantBuffer<IRandomNumberGenerationStrategy> gStrategy;
[numthreads(4, 1, 1)]
@@ -87,7 +81,7 @@ void computeMain(
//
// Here's the incantation to make the test runner fill in the constant buffer:
//
-//TEST_INPUT:cbuffer(data=[256 0 0 0 16 0 0 0], stride=4):dxbinding(1),glbinding(2)
+//TEST_INPUT:cbuffer(data=[256 0 0 0 16 0 0 0], stride=4):dxbinding(0),glbinding(1)
//
// So, the value `256` will be used for `extra` and the value `16`
// will be written to the first four bytes of the concrete value
@@ -142,3 +136,14 @@ struct MyModifier : IModifier
//TEST_INPUT: globalExistentialType MyStrategy
//TEST_INPUT: entryPointExistentialType MyModifier
+
+// Once the concrete types are plugged in, the compiler can
+// see that `gStrategy` needs a constant buffer register/binding.
+// It will alocate the location for `gStrategy` after all other
+// shader parameters, to ensure that different specializations
+// of the same shader will agree on the locations for all the
+// non-interface-type parameters.
+//
+//TEST_INPUT:cbuffer(data=[1 0 0 0], stride=4):dxbinding(1),glbinding(2)
+
+
diff --git a/tests/compute/interface-shader-param4.slang b/tests/compute/interface-shader-param4.slang
new file mode 100644
index 000000000..07c6951e0
--- /dev/null
+++ b/tests/compute/interface-shader-param4.slang
@@ -0,0 +1,132 @@
+// interface-shader-param3.slang
+
+// This test builds on `interface-shader-param3.slang` by putting
+// resources into the concrete types that satisfy interface-type
+// shader parameters.
+//
+
+//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute
+//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -dx12
+//TEST(compute, vulkan):COMPARE_COMPUTE_EX:-vk -compute
+
+// A lot of the setup is the same as for `interface-shader-param`,
+// so look there if you want the comments.
+
+interface IRandomNumberGenerator
+{
+ [mutating]
+ int randomInt();
+}
+
+interface IRandomNumberGenerationStrategy
+{
+ associatedtype Generator : IRandomNumberGenerator;
+ Generator makeGenerator(int seed);
+}
+
+interface IModifier
+{
+ int modify(int val);
+}
+
+int test(
+ int seed,
+ IRandomNumberGenerationStrategy inStrategy,
+ IModifier modifier)
+{
+ let strategy = inStrategy;
+ var generator = strategy.makeGenerator(seed);
+ let unused = generator.randomInt();
+ let val = generator.randomInt();
+ let modifiedVal = modifier.modify(val);
+ return modifiedVal;
+}
+
+
+//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):dxbinding(0),glbinding(0),out
+RWStructuredBuffer<int> gOutputBuffer;
+
+ConstantBuffer<IRandomNumberGenerationStrategy> gStrategy;
+
+[numthreads(4, 1, 1)]
+void computeMain(
+
+// Similarly to the previous test, we are declaring two `uniform`
+// paameters on the entry point, where one will be plugged in
+// with a concrete type, and thus get laid out second.
+//
+ uniform IModifier modifier,
+ uniform int extra,
+//
+// The uniform/ordinary data for these two parameters will end
+// up in the constant buffers, so let's declare that. Unlike
+// the previous test, the concrete type plugged in for `modifier`
+// has no uniform/ordinary data, so we don't need to fill it in.
+//
+//TEST_INPUT:cbuffer(data=[256]):dxbinding(0),glbinding(2)
+
+ uint3 dispatchThreadID : SV_DispatchThreadID)
+{
+ let tid = dispatchThreadID.x;
+
+ let inputVal : int = tid;
+ let outputVal = test(inputVal, gStrategy, modifier)
+ + extra*extra;
+
+ gOutputBuffer[tid] = outputVal;
+}
+
+// Okay, now we get to the part that is unique starting
+// in this test: we add data to the concrete types
+// that we will use as parameters.
+
+struct MyStrategy : IRandomNumberGenerationStrategy
+{
+ RWStructuredBuffer<int> globalSeeds;
+
+ struct Generator : IRandomNumberGenerator
+ {
+ int state;
+
+ [mutating]
+ int randomInt()
+ {
+ return state++;
+ }
+ }
+
+ Generator makeGenerator(int seed)
+ {
+ Generator generator = { globalSeeds[seed] };
+ return generator;
+ }
+}
+
+struct MyModifier : IModifier
+{
+ RWStructuredBuffer<int> localModifiers;
+
+ int modify(int val)
+ {
+ return val ^ localModifiers[val & 3];
+ }
+}
+
+//TEST_INPUT: globalExistentialType MyStrategy
+//TEST_INPUT: entryPointExistentialType MyModifier
+
+// The concrete types we plug in for `gStrategy` and `modifier`
+// have buffer resources in them, so we need to assign them
+// data. The registers/bindings for these parameters will
+// always come after all other shader parameters, and their
+// relative order will match the relative order of their
+// declarations in the global order that Slang uses for
+// assigning bindings (all globals before all entry point parameters).
+//
+// Here's the data for `gStrategy`:
+//
+//TEST_INPUT:ubuffer(data=[1 2 4 8], stride=4):dxbinding(1),glbinding(1)
+//
+// Here's the data for `modifier`:
+//
+//TEST_INPUT:ubuffer(data=[16 32 64 128], stride=4):dxbinding(2),glbinding(3)
diff --git a/tests/compute/interface-shader-param4.slang.expected.txt b/tests/compute/interface-shader-param4.slang.expected.txt
new file mode 100644
index 000000000..e962124e1
--- /dev/null
+++ b/tests/compute/interface-shader-param4.slang.expected.txt
@@ -0,0 +1,4 @@
+10042
+10083
+10025
+10029
diff --git a/tests/reflection/parameter-block-explicit-space.slang.expected b/tests/reflection/parameter-block-explicit-space.slang.expected
index e683a641f..04093f4ca 100644
--- a/tests/reflection/parameter-block-explicit-space.slang.expected
+++ b/tests/reflection/parameter-block-explicit-space.slang.expected
@@ -6,7 +6,10 @@ standard output = {
"parameters": [
{
"name": "a",
- "binding": {"kind": "constantBuffer", "space": 2, "index": 0, "count": 0},
+ "bindings": [
+ {"kind": "constantBuffer", "space": 2, "index": 0, "count": 0},
+ {"kind": "registerSpace", "index": 2}
+ ],
"type": {
"kind": "parameterBlock",
"elementType": {
@@ -54,7 +57,10 @@ standard output = {
},
{
"name": "b",
- "binding": {"kind": "constantBuffer", "space": 3, "index": 0, "count": 0},
+ "bindings": [
+ {"kind": "constantBuffer", "space": 3, "index": 0, "count": 0},
+ {"kind": "registerSpace", "index": 3}
+ ],
"type": {
"kind": "parameterBlock",
"elementType": {