summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTim Foley <tfoleyNV@users.noreply.github.com>2020-02-19 14:36:44 -0800
committerGitHub <noreply@github.com>2020-02-19 14:36:44 -0800
commit788556aaaab1b5767e24cf86dc2f71fd285c06f5 (patch)
tree5c1e05db1994e19ff22f07fe2b1d414cd1dad2fa
parent9603fde866eacdde6c7cb93b04138b637a505079 (diff)
Don't allocate a default space for a VK push-constant buffer (#1231)
When a shader only uses `ParameterBlock`s plus a single buffer for root constants: ```hlsl ParameterBlock<A> a; ParameterBlock<B> b; [[vk::push_constant]] cbuffer Stuff { ... } ``` we expect the push-constant buffer should not affect the `space` allocated to the parameter blocks (so `a` should get `space=0`). This behavior wasn't being implemented correctly in `slang-parameter-binding.cpp`. There was logic to ignore certain resource kinds in entry-point parameter lists when computing whether a default space is needed, but the equivalent logic for the global scope only considered parameters that consuem whole register spaces/sets. This change shuffles the code around and makes sure it considers a global push-constant buffer as *not* needing a default space/set. Note that this change will have no impact on D3D targets, where `Stuff` above would always get put in `space0` because for D3D targets a push-constant buffer is no different from any other constant buffer in terms of register/space allocation. One unrelated point that this change brings to mind is the `[[vk::push_constant]]` should ideally also be allowed to apply to an entry point (where it would modify the default/implicit constant buffer). In fact, it could be argued that push-constant allocation should be the *default* for (non-RT) entry point `uniform` parameters (while `[[vk::shader_record]]` should be the default for RT entry point `uniform` parameters).
-rw-r--r--source/slang/hlsl.meta.slang.h1
-rw-r--r--source/slang/slang-parameter-binding.cpp24
-rw-r--r--tests/cross-compile/vk-push-constant-set.slang23
-rw-r--r--tests/cross-compile/vk-push-constant-set.slang.glsl28
4 files changed, 67 insertions, 9 deletions
diff --git a/source/slang/hlsl.meta.slang.h b/source/slang/hlsl.meta.slang.h
index b39c4bbcb..8614fd756 100644
--- a/source/slang/hlsl.meta.slang.h
+++ b/source/slang/hlsl.meta.slang.h
@@ -2030,7 +2030,6 @@ SLANG_RAW("// The ray query is effectively a coroutine that user shader\n")
SLANG_RAW("// code can resume to continue tracing the ray, and which yields\n")
SLANG_RAW("// back to the user code at interesting events along the ray.\n")
SLANG_RAW("//\n")
-SLANG_RAW("//__generic<let rayFlags : RAY_FLAG = RAY_FLAG_NONE>\n")
SLANG_RAW("__target_intrinsic(hlsl, RayQuery)\n")
SLANG_RAW("struct RayQuery <let rayFlags : RAY_FLAG = RAY_FLAG_NONE>\n")
SLANG_RAW("{\n")
diff --git a/source/slang/slang-parameter-binding.cpp b/source/slang/slang-parameter-binding.cpp
index 5cd89bd95..0268e1850 100644
--- a/source/slang/slang-parameter-binding.cpp
+++ b/source/slang/slang-parameter-binding.cpp
@@ -3343,14 +3343,7 @@ RefPtr<ProgramLayout> generateParameterBindings(
//
for (auto resInfo : varLayout->typeLayout->resourceInfos)
{
- // We don't care about whole register spaces/sets, since
- // we don't need to allocate a default space/set for a parameter
- // that itself consumes a whole space/set.
- //
- if( resInfo.kind == LayoutResourceKind::RegisterSpace )
- continue;
-
- // We also don't want to consider resource kinds for which
+ // We don't want to consider resource kinds for which
// the variable already has an (explicit) binding, since
// the space from the explicit binding will be used, so
// that a default space isn't needed.
@@ -3358,6 +3351,21 @@ RefPtr<ProgramLayout> generateParameterBindings(
if( parameterInfo->bindingInfo[resInfo.kind].count != 0 )
continue;
+ // We also want to exclude certain resource kinds from
+ // consideration, since parameters using those resource
+ // kinds wouldn't be allocated into the default space
+ // anyway.
+ //
+ switch( resInfo.kind )
+ {
+ case LayoutResourceKind::RegisterSpace:
+ case LayoutResourceKind::PushConstantBuffer:
+ continue;
+
+ default:
+ break;
+ }
+
// Otherwise, we have a shader parameter that will need
// a default space or set to live in.
//
diff --git a/tests/cross-compile/vk-push-constant-set.slang b/tests/cross-compile/vk-push-constant-set.slang
new file mode 100644
index 000000000..ee9c68ed8
--- /dev/null
+++ b/tests/cross-compile/vk-push-constant-set.slang
@@ -0,0 +1,23 @@
+// vk-push-constant.slang
+
+// Test to confirm that a `[[vk::push_constant]]` buffer
+// doesn't end up reserving `space=0` for global scope
+// parameters and shifting a parameer block over to
+// `space=1`.
+
+//TEST:CROSS_COMPILE:-target spirv-assembly -entry main -stage fragment
+
+struct S
+{
+ float4 v;
+}
+
+[[vk::push_constant]]
+ConstantBuffer<S> x;
+
+ParameterBlock<S> y;
+
+float4 main() : SV_Target
+{
+ return x.v + y.v;
+}
diff --git a/tests/cross-compile/vk-push-constant-set.slang.glsl b/tests/cross-compile/vk-push-constant-set.slang.glsl
new file mode 100644
index 000000000..189b14caf
--- /dev/null
+++ b/tests/cross-compile/vk-push-constant-set.slang.glsl
@@ -0,0 +1,28 @@
+// vk-push-constant.slang.glsl
+#version 450
+
+struct S_0
+{
+ vec4 v_0;
+};
+
+layout(push_constant)
+layout(std140) uniform _S1
+{
+ S_0 _data;
+} x_0;
+
+layout(binding = 0, set = 0)
+layout(std140) uniform _S2
+{
+ S_0 _data;
+} y_0;
+
+layout(location = 0)
+out vec4 _S3;
+
+void main()
+{
+ _S3 = x_0._data.v_0 + y_0._data.v_0;
+ return;
+}