summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTim Foley <tfoleyNV@users.noreply.github.com>2019-04-29 08:32:24 -0700
committerGitHub <noreply@github.com>2019-04-29 08:32:24 -0700
commitb7c60910367f2af2c359d76783975de0a4659c68 (patch)
treeb5a4fd9a6435737d85b8711e854bfb01ba862933
parentd3709c0a58eae582d3750310a9a19b5593876e3b (diff)
Enable appropriate GLSL extension for unbounded-size resource arrays (#957)
Fixes #941 The GLSL we were emitting for unbounded-size arrays was the obvious: ```hlsl // This HLSL: Texture2D t[]; ``` ```glsl // ... becomes this GLSL: texture2D t[]; ``` Unfortunately, the legacy GLSL behavior for an array without a declared size is what is called an "implicitly-sized" array, which means that it is assumed to actually have a fixed size, which is determined by the maximum integer constant value used to index into it (and only integer constants are allowed to be used when indexing into it). Users hadn't noticed the issue for a while, because most of our users who rely on unbounded-size arrays were also using the HLSL `NonUniformResourceIndex` function: ```hlsl float4 v = t[NonUniformResourceIndex(idx)].Sample(...); ``` When mapping such code to GLSL we use the `nonuniformEXT` qualifier added by the `GL_EXT_nonuniform_qualifier` extension, and it turns out that a secondary feature of that extension is that it changes the GLSL language semantics for arrays (of resources) with an unspecified size, so that they instead behave like we want. So users were happy and we were blissfully ignorant of the lurking issue. The problem is that as soon as a user neglects to use `NonUniformResourceIndex` (perhaps because an index really is uniform): ```hlsl cbuffer C { uint definitelyUniform; } ... float4 v = t[definitelyUniform].Sample(...); ``` Now the code we emit doesn't need `nonuniformEXT` so it doesn't enable `GL_EXT_nonuniform_qualifier` and the declaration of `t` now falls under the "implicitly-sized" array rules, and thus the code fails because `definitelyUniform` is being used as an index but is *not* an integer constant. The fix is pretty simple: when emitting a declaration of a global shader parameter to GLSL, we check if it is an unbounded-size array of resources and, if so, enable the `GL_EXT_nonuniform_qualifier` extension. We don't need any clever handling to deal with resource parameters nested in `struct` types or in entry-point parameter lists, etc., because previous IR passes will have split up complex types and moved everything to the global scope already.
-rw-r--r--source/slang/emit.cpp19
-rw-r--r--tests/bugs/gh-941.slang17
-rw-r--r--tests/bugs/gh-941.slang.glsl38
3 files changed, 74 insertions, 0 deletions
diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp
index c17bb3266..e69ee2a93 100644
--- a/source/slang/emit.cpp
+++ b/source/slang/emit.cpp
@@ -6542,6 +6542,25 @@ struct EmitVisitor
return;
}
}
+
+ // When emitting unbounded-size resource arrays with GLSL we need
+ // to use the `GL_EXT_nonuniform_qualifier` extension to ensure
+ // that they are not treated as "implicitly-sized arrays" which
+ // are arrays that have a fixed size that just isn't specified
+ // at the declaration site (instead being inferred from use sites).
+ //
+ // While the extension primarily introduces the `nonuniformEXT`
+ // qualifier that we use to implement `NonUniformResourceIndex`,
+ // it also changes the GLSL language semantics around (resource) array
+ // declarations that don't specify a size.
+ //
+ if( as<IRUnsizedArrayType>(varType) )
+ {
+ if(isResourceType(unwrapArray(varType)))
+ {
+ requireGLSLExtension("GL_EXT_nonuniform_qualifier");
+ }
+ }
}
// Need to emit appropriate modifiers here.
diff --git a/tests/bugs/gh-941.slang b/tests/bugs/gh-941.slang
new file mode 100644
index 000000000..f66c25e97
--- /dev/null
+++ b/tests/bugs/gh-941.slang
@@ -0,0 +1,17 @@
+//TEST:CROSS_COMPILE: -profile ps_5_0 -entry main -target spirv-assembly
+
+// Ensure that we add the `GL_EXT_nonuniform_qualifier` extension for any code that uses unbounded-size arrays of resources.
+
+Texture2D t[];
+SamplerState s;
+
+cbuffer C
+{
+ float2 uv;
+ uint index;
+}
+
+float4 main() : SV_Target
+{
+ return t[index].Sample(s, uv);
+}
diff --git a/tests/bugs/gh-941.slang.glsl b/tests/bugs/gh-941.slang.glsl
new file mode 100644
index 000000000..d3c29820d
--- /dev/null
+++ b/tests/bugs/gh-941.slang.glsl
@@ -0,0 +1,38 @@
+//TEST_IGNORE_FILE:
+
+#version 450
+
+#extension GL_EXT_nonuniform_qualifier : require
+
+struct SLANG_ParameterGroup_C_0
+{
+ vec2 uv_0;
+ uint index_0;
+};
+
+layout(binding = 2)
+layout(std140)
+uniform _S1
+{
+ SLANG_ParameterGroup_C_0 _data;
+} C_0;
+
+layout(binding = 0)
+uniform texture2D t_0[];
+
+layout(binding = 1)
+uniform sampler s_0;
+
+layout(location = 0)
+out vec4 _S2;
+
+void main()
+{
+ vec4 _S3 = texture(
+ sampler2D(
+ t_0[C_0._data.index_0],
+ s_0),
+ C_0._data.uv_0);
+ _S2 = _S3;
+ return;
+} \ No newline at end of file