diff options
| author | Tim Foley <tfoleyNV@users.noreply.github.com> | 2019-01-14 14:27:44 -0800 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2019-01-14 14:27:44 -0800 |
| commit | 5b0c5076326b98d0e9ee0f7bddda7f619676707c (patch) | |
| tree | a08bc1710042bed328541a1d1b4e439ef4a70f50 /tests | |
| parent | 35dfe37695b5703bc7d61aa03b17f195ee4c9e07 (diff) | |
Add an error for global uniform parameter declarations (#773)
A global uniform parameter in HLSL might canonically be defined like this:
```hlsl
uniform float gSomeParameter;
```
The fxc and dxc compilers automatically collect all such parameters into a synthesized constant buffer, along the lines of:
```hlsl
cbuffer $Globals
{
float gSomeParameter;
}
```
Slang currently supports parsing and semantic checking of declarations like the above, and computes shader parameter layout/binding information that is appropriate for a constant buffer like `$Globals` above, but it does not include the support to emit HLSL or GLSL code that matches that layout, so that use of global uniforms in Slang is silently unsupported.
Making this problem worse, the HLSL language is quite lax, and will parse the following as shader parameters as well:
```hlsl
int gCounter = 0;
const float kScaleFactor = 2.0f;
```
Each of those declarations introduces a global shader parameter, and then provides a default value for it via the initializer. These declarations do *not* introduce an ordinary global variable or constant as might be expected.
(For anybody who wants to know, `static` is required to introduce a "real" global variable (although it will be a *thread-local* global in practice), while `static const` is required to introduce a global constant)
I was not too worried about users trying to use global-scope uniforms and failing (since that has fallen out of common HLSL/GLSL practice), but the possibility that users might try to declare global variables/constants and get shader parameters by mistake creates more of a risk so that this hole is worth plugging.
The right long-term fix is of course to support the intended semantics of global-scope uniforms, but that feature needs to be prioritized against other requests.
A few of the Slang tests were unwittingly relying on this functionality, including some compute tests that seemingly got away with it based on the DXBC generated from the HLSL output by Slang just happening to match the layout they expected. These tests have all been tweaked to use explicit `cbuffer`s or `ParameterBlock`s instead.
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/compute/global-type-param1.slang | 7 | ||||
| -rw-r--r-- | tests/compute/global-type-param2.slang | 2 | ||||
| -rw-r--r-- | tests/diagnostics/global-uniform.slang | 13 | ||||
| -rw-r--r-- | tests/diagnostics/global-uniform.slang.expected | 8 | ||||
| -rw-r--r-- | tests/reflection/global-type-params.slang | 4 | ||||
| -rw-r--r-- | tests/reflection/global-type-params.slang.expected | 52 | ||||
| -rw-r--r-- | tests/reflection/global-uniforms.hlsl | 1 |
7 files changed, 56 insertions, 31 deletions
diff --git a/tests/compute/global-type-param1.slang b/tests/compute/global-type-param1.slang index 08e548b81..e16ffa9da 100644 --- a/tests/compute/global-type-param1.slang +++ b/tests/compute/global-type-param1.slang @@ -28,9 +28,12 @@ struct Impl : IBase __generic_param TImpl : IBase; -TImpl impl; +ParameterBlock<TImpl> impl; -float base0; // = 0.5 +cbuffer C +{ + float base0; // = 0.5 +} Texture2D tex1; // = 0.0 SamplerState sampler; diff --git a/tests/compute/global-type-param2.slang b/tests/compute/global-type-param2.slang index 51b586cf7..f29d01407 100644 --- a/tests/compute/global-type-param2.slang +++ b/tests/compute/global-type-param2.slang @@ -40,7 +40,7 @@ struct Impl : IBase __generic_param TImpl : IBase; -TImpl impl; +ParameterBlock<TImpl> impl; // at binding c0: cbuffer existingBuffer diff --git a/tests/diagnostics/global-uniform.slang b/tests/diagnostics/global-uniform.slang new file mode 100644 index 000000000..a3f17e536 --- /dev/null +++ b/tests/diagnostics/global-uniform.slang @@ -0,0 +1,13 @@ +// global-uniform.slang +//TEST:SIMPLE:-target hlsl + + +// Any attempt to declare a global variable that actually declares a +// global uniform should be diagnosed as unsupported. + +uniform float a; + +const uint4 b = uint4(0,1,2,3); + +struct C { float x; int y; }; +C c; diff --git a/tests/diagnostics/global-uniform.slang.expected b/tests/diagnostics/global-uniform.slang.expected new file mode 100644 index 000000000..a77144c4f --- /dev/null +++ b/tests/diagnostics/global-uniform.slang.expected @@ -0,0 +1,8 @@ +result code = -1 +standard error = { +tests/diagnostics/global-uniform.slang(8): error 39016: 'a' is implicitly a global uniform shader parameter, which is currently unsupported by Slang. If a uniform parameter is intended, use a constant buffer or parameter block. If a global is intended, use the 'static' modifier. +tests/diagnostics/global-uniform.slang(10): error 39016: 'b' is implicitly a global uniform shader parameter, which is currently unsupported by Slang. If a uniform parameter is intended, use a constant buffer or parameter block. If a global is intended, use the 'static' modifier. +tests/diagnostics/global-uniform.slang(13): error 39016: 'c' is implicitly a global uniform shader parameter, which is currently unsupported by Slang. If a uniform parameter is intended, use a constant buffer or parameter block. If a global is intended, use the 'static' modifier. +} +standard output = { +} diff --git a/tests/reflection/global-type-params.slang b/tests/reflection/global-type-params.slang index bfeb7fb2e..74961b7cc 100644 --- a/tests/reflection/global-type-params.slang +++ b/tests/reflection/global-type-params.slang @@ -3,7 +3,6 @@ // Confirm that we handle global generic parameters -float4 u; interface IBase {}; @@ -24,10 +23,11 @@ SamplerState s; cbuffer CB { + float4 u; float4 v; + float4 w; } -float4 w; float4 main() : SV_Target { diff --git a/tests/reflection/global-type-params.slang.expected b/tests/reflection/global-type-params.slang.expected index 308738b55..8f6ba2838 100644 --- a/tests/reflection/global-type-params.slang.expected +++ b/tests/reflection/global-type-params.slang.expected @@ -5,18 +5,6 @@ standard output = { { "parameters": [ { - "name": "u", - "binding": {"kind": "uniform", "offset": 0, "size": 16}, - "type": { - "kind": "vector", - "elementCount": 4, - "elementType": { - "kind": "scalar", - "scalarType": "float32" - } - } - }, - { "name": "arg", "binding": {"kind": "generic", "index": 0}, "type": { @@ -65,14 +53,14 @@ standard output = { }, { "name": "CB", - "binding": {"kind": "constantBuffer", "index": 1}, + "binding": {"kind": "constantBuffer", "index": 0}, "type": { "kind": "constantBuffer", "elementType": { "kind": "struct", "fields": [ { - "name": "v", + "name": "u", "type": { "kind": "vector", "elementCount": 4, @@ -82,22 +70,34 @@ standard output = { } }, "binding": {"kind": "uniform", "offset": 0, "size": 16} + }, + { + "name": "v", + "type": { + "kind": "vector", + "elementCount": 4, + "elementType": { + "kind": "scalar", + "scalarType": "float32" + } + }, + "binding": {"kind": "uniform", "offset": 16, "size": 16} + }, + { + "name": "w", + "type": { + "kind": "vector", + "elementCount": 4, + "elementType": { + "kind": "scalar", + "scalarType": "float32" + } + }, + "binding": {"kind": "uniform", "offset": 32, "size": 16} } ] } } - }, - { - "name": "w", - "binding": {"kind": "uniform", "offset": 16, "size": 16}, - "type": { - "kind": "vector", - "elementCount": 4, - "elementType": { - "kind": "scalar", - "scalarType": "float32" - } - } } ], "entryPoints": [ diff --git a/tests/reflection/global-uniforms.hlsl b/tests/reflection/global-uniforms.hlsl index 884042cfa..6d4992db2 100644 --- a/tests/reflection/global-uniforms.hlsl +++ b/tests/reflection/global-uniforms.hlsl @@ -1,3 +1,4 @@ +//TEST_IGNORE_FILE //TEST:REFLECTION:-profile ps_4_0 -target hlsl // Confirm that we handle uniforms at global scope |
