Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Metal type layout reflection for nested parameter blocks. #6042

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions source/slang/slang-type-layout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -845,6 +845,8 @@ struct GLSLObjectLayoutRulesImpl : ObjectLayoutRulesImpl
{
case ShaderParameterKind::SubpassInput:
return SimpleLayoutInfo(LayoutResourceKind::InputAttachmentIndex, slotCount);
case ShaderParameterKind::ParameterBlock:
return SimpleLayoutInfo(LayoutResourceKind::SubElementRegisterSpace, 1);
default:
break;
}
Expand Down Expand Up @@ -891,7 +893,8 @@ struct HLSLObjectLayoutRulesImpl : ObjectLayoutRulesImpl
{
case ShaderParameterKind::ConstantBuffer:
return SimpleLayoutInfo(LayoutResourceKind::ConstantBuffer, 1);

case ShaderParameterKind::ParameterBlock:
return SimpleLayoutInfo(LayoutResourceKind::SubElementRegisterSpace, 1);
case ShaderParameterKind::TextureUniformBuffer:
case ShaderParameterKind::StructuredBuffer:
case ShaderParameterKind::RawBuffer:
Expand Down Expand Up @@ -1180,6 +1183,7 @@ struct CPUObjectLayoutRulesImpl : ObjectLayoutRulesImpl
switch (kind)
{
case ShaderParameterKind::ConstantBuffer:
case ShaderParameterKind::ParameterBlock:
// It's a pointer to the actual uniform data
return SimpleLayoutInfo(
LayoutResourceKind::Uniform,
Expand Down Expand Up @@ -1262,6 +1266,7 @@ struct CUDAObjectLayoutRulesImpl : CPUObjectLayoutRulesImpl
switch (kind)
{
case ShaderParameterKind::ConstantBuffer:
case ShaderParameterKind::ParameterBlock:
{
// It's a pointer to the actual uniform data
return SimpleLayoutInfo(
Expand Down Expand Up @@ -1856,6 +1861,7 @@ struct MetalObjectLayoutRulesImpl : ObjectLayoutRulesImpl
switch (kind)
{
case ShaderParameterKind::ConstantBuffer:
case ShaderParameterKind::ParameterBlock:
case ShaderParameterKind::StructuredBuffer:
case ShaderParameterKind::MutableStructuredBuffer:
case ShaderParameterKind::RawBuffer:
Expand Down Expand Up @@ -1917,6 +1923,7 @@ struct MetalArgumentBufferElementLayoutRulesImpl : ObjectLayoutRulesImpl, Defaul
switch (kind)
{
case ShaderParameterKind::ConstantBuffer:
case ShaderParameterKind::ParameterBlock:
case ShaderParameterKind::StructuredBuffer:
case ShaderParameterKind::MutableStructuredBuffer:
case ShaderParameterKind::RawBuffer:
Expand Down Expand Up @@ -2341,6 +2348,10 @@ static SimpleLayoutInfo _getParameterGroupLayoutInfo(
}
else if (as<ParameterBlockType>(type))
{
auto info =
rules->GetObjectLayout(ShaderParameterKind::ParameterBlock, context.objectLayoutOptions)
.getSimple();

// Note: we default to consuming zero register spces here, because
// a parameter block might not contain anything (or all it contains
// is other blocks), and so it won't get a space allocated.
Expand All @@ -2351,7 +2362,9 @@ static SimpleLayoutInfo _getParameterGroupLayoutInfo(
//
// TODO: wouldn't it be any different to just allocate this
// as an empty `SimpleLayoutInfo` of any other kind?
return SimpleLayoutInfo(LayoutResourceKind::SubElementRegisterSpace, 0);
if (info.kind == LayoutResourceKind::SubElementRegisterSpace)
info.size = 0;
return info;
}

// TODO: the vertex-input and fragment-output cases should
Expand Down Expand Up @@ -4049,6 +4062,21 @@ RefPtr<VarLayout> StructTypeLayoutBuilder::addField(
RefPtr<TypeLayout> fieldTypeLayout = fieldResult.layout;
UniformLayoutInfo fieldInfo = fieldResult.info.getUniformLayout();

if (fieldTypeLayout->resourceInfos.getCount() == 0)
{
if (auto paramGroupTypeLayout = as<ParameterGroupTypeLayout>(fieldTypeLayout))
{
// If field type layout is a parameter block and it has a size that is not just a space,
// we need to count for it in the struct layout.
auto containerTypeLayout = paramGroupTypeLayout->containerVarLayout->getTypeLayout();
if (containerTypeLayout->FindResourceInfo(
LayoutResourceKind::SubElementRegisterSpace) == nullptr)
{
fieldTypeLayout = containerTypeLayout;
}
}
}

// Note: we don't add any zero-size fields
// when computing structure layout, just
// to avoid having a resource type impact
Expand Down Expand Up @@ -4079,7 +4107,7 @@ RefPtr<VarLayout> StructTypeLayoutBuilder::addField(
// for each field of the structure.
RefPtr<VarLayout> fieldLayout = new VarLayout();
fieldLayout->varDecl = field;
fieldLayout->typeLayout = fieldTypeLayout;
fieldLayout->typeLayout = fieldResult.layout;
m_typeLayout->fields.add(fieldLayout);

if (field)
Expand Down
2 changes: 2 additions & 0 deletions source/slang/slang-type-layout.h
Original file line number Diff line number Diff line change
Expand Up @@ -975,6 +975,8 @@ enum class ShaderParameterKind
AtomicUint,

SubpassInput,

ParameterBlock,
};

struct SimpleLayoutRulesImpl
Expand Down
18 changes: 18 additions & 0 deletions tests/metal/nested-parameter-block-reflection.slang
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//TEST:REFLECTION(filecheck=CHECK): -target metal

// CHECK:"name": "tex",
// CHECK:"binding": {"kind": "metalArgumentBufferElement", "index": 1}

struct Data { int3 content; }
struct Params
{
ParameterBlock<Data> pdata;
Texture2D tex;
}
ParameterBlock<Params> gParams;
RWStructuredBuffer<float4> output;
[numthreads(1,1,1)]
void computeMain()
{
output[0] = gParams.tex.Load(gParams.pdata.content);
}
Loading