From 788556aaaab1b5767e24cf86dc2f71fd285c06f5 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Wed, 19 Feb 2020 14:36:44 -0800 Subject: 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; ParameterBlock 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). --- source/slang/hlsl.meta.slang.h | 1 - source/slang/slang-parameter-binding.cpp | 24 ++++++++++++++++-------- 2 files changed, 16 insertions(+), 9 deletions(-) (limited to 'source') 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\n") SLANG_RAW("__target_intrinsic(hlsl, RayQuery)\n") SLANG_RAW("struct RayQuery \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 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 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. // -- cgit v1.2.3