Skip to content

Commit 79f6a01

Browse files
author
Tim Foley
authored
Change rules for layout of buffers/blocks containing only interface types (shader-slang#1318)
TL;DR: This is a tweak the rules for layout that only affects a corner case for people who actually use `interface`-type shader parameters (which for now is just our own test cases). The tweaked rules seem like they make it easier to write the application code for interfacing with Slang, but even if we change our minds later the risk here should be low (again: nobody is using this stuff right now). Slang already has a rule that a constant buffer that contains no ordinary/uniform data doesn't actually allocate a constant buffer `binding`/`register`: struct A { float4 x; Texture2D y; } // has uniform/ordinary data struct B { Texture2D u; SamplerState v; } // has none ConstantBuffer<A> gA; // gets a constant buffer register/binding ConstantBuffer<B> gB; // does not There is similar logic for `ParameterBlock`, where the feature makes more sense. A user would be somewhat surprised if they declared a parmaeter block with a texture and a sampler in it, but then the generating code reserved Vulkan `binding=0` for a constant buffer they never asked for. The behavior in the case of a plain `ConstantBuffer` is chosen to be consistent with the parameter block case. (Aside: all of this is a non-issue for targets with direct support for pointers, like CUDA and CPU. On those platforms a constant buffer or parameter block always translates to a pointer to the contained data.) Now, suppose the user declares a constant buffer with an interface type in it: interface IFoo { ... } ConstantBuffer<IFoo> gBuffer; When the layout logic sees the declaration of `gBuffer` it doesn't yet know what type will be plugged in as `IFoo` there. Will it contain uniform/ordinary data, such that a constant buffer is needed? The existing logic in the type layout step implemented a complicated rule that amounted to: * A `ConstantBuffer` or `cbuffer` that only contains `interface`/existential-type data will *not* be allocated a constant buffer `register`/`binding` during the initial layout process (on unspecialized code). That means that any resources declared after it will take the next consecutive `register`/`binding` without leaving any "gap" for the `ConstantBuffer` variable. * After specialization (e.g., when we know that `Thing` should be plugged in for `IFoo`), if we discover that there is uniform/ordinary data in `Thing` then we will allocate a constant buffer `register`/`binding` for the `ConstantBuffer`, but that register/binding will necessarily come *after* any `register`s/`binding`s that were allocated to parameters during the first pass. * Parameter blocks were intended to work the same when when it comes to whether or not they allocate a default `space`/`set`, but that logic appears to not have worked as intended. These rules make some logical sense: a `ConstantBuffer` declaration only pays for what the element type actually needs, and if that changes due to specialization then the new resource allocation comes after the unspecialized resources (so that the locations of unspecialized parameters are stable across specializations). The problem is that in practice it is almost impossible to write client application code that uses the Slang reflection API and makes reasonable choices in the presence of these rules. A general-purpose `ShaderObject` abstraction in application code ends up having to deal with multiple possible states that an object could be in: 1. An object where the element type `E` contains no uniform/ordinary data, and no interface/existential fields, so a constant buffer doesn't need to be allocated or bound. 2. An object where the element type `E` contains no uniform/ordinary data, but has interace/existential fields, with two sub-cases: a. When no values bound to interface/existential fields use uniform/ordinary dat, then the parent object must not bind a buffer b. When the type of value bound to an interface/existential field uses uniform/ordinary data, then the parent object needs to have a buffer allocated, and bind it. 3. When the element type `E` contains uniform/ordinary data, then a buffer should be allocated and bound (although its size/contents may change as interface/existential fields get re-bound) Needing to deal with a possible shift between cases (2a) and (2b) based on what gets bound at runtime is a mess, and it is important to note that even though both (2a) and (3) require a buffer to be bound, the rules about *where* the buffer gets bound aren't consistent (so that the application needs to undrestand the distinction between "primary" and "pending" data in a type layout). This change introduces a different rule, which seems to be more complicated to explain, but actually seems to simplify things for the application: * A `ConstantBuffer` or `cbuffer` that only contains `interface`/existential-type data always has a constant buffer `register`/`binding` allocated for it "just in case." * If after specialization there is any uniform/ordinary data, then that will use the buffer `register`/`binding` that was already allocated (that's easy enough). * If after speciazliation there *isn't* any uniform/ordinary data, then the generated HLSL/GLSL shader code won't declare a buffer, but the `register`/`binding` is still claimed. * A `ParameterBlock` behaves equivalently, so that if it contains any `interface`/existential fields, then it will always allocate a `space`/`set` "just in case" The effect of these rules is to streamline the cases that an application needs to deal with down to two: 1. If the element type `E` of a shader object contains no uniform/ordinary or interface/existential fields, then no buffer needs to be allocated or bound 2. If the element type `E` contains *any* uniform/ordinary or interface/existential fields, then it is always safe to allocate and bind a buffer (even in the cases where it might be ignored). Furthermore, the reflection data for the constant buffer `register`/`binding` becomes consistent in case (2), so that the application can always expect to find it in the same way.
1 parent b2c9fcc commit 79f6a01

File tree

3 files changed

+141
-102
lines changed

3 files changed

+141
-102
lines changed

source/slang/slang-type-layout.cpp

+111-99
Original file line numberDiff line numberDiff line change
@@ -1797,6 +1797,11 @@ static bool _usesOrdinaryData(RefPtr<TypeLayout> typeLayout)
17971797
return _usesResourceKind(typeLayout, LayoutResourceKind::Uniform);
17981798
}
17991799

1800+
static bool _usesExistentialData(RefPtr<TypeLayout> typeLayout)
1801+
{
1802+
return _usesResourceKind(typeLayout, LayoutResourceKind::ExistentialObjectParam);
1803+
}
1804+
18001805
/// Add resource usage from `srcTypeLayout` to `dstTypeLayout` unless it would be "masked."
18011806
///
18021807
/// This function is appropriate for applying resource usage from an element type
@@ -1918,9 +1923,32 @@ static RefPtr<TypeLayout> _createParameterGroupTypeLayout(
19181923
//
19191924
// To determine if we actually need a constant-buffer binding,
19201925
// we will inspect the element type and see if it contains
1921-
// any ordinary/uniform data.
1926+
// any ordinary/uniform data *or* any interface/existential-type
1927+
// slots.
1928+
//
1929+
// The latter detail might sound surprising, because it means
1930+
// that for a declaration like:
19221931
//
1923-
bool wantConstantBuffer = _usesOrdinaryData(rawElementTypeLayout);
1932+
// cbuffer U { IThing gThing; }
1933+
//
1934+
// we will allocate a constant-buffer binding for `U` whether
1935+
// or not it turns out that the concrete type plugged in for
1936+
// `IThing gThing` has any ordinary/uniform data at all (that is,
1937+
// if the user plugs in a type that only holds a `Texture2D`,
1938+
// we will still have allocated the constant buffer binding/register,
1939+
// and waste it on an empty buffer).
1940+
//
1941+
// The reason for this choice is that it greatly simplifies
1942+
// logic for clients of Slang: a given `ConstantBuffer<>` or
1943+
// `cbuffer` variable can be statically determined to either
1944+
// need a constant buffer binding or not, based on its declared
1945+
// element type, and *nothing* that happens later can change
1946+
// that (e.g., plugging in a new value/object for `gThing`
1947+
// can't retroactively change whether or not `U` needed
1948+
// a constant buffer).
1949+
//
1950+
bool wantConstantBuffer = _usesOrdinaryData(rawElementTypeLayout)
1951+
|| _usesExistentialData(rawElementTypeLayout);
19241952
if( wantConstantBuffer )
19251953
{
19261954
// If there is any ordinary data, then we'll need to
@@ -1932,10 +1960,8 @@ static RefPtr<TypeLayout> _createParameterGroupTypeLayout(
19321960
}
19331961

19341962
// Similarly to how we only need a constant buffer to be allocated
1935-
// if the contents of the group actually had ordinary/uniform data,
1936-
// we also only want to allocate a `space` or `set` if that is really
1937-
// required.
1938-
//
1963+
// if the contents of the group actually call for it, we also only
1964+
// want to allocate a `space` or `set` if that is really required.
19391965
//
19401966
bool canUseSpaceOrSet = false;
19411967
//
@@ -1970,9 +1996,11 @@ static RefPtr<TypeLayout> _createParameterGroupTypeLayout(
19701996
if( canUseSpaceOrSet )
19711997
{
19721998
// Note that if we are allocating a constant buffer to hold
1973-
// some ordinary/uniform data then we definitely want a space/set,
1974-
// but we don't need to special-case that because the loop
1975-
// here will also detect the `LayoutResourceKind::Uniform` usage.
1999+
// some ordinary/uniform (or existential) data then we
2000+
// definitely want a space/set (because we will need it for
2001+
// the constant buffer we allocated above) but we don't need
2002+
// to special-case that because the loop here will also detect
2003+
// the `LayoutResourceKind::Uniform` usage.
19762004

19772005
for( auto elementResourceInfo : rawElementTypeLayout->resourceInfos )
19782006
{
@@ -2080,89 +2108,27 @@ static RefPtr<TypeLayout> _createParameterGroupTypeLayout(
20802108
{
20812109
auto rules = rawElementTypeLayout->rules;
20822110

2083-
// One really annoying complication we need to deal with here
2084-
// its that it is possible that the original parameter group
2085-
// declaration didn't need a constant buffer or `space`/`set`
2086-
// to be allocated, but once we consider the "pending" data
2087-
// we need to have a constant buffer and/or space.
2111+
// Note that because we conservatively allocated both
2112+
// a constant buffer `register`/`binding` and a `space`/`set`
2113+
// for the container in cases where the element type
2114+
// might need it (which included interface/existential types),
2115+
// there is no need to worry about a case where `pendingElementType`
2116+
// could require a constant buffer `register`/`binding` or
2117+
// as `space`/`set` to be allocated but we didn't already
2118+
// allocate one in the non-pending layout.
2119+
//
2120+
// Out focus here is then on setting up the representation
2121+
// of the "pending" data for the element type, and in
2122+
// particular on dealing with any data that needs to
2123+
// "bleed through" to the resource usage of the overall
2124+
// parameter group.
20882125
//
2089-
// We will compute whether the pending data create a demand
2090-
// for a constant buffer and/or a space/set, so that we know
2091-
// if we are in the tricky case.
2092-
//
2093-
bool pendingDataWantsConstantBuffer = _usesOrdinaryData(pendingElementTypeLayout);
2094-
bool pendingDataWantsSpaceOrSet = false;
2095-
if( canUseSpaceOrSet )
2096-
{
2097-
for( auto resInfo : pendingElementTypeLayout->resourceInfos )
2098-
{
2099-
if( resInfo.kind != LayoutResourceKind::RegisterSpace )
2100-
{
2101-
pendingDataWantsSpaceOrSet = true;
2102-
break;
2103-
}
2104-
}
2105-
}
2106-
2107-
// We will use a few different variables to track resource
2108-
// usage for the pending data, with roles similar to the
2109-
// umbrella type layout, container layout, and element layout
2110-
// that already came up for the main part of the parameter group type.
2111-
2112-
2113-
RefPtr<TypeLayout> pendingContainerTypeLayout = new TypeLayout();
2114-
pendingContainerTypeLayout->type = parameterGroupType;
2115-
pendingContainerTypeLayout->rules = parameterGroupRules;
2116-
2117-
containerTypeLayout->pendingDataTypeLayout = pendingContainerTypeLayout;
2118-
2119-
RefPtr<VarLayout> pendingContainerVarLayout = new VarLayout();
2120-
pendingContainerVarLayout->typeLayout = pendingContainerTypeLayout;
2121-
2122-
containerVarLayout->pendingVarLayout = pendingContainerVarLayout;
2123-
2124-
21252126
RefPtr<VarLayout> pendingElementVarLayout = new VarLayout();
21262127
pendingElementVarLayout->typeLayout = pendingElementTypeLayout;
21272128

21282129
elementVarLayout->pendingVarLayout = pendingElementVarLayout;
21292130

2130-
// If we need a space/set for the pending data, and don't already
2131-
// have one, then we will allocate it now, as part of the
2132-
// "full" data type.
2133-
//
2134-
if( pendingDataWantsSpaceOrSet && !wantSpaceOrSet )
2135-
{
2136-
pendingContainerTypeLayout->addResourceUsage(LayoutResourceKind::RegisterSpace, 1);
2137-
2138-
// From here on, we know we have access to a register space,
2139-
// and we can mask any registers/bindings appropriately.
2140-
//
2141-
wantSpaceOrSet = true;
2142-
}
2143-
2144-
// If we need a constant buffer for laying out ordinary
2145-
// data, and didn't have one allocated before, we will create
2146-
// one.
2147-
//
2148-
if( pendingDataWantsConstantBuffer && !wantConstantBuffer )
2149-
{
2150-
auto cbUsage = rules->GetObjectLayout(ShaderParameterKind::ConstantBuffer);
2151-
pendingContainerTypeLayout->addResourceUsage(cbUsage.kind, cbUsage.size);
2152-
2153-
wantConstantBuffer = true;
2154-
}
2155-
2156-
for( auto resInfo : pendingContainerTypeLayout->resourceInfos )
2157-
{
2158-
pendingContainerVarLayout->findOrAddResourceInfo(resInfo.kind);
2159-
}
2160-
2161-
// Now that we've added in the resource usage for any CB or set/space
2162-
// we needed to allocate just for the pending data, we can safely
2163-
// lay out the pending data itself.
2164-
//
2165-
// The ordinary/uniform part of things wil always be "masked" and
2131+
// Any ordinary/uniform part of the pending data wil always be "masked" and
21662132
// needs to come after any uniform data from the original element type.
21672133
//
21682134
// To kick things off we will initialize state for `struct` type layout,
@@ -2183,8 +2149,8 @@ static RefPtr<TypeLayout> _createParameterGroupTypeLayout(
21832149
if( resInfo.kind == LayoutResourceKind::Uniform )
21842150
{
21852151
// For the ordinary/uniform resource kind, we will add the resource
2186-
// usage as a structure field, and then write the resulting offset
2187-
// into the variable layout for the pending data.
2152+
// usage as if it was a structure field, and then write the resulting
2153+
// offset into the variable layout for the pending data.
21882154
//
21892155
auto offset = rules->AddStructField(
21902156
&uniformLayout,
@@ -2195,16 +2161,11 @@ static RefPtr<TypeLayout> _createParameterGroupTypeLayout(
21952161
}
21962162
else
21972163
{
2198-
// For all other resource kinds, we will set the offset in
2199-
// the variable layout based on the total resources of that
2200-
// kind seen so far (including the "container" if any),
2201-
// and then bump the count for total resource usage.
2164+
// For all other resource kinds, we simply need to add an
2165+
// entry to the pending layout to represent the resource
2166+
// usage of the pending data.
22022167
//
2203-
auto elementVarResInfo = pendingElementVarLayout->findOrAddResourceInfo(resInfo.kind);
2204-
if( auto containerTypeInfo = pendingContainerTypeLayout->FindResourceInfo(resInfo.kind) )
2205-
{
2206-
elementVarResInfo->index = containerTypeInfo->count.getFiniteValue();
2207-
}
2168+
pendingElementVarLayout->findOrAddResourceInfo(resInfo.kind);
22082169
}
22092170
}
22102171
rules->EndStructLayout(&uniformLayout);
@@ -2218,7 +2179,6 @@ static RefPtr<TypeLayout> _createParameterGroupTypeLayout(
22182179
// up the hierarchy.
22192180
//
22202181
RefPtr<TypeLayout> unmaskedPendingDataTypeLayout = new TypeLayout();
2221-
_addUnmaskedResourceUsage(true, unmaskedPendingDataTypeLayout, pendingContainerTypeLayout, wantSpaceOrSet);
22222182
_addUnmaskedResourceUsage(false, unmaskedPendingDataTypeLayout, pendingElementTypeLayout, wantSpaceOrSet);
22232183

22242184
// TODO: we should probably optimize for the case where there is no unmasked
@@ -2228,6 +2188,58 @@ static RefPtr<TypeLayout> _createParameterGroupTypeLayout(
22282188
//
22292189
typeLayout->pendingDataTypeLayout = unmaskedPendingDataTypeLayout;
22302190

2191+
// We will now attempt to compute reasonable offset information for
2192+
// (non-uniform) pending data in the element type. There are basically
2193+
// two cases here:
2194+
//
2195+
// 1. If the resource kind is one that is "masked" by the container,
2196+
// then the pending data can be statically placed at an offset fater
2197+
// the diret (non-pending) element data.
2198+
//
2199+
// 2. If the resource kind is one that "bleeds through" to the container,
2200+
// then its offset will always be relative to the location that
2201+
// gets allocated for pending data in the container, which means it
2202+
// is always zero.
2203+
//
2204+
// Because the offsets are currently all set to zero, we only
2205+
// need to check for case (1).
2206+
//
2207+
for( auto pendingVarResInfo : pendingElementVarLayout->resourceInfos )
2208+
{
2209+
auto kind = pendingVarResInfo.kind;
2210+
2211+
// If we are looking at uniform resource usage, we already
2212+
// handled it easlier.
2213+
//
2214+
if(kind == LayoutResourceKind::Uniform)
2215+
continue;
2216+
2217+
// If the usage is unmasked, the nwe are in case (2) and should
2218+
// skip out.
2219+
//
2220+
if(unmaskedPendingDataTypeLayout->FindResourceInfo(kind))
2221+
continue;
2222+
2223+
// Okay, we have resource info for somethign that is going
2224+
// to be "masked" by the container, in which case we
2225+
// can compute a fixed offset, after any existing data
2226+
// of the same kind.
2227+
//
2228+
auto existingVarResInfo = elementVarLayout->FindResourceInfo(kind);
2229+
if(!existingVarResInfo)
2230+
continue;
2231+
2232+
auto existingTypeResInfo = elementVarLayout->typeLayout->FindResourceInfo(kind);
2233+
if(!existingTypeResInfo)
2234+
continue;
2235+
2236+
// TODO: We need a more robust solution than just calling
2237+
// `getFiniteValue` here.
2238+
//
2239+
pendingVarResInfo.index = existingVarResInfo->index
2240+
+ existingTypeResInfo->count.getFiniteValue();
2241+
}
2242+
22312243
// TODO: we should probably adjust the size reported by the element type
22322244
// to include any "pending" data that was allocated into the group, so
22332245
// that it can be easier for client code to allocate their instances.
@@ -3025,7 +3037,7 @@ static TypeLayoutResult _createTypeLayout(
30253037
// buffer, including offsets, etc.
30263038
//
30273039
// 2. Compute information about any object types inside
3028-
// the constant buffer, which need to be surfaces out
3040+
// the constant buffer, which need to be surfaced out
30293041
// to the top level.
30303042
//
30313043
auto typeLayout = createParameterGroupTypeLayout(

tests/compute/interface-shader-param-in-struct.slang

+22-3
Original file line numberDiff line numberDiff line change
@@ -44,20 +44,39 @@ int test(
4444
//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out
4545
RWStructuredBuffer<int> gOutputBuffer;
4646

47+
// Note: even though `C` doesn't include any
48+
// uniform/ordinary dat as declared, it gets a
49+
// constant buffer allocated for it because
50+
// there is no way to rule out the possibility
51+
// that it *will* contain uniform/ordinary data
52+
// after specialization.
53+
//
54+
//TEST_INPUT:cbuffer(data=[0]):
4755
cbuffer C
4856
{
4957
IRandomNumberGenerationStrategy gStrategy;
5058
}
5159

52-
//TEST_INPUT: globalExistentialType MyStrategy
53-
//TEST_INPUT:ubuffer(data=[1 2 4 8], stride=4):
54-
5560
struct Stuff
5661
{
5762
IModifier modifier;
5863
int extra;
5964
}
6065

66+
// Note: the data for global-scope existential parameters
67+
// is being introduced *before* the entry point declaration,
68+
// because the default policy used by `slangc` (which the
69+
// render test also uses) is to specialize the parameters at
70+
// the global scope (producing a new layout) and then compose
71+
// that specialized global scope with the entry point.
72+
//
73+
// (The net result is that data related to global-scope
74+
// specialization always precedes the data for entry point
75+
// parameters in these tests today)
76+
//
77+
//TEST_INPUT: globalExistentialType MyStrategy
78+
//TEST_INPUT:ubuffer(data=[1 2 4 8], stride=4):
79+
6180
[numthreads(4, 1, 1)]
6281
void computeMain(
6382
//TEST_INPUT:cbuffer(data=[256]):

tests/compute/interface-shader-param4.slang

+8
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,14 @@ int test(
4646
//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out
4747
RWStructuredBuffer<int> gOutputBuffer;
4848

49+
// Note: a constant buffer register/binding is always claimed
50+
// for `gStrategy` during initial compilation (before specialization)
51+
// because the layout logic has no way of knowing if the type
52+
// that gets plugged in will involve uniform/ordinary data
53+
// or not.
54+
//
55+
//TEST_INPUT:cbuffer(data=[0]):
56+
//
4957
ConstantBuffer<IRandomNumberGenerationStrategy> gStrategy;
5058

5159
// The concrete types we plug in for `gStrategy` and `modifier`

0 commit comments

Comments
 (0)