summaryrefslogtreecommitdiffstats
path: root/tests/reflection
diff options
context:
space:
mode:
authorTim Foley <tfoleyNV@users.noreply.github.com>2019-12-06 09:29:09 -0800
committerGitHub <noreply@github.com>2019-12-06 09:29:09 -0800
commit54d3c5f6657b1099326e1ce7ec0f0692e7025442 (patch)
treeaa9baed99939fc3b068f3c330e4a59f5147020e4 /tests/reflection
parent4e2cfc95fb02fb47f02b8702494929e7cca3bec7 (diff)
Remove legacy feature for merging global shader parameters (#1139)
* Remove legacy feature for merging global shader parameters There is a fair amount of special-case code in the Slang compiler today to deal with the scenario where a programmer declares the "same" shader parameter across two different translation units: ```hlsl // a.hlsl Texture2D a; cbuffer C { float4 c; } ``` ```hlsl // b.hlsl cbuffer C { float4 c; } Texture2D b; ``` An important note here is that the declaration of `C` may be in a header file that both `a.hlsl` and `b.hlsl` `#include`, because from the standpoint of the parser and later stages of the compiler, there is no difference between `C` being in an included file vs. it being copy-pasted across both `a.hlsl` and `b.hlsl`. When a user invokes `slangc a.hlsl b.hlsl` (or the equivalent via the API), then they may decide that it is "obvious" that the shader parameter `C` is the "same" in both `a.hlsl` and `b.hlsl`. Knowing that the parameter is the "same" may lead them to make certain assumptions: * They may assume that generated code for entry points in `a.hlsl` and `b.hlsl` will both agree on the exact `register`/`binding` occupied by `C`. * They may assume that reflection information for their program will only reflect `C` once, and it will reflect it in a way that is applicable to entry points in both `a.hlsl` and `b.hlsl` * They may assume that the compiler can and should handle this use case even when `C` contains fields with `struct` types that are declared in both `a.hlsl` and `b.hlsl` that have the "same" definition. * They may assume that in cases where `C` is declared inconsistently between `a.hlsl` and `b.hlsl` the compiler can and will diagnose an error. Making these assumptions work in practice required a lot of special-case code: * When composing/linking programs was `ComponentType`s we had to include a special case `LegacyProgram` type that could provide these "do what I mean" semantics, since they are *not* what one would want in the general case for a `CompositeComponentType`. * During enumeration of global shader parameter in a `LegacyProgram`, we had to detect parameters from distinct modules (translation units) with the same name, and then enforce that they must have the "same" type (via an ad hoc recursive structural type match). No other semantic checking logic needs or uses that kind of structural check. * During parameter binding generation, we need to handle the case where a single global shader parameter might have multiple declarations, and make sure to collect explicit bindings from all of them (checking for inconsistency) and also to apply generated bindings to all of them. * The `mapVarToLayout` member in `StructTypeLayout` is a concession to the fact that we might have multiple `VarDecl`s for each field of the struct that represents the global scope, we might need to look up a field and its layout using any of those declarations (much of the need for this field had gone away now that IR passes are largely using IR-based layout). All of these different special cases added more complex code in many places in the compiler, all to support a scenario that isn't especially common. Most users won't be affected by the original issue, because they will do one of several things that rule it out: * Anybody using `slangc` like a stand-in for `fxc` or `dxc` and compiling one translation unit at a time will not suffer from any problems. If/when such users want consistent bindings across translation units, they already use either explicit binding or rely on consistent ordering and implicit binding. * Anybody who puts all the entry points that get combined into a pass/pipeline in a single file will not have problems. They will automatically get consistent bindings because of Slang's guarantees, and there can't be duplicated declarations when there is only one translation unit. * Anybody using `import` to factor out common declarations while compiling multiple translation units at once will not be affected. Parameters declared in an `import`ed module are the "same" in a much deeper way that it is trivial for Slang to support. Only users of the Falcor framework are likely to be affected by this, and they have two easy migration paths: either put related entry points into the same file, or factor common parameters into an `import`ed module. (It is also worth noting that for command-line `slangc`, it is possible to have a single module with multiple `.slang` files in it, which can all see global declarations like parameters across all the files. Anybody who buys into doing things the Slang Way should have no problem avoiding duplicated declarations) With the rationale out of the way, the actual change mostly just amounts to deleting lots of code that is no longer needed. An astute reviewer might notice several `assert`-fail conditions where complex Slang features were never actually made to work correctly with this legacy behavior. A small number of test cases broke with the code changes, but these were tests that specifically exercised the behavior being removed. In the case of the tests around binding/reflection generating, I rewrote the tests to use one of the idomatic workarounds (putting the shared parameters into an `import`ed module), but doing so required me to add support for `#include` when doing pass-through compilation with `fxc`. That logic added a bit more cruft than I had originally hoped to this commit, but having `#include` support when doing pass-through compilation is probably a net win. * fixup: 64-bit warning
Diffstat (limited to 'tests/reflection')
-rw-r--r--tests/reflection/multi-file-defines.h44
-rw-r--r--tests/reflection/multi-file-extra.hlsl51
-rw-r--r--tests/reflection/multi-file-shared.slang25
-rw-r--r--tests/reflection/multi-file.hlsl68
-rw-r--r--tests/reflection/multi-file.hlsl.expected82
5 files changed, 144 insertions, 126 deletions
diff --git a/tests/reflection/multi-file-defines.h b/tests/reflection/multi-file-defines.h
new file mode 100644
index 000000000..aed4c510a
--- /dev/null
+++ b/tests/reflection/multi-file-defines.h
@@ -0,0 +1,44 @@
+// multi-file-defines.h
+
+#ifdef __SLANG__
+#define R(X) /**/
+#define BEGIN_CBUFFER(NAME) cbuffer NAME
+#define END_CBUFFER(NAME, REG) /**/
+#define CBUFFER_REF(NAME, FIELD) FIELD
+#else
+#define R(X) /*X*/
+#define BEGIN_CBUFFER(NAME) struct SLANG_ParameterGroup_##NAME
+#define END_CBUFFER(NAME, REG) ; cbuffer NAME /*REG*/ { SLANG_ParameterGroup_##NAME NAME; }
+#define CBUFFER_REF(NAME, FIELD) NAME.FIELD
+
+#define sharedC sharedC_0
+#define sharedCA sharedCA_0
+#define sharedCB sharedCB_0
+#define sharedCC sharedCC_0
+#define sharedCD sharedCD_0
+
+#define vertexC vertexC_0
+#define vertexCA vertexCA_0
+#define vertexCB vertexCB_0
+#define vertexCC vertexCC_0
+#define vertexCD vertexCD_0
+
+#define fragmentC fragmentC_0
+#define fragmentCA fragmentCA_0
+#define fragmentCB fragmentCB_0
+#define fragmentCC fragmentCC_0
+#define fragmentCD fragmentCD_0
+
+#define sharedS sharedS_0
+#define sharedT sharedT_0
+#define sharedTV sharedTV_0
+#define sharedTF sharedTF_0
+
+#define vertexS vertexS_0
+#define vertexT vertexT_0
+
+#define fragmentS fragmentS_0
+#define fragmentT fragmentT_0
+
+#endif
+
diff --git a/tests/reflection/multi-file-extra.hlsl b/tests/reflection/multi-file-extra.hlsl
index be5757d14..701118d01 100644
--- a/tests/reflection/multi-file-extra.hlsl
+++ b/tests/reflection/multi-file-extra.hlsl
@@ -1,63 +1,32 @@
//TEST_IGNORE_FILE:
-// Here we are going to test that we can correctly generating bindings when we
-// are presented with a program spanning multiple input files (and multiple entry points)
-
-// This file provides the fragment shader, and is only meant to be tested in combination with `multi-file.hlsl`
-
-// Let's make sure we generate correct output in cases
-// where there are non-trivial `packoffset`s needed
+#include "multi-file-defines.h"
#ifdef __SLANG__
-#define R(X) /**/
+import multi_file_shared;
#else
-#define R(X) X
+#include "multi-file-shared.slang"
#endif
-float4 use(float val) { return val; };
-float4 use(float2 val) { return float4(val,0.0,0.0); };
-float4 use(float3 val) { return float4(val,0.0); };
-float4 use(float4 val) { return val; };
-float4 use(Texture2D t, SamplerState s) { return t.SampleLevel(s, 0.0, 0.0); }
-
-// Start with some parameters that will appear in both shaders
-Texture2D sharedT;
-SamplerState sharedS;
-cbuffer sharedC
-{
- float3 sharedCA;
- float sharedCB;
- float3 sharedCC;
- float2 sharedCD;
-}
-
-// Then some parameters specific to this shader.
-// These will be placed *after* the ones from the main file,
-// and even after the parameters further down in this file
-// that end up being shared between the two files.
+Texture2D fragmentT R(: register(t1));
+SamplerState fragmentS R(: register(s1));
-Texture2D fragmentT;
-SamplerState fragmentS;
-cbuffer fragmentC
+BEGIN_CBUFFER(fragmentC)
{
float3 fragmentCA;
float fragmentCB;
float3 fragmentCC;
float2 fragmentCD;
}
+END_CBUFFER(fragmentC, register(b1))
-// And end with some shared parameters again
-Texture2D sharedTV;
-Texture2D sharedTF;
-
-
-float4 mainVS() : SV_Position
+float4 mainFS() : SV_TARGET
{
// Go ahead and use everything here, just to make sure things got placed correctly
return use(sharedT, sharedS)
- + use(sharedCD)
+ + use(CBUFFER_REF(sharedC,sharedCD))
+ use(fragmentT, fragmentS)
- + use(fragmentCD)
+ + use(CBUFFER_REF(fragmentC, fragmentCD))
+ use(sharedTF, sharedS)
;
} \ No newline at end of file
diff --git a/tests/reflection/multi-file-shared.slang b/tests/reflection/multi-file-shared.slang
new file mode 100644
index 000000000..af91d5251
--- /dev/null
+++ b/tests/reflection/multi-file-shared.slang
@@ -0,0 +1,25 @@
+// multi-file-shared.slang
+//TEST_IGNORE_FILE:
+
+#include "multi-file-defines.h"
+
+float4 use(float val) { return val; };
+float4 use(float2 val) { return float4(val,0.0,0.0); };
+float4 use(float3 val) { return float4(val,0.0); };
+float4 use(float4 val) { return val; };
+float4 use(Texture2D t, SamplerState s) { return t.SampleLevel(s, 0.0, 0.0); }
+
+Texture2D sharedT R(: register(t2));
+SamplerState sharedS R(: register(s2));
+
+BEGIN_CBUFFER(sharedC)
+{
+ float3 sharedCA;
+ float sharedCB;
+ float3 sharedCC;
+ float2 sharedCD;
+}
+END_CBUFFER(sharedC, register(b2))
+
+Texture2D sharedTV R(: register(t3));
+Texture2D sharedTF R(: register(t4));
diff --git a/tests/reflection/multi-file.hlsl b/tests/reflection/multi-file.hlsl
index 4a9cf9a86..fd9235ab5 100644
--- a/tests/reflection/multi-file.hlsl
+++ b/tests/reflection/multi-file.hlsl
@@ -1,56 +1,36 @@
-//TEST:REFLECTION:-entry mainFS -profile ps_4_0 -target hlsl tests/reflection/multi-file-extra.hlsl -entry mainVS -profile vs_4_0
+//TEST:REFLECTION:-D__SLANG__ -entry mainVS -profile vs_4_0 -target hlsl tests/reflection/multi-file-extra.hlsl -entry mainFS -profile ps_4_0
// Here we are testing the case where multiple translation units are provided
// at once, so that we want combined reflection information for the resulting
// program. The other part of this program is in `multi-file-extra.hlsl`.
-float4 use(float val) { return val; };
-float4 use(float2 val) { return float4(val,0.0,0.0); };
-float4 use(float3 val) { return float4(val,0.0); };
-float4 use(float4 val) { return val; };
-float4 use(Texture2D t, SamplerState s)
-{
- // This is the vertex shader, so we can't do implicit-gradient sampling
- return t.SampleGrad(s, 0.0, 0.0, 0.0);
-}
+#include "multi-file-defines.h"
-// Start with some parameters that will appear in both shaders
-Texture2D sharedT;
-SamplerState sharedS;
-cbuffer sharedC
-{
- float3 sharedCA;
- float sharedCB;
- float3 sharedCC;
- float2 sharedCD;
-}
+#ifdef __SLANG__
+import multi_file_shared;
+#else
+#include "multi-file-shared.slang"
+#endif
-// Then some parameters specific to this shader
-// (these will get placed before the ones in the `extra` file,
-// based on how they get named on the command-line)
+Texture2D vertexT R(: register(t0));
+SamplerState vertexS R(: register(s0));
-Texture2D vertexT;
-SamplerState vertexS;
-cbuffer vertexC
+BEGIN_CBUFFER(vertexC)
{
- float3 vertexCA;
- float vertexCB;
- float3 vertexCC;
- float2 vertexCD;
+ float3 vertexCA;
+ float vertexCB;
+ float3 vertexCC;
+ float2 vertexCD;
}
+END_CBUFFER(vertexC, register(b0))
-// And end with some shared parameters again
-Texture2D sharedTV;
-Texture2D sharedTF;
-
-
-float4 mainFS() : SV_Target
+float4 mainVS() : SV_POSITION
{
- // Go ahead and use everything here, just to make sure things got placed correctly
- return use(sharedT, sharedS)
- + use(sharedCD)
- + use(vertexT, vertexS)
- + use(vertexCD)
- + use(sharedTV, vertexS)
- ;
-} \ No newline at end of file
+ // Go ahead and use everything here, just to make sure things got placed correctly
+ return use(sharedT, sharedS)
+ + use(CBUFFER_REF(sharedC, sharedCD))
+ + use(vertexT, vertexS)
+ + use(CBUFFER_REF(vertexC, vertexCD))
+ + use(sharedTV, vertexS)
+ ;
+}
diff --git a/tests/reflection/multi-file.hlsl.expected b/tests/reflection/multi-file.hlsl.expected
index 4ad95fb35..814c648e0 100644
--- a/tests/reflection/multi-file.hlsl.expected
+++ b/tests/reflection/multi-file.hlsl.expected
@@ -5,7 +5,7 @@ standard output = {
{
"parameters": [
{
- "name": "sharedT",
+ "name": "vertexT",
"binding": {"kind": "shaderResource", "index": 0},
"type": {
"kind": "resource",
@@ -13,14 +13,14 @@ standard output = {
}
},
{
- "name": "sharedS",
+ "name": "vertexS",
"binding": {"kind": "samplerState", "index": 0},
"type": {
"kind": "samplerState"
}
},
{
- "name": "sharedC",
+ "name": "vertexC",
"binding": {"kind": "constantBuffer", "index": 0},
"type": {
"kind": "constantBuffer",
@@ -28,7 +28,7 @@ standard output = {
"kind": "struct",
"fields": [
{
- "name": "sharedCA",
+ "name": "vertexCA",
"type": {
"kind": "vector",
"elementCount": 3,
@@ -40,7 +40,7 @@ standard output = {
"binding": {"kind": "uniform", "offset": 0, "size": 12}
},
{
- "name": "sharedCB",
+ "name": "vertexCB",
"type": {
"kind": "scalar",
"scalarType": "float32"
@@ -48,7 +48,7 @@ standard output = {
"binding": {"kind": "uniform", "offset": 12, "size": 4}
},
{
- "name": "sharedCC",
+ "name": "vertexCC",
"type": {
"kind": "vector",
"elementCount": 3,
@@ -60,7 +60,7 @@ standard output = {
"binding": {"kind": "uniform", "offset": 16, "size": 12}
},
{
- "name": "sharedCD",
+ "name": "vertexCD",
"type": {
"kind": "vector",
"elementCount": 2,
@@ -76,7 +76,7 @@ standard output = {
}
},
{
- "name": "vertexT",
+ "name": "fragmentT",
"binding": {"kind": "shaderResource", "index": 1},
"type": {
"kind": "resource",
@@ -84,14 +84,14 @@ standard output = {
}
},
{
- "name": "vertexS",
+ "name": "fragmentS",
"binding": {"kind": "samplerState", "index": 1},
"type": {
"kind": "samplerState"
}
},
{
- "name": "vertexC",
+ "name": "fragmentC",
"binding": {"kind": "constantBuffer", "index": 1},
"type": {
"kind": "constantBuffer",
@@ -99,7 +99,7 @@ standard output = {
"kind": "struct",
"fields": [
{
- "name": "vertexCA",
+ "name": "fragmentCA",
"type": {
"kind": "vector",
"elementCount": 3,
@@ -111,7 +111,7 @@ standard output = {
"binding": {"kind": "uniform", "offset": 0, "size": 12}
},
{
- "name": "vertexCB",
+ "name": "fragmentCB",
"type": {
"kind": "scalar",
"scalarType": "float32"
@@ -119,7 +119,7 @@ standard output = {
"binding": {"kind": "uniform", "offset": 12, "size": 4}
},
{
- "name": "vertexCC",
+ "name": "fragmentCC",
"type": {
"kind": "vector",
"elementCount": 3,
@@ -131,7 +131,7 @@ standard output = {
"binding": {"kind": "uniform", "offset": 16, "size": 12}
},
{
- "name": "vertexCD",
+ "name": "fragmentCD",
"type": {
"kind": "vector",
"elementCount": 2,
@@ -147,7 +147,7 @@ standard output = {
}
},
{
- "name": "sharedTV",
+ "name": "sharedT",
"binding": {"kind": "shaderResource", "index": 2},
"type": {
"kind": "resource",
@@ -155,30 +155,14 @@ standard output = {
}
},
{
- "name": "sharedTF",
- "binding": {"kind": "shaderResource", "index": 3},
- "type": {
- "kind": "resource",
- "baseShape": "texture2D"
- }
- },
- {
- "name": "fragmentT",
- "binding": {"kind": "shaderResource", "index": 4},
- "type": {
- "kind": "resource",
- "baseShape": "texture2D"
- }
- },
- {
- "name": "fragmentS",
+ "name": "sharedS",
"binding": {"kind": "samplerState", "index": 2},
"type": {
"kind": "samplerState"
}
},
{
- "name": "fragmentC",
+ "name": "sharedC",
"binding": {"kind": "constantBuffer", "index": 2},
"type": {
"kind": "constantBuffer",
@@ -186,7 +170,7 @@ standard output = {
"kind": "struct",
"fields": [
{
- "name": "fragmentCA",
+ "name": "sharedCA",
"type": {
"kind": "vector",
"elementCount": 3,
@@ -198,7 +182,7 @@ standard output = {
"binding": {"kind": "uniform", "offset": 0, "size": 12}
},
{
- "name": "fragmentCB",
+ "name": "sharedCB",
"type": {
"kind": "scalar",
"scalarType": "float32"
@@ -206,7 +190,7 @@ standard output = {
"binding": {"kind": "uniform", "offset": 12, "size": 4}
},
{
- "name": "fragmentCC",
+ "name": "sharedCC",
"type": {
"kind": "vector",
"elementCount": 3,
@@ -218,7 +202,7 @@ standard output = {
"binding": {"kind": "uniform", "offset": 16, "size": 12}
},
{
- "name": "fragmentCD",
+ "name": "sharedCD",
"type": {
"kind": "vector",
"elementCount": 2,
@@ -232,16 +216,32 @@ standard output = {
]
}
}
+ },
+ {
+ "name": "sharedTV",
+ "binding": {"kind": "shaderResource", "index": 3},
+ "type": {
+ "kind": "resource",
+ "baseShape": "texture2D"
+ }
+ },
+ {
+ "name": "sharedTF",
+ "binding": {"kind": "shaderResource", "index": 4},
+ "type": {
+ "kind": "resource",
+ "baseShape": "texture2D"
+ }
}
],
"entryPoints": [
{
- "name": "mainFS",
- "stage:": "fragment"
- },
- {
"name": "mainVS",
"stage:": "vertex"
+ },
+ {
+ "name": "mainFS",
+ "stage:": "fragment"
}
]
}