Skip to content

Commit 788556a

Browse files
author
Tim Foley
authored
Don't allocate a default space for a VK push-constant buffer (shader-slang#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).
1 parent 9603fde commit 788556a

File tree

4 files changed

+67
-9
lines changed

4 files changed

+67
-9
lines changed

source/slang/hlsl.meta.slang.h

-1
Original file line numberDiff line numberDiff line change
@@ -2030,7 +2030,6 @@ SLANG_RAW("// The ray query is effectively a coroutine that user shader\n")
20302030
SLANG_RAW("// code can resume to continue tracing the ray, and which yields\n")
20312031
SLANG_RAW("// back to the user code at interesting events along the ray.\n")
20322032
SLANG_RAW("//\n")
2033-
SLANG_RAW("//__generic<let rayFlags : RAY_FLAG = RAY_FLAG_NONE>\n")
20342033
SLANG_RAW("__target_intrinsic(hlsl, RayQuery)\n")
20352034
SLANG_RAW("struct RayQuery <let rayFlags : RAY_FLAG = RAY_FLAG_NONE>\n")
20362035
SLANG_RAW("{\n")

source/slang/slang-parameter-binding.cpp

+16-8
Original file line numberDiff line numberDiff line change
@@ -3343,21 +3343,29 @@ RefPtr<ProgramLayout> generateParameterBindings(
33433343
//
33443344
for (auto resInfo : varLayout->typeLayout->resourceInfos)
33453345
{
3346-
// We don't care about whole register spaces/sets, since
3347-
// we don't need to allocate a default space/set for a parameter
3348-
// that itself consumes a whole space/set.
3349-
//
3350-
if( resInfo.kind == LayoutResourceKind::RegisterSpace )
3351-
continue;
3352-
3353-
// We also don't want to consider resource kinds for which
3346+
// We don't want to consider resource kinds for which
33543347
// the variable already has an (explicit) binding, since
33553348
// the space from the explicit binding will be used, so
33563349
// that a default space isn't needed.
33573350
//
33583351
if( parameterInfo->bindingInfo[resInfo.kind].count != 0 )
33593352
continue;
33603353

3354+
// We also want to exclude certain resource kinds from
3355+
// consideration, since parameters using those resource
3356+
// kinds wouldn't be allocated into the default space
3357+
// anyway.
3358+
//
3359+
switch( resInfo.kind )
3360+
{
3361+
case LayoutResourceKind::RegisterSpace:
3362+
case LayoutResourceKind::PushConstantBuffer:
3363+
continue;
3364+
3365+
default:
3366+
break;
3367+
}
3368+
33613369
// Otherwise, we have a shader parameter that will need
33623370
// a default space or set to live in.
33633371
//
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// vk-push-constant.slang
2+
3+
// Test to confirm that a `[[vk::push_constant]]` buffer
4+
// doesn't end up reserving `space=0` for global scope
5+
// parameters and shifting a parameer block over to
6+
// `space=1`.
7+
8+
//TEST:CROSS_COMPILE:-target spirv-assembly -entry main -stage fragment
9+
10+
struct S
11+
{
12+
float4 v;
13+
}
14+
15+
[[vk::push_constant]]
16+
ConstantBuffer<S> x;
17+
18+
ParameterBlock<S> y;
19+
20+
float4 main() : SV_Target
21+
{
22+
return x.v + y.v;
23+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// vk-push-constant.slang.glsl
2+
#version 450
3+
4+
struct S_0
5+
{
6+
vec4 v_0;
7+
};
8+
9+
layout(push_constant)
10+
layout(std140) uniform _S1
11+
{
12+
S_0 _data;
13+
} x_0;
14+
15+
layout(binding = 0, set = 0)
16+
layout(std140) uniform _S2
17+
{
18+
S_0 _data;
19+
} y_0;
20+
21+
layout(location = 0)
22+
out vec4 _S3;
23+
24+
void main()
25+
{
26+
_S3 = x_0._data.v_0 + y_0._data.v_0;
27+
return;
28+
}

0 commit comments

Comments
 (0)