Skip to content

Commit 39975b2

Browse files
author
Tim Foley
authored
Fixes to get shader-object example working on CUDA (shader-slang#1708)
The purpose of these changes is to make the `shader-object` example work correctly on CUDA. Originally I had tried to add changes to the "flat" reflection information so that it introduced descriptor ranges to match the binding ranges it added for interface/existential-type fields. This approach helped the CUDA code that was using that information to try and compute uniform offsets for those fields, but it broke most of the other renderer back-ends. Instead, I removed the relevant asserts from `CUDAShaderObject::setObject()`. Note taht there are leftover changes from my edits to the flat reflection information, around how it handles "leaf" fields that consume multiple resource kinds. I believe that those changes are, on balance, "more correct" now than they were before, so I decided to leave them in. The other major fix here is to specialize the `CUDAShaderObject::setObject()` logic to handle the case of setting a shader object for a parameter that has interface type instead of a constant-buffer or parameter block. Mostly I just copy bytes from the child object into the parent object. There are a few caveats, though: * I am not writing the RTTI or witness-table information, so dynamic dispatch won't work. * I am assuming a hard-coded offset of 16 bytes for the any-value, which will work for now but is a bit too "magical" and might also break once we support conjunctions of interfaces with dynamic dispatch * I am assuming that the child value to be writen into the field will "fit" into the any-value area. We need some way to determine whether or not things fit dynamically (ideally using the reflection data), and adapt accordingly. * I had to add another method on the base CUDA shader object type to handle setting data using a device-memory pointr instead of a host-memory pointer * There's not a lot we can do about it, but in the case of assigning an ordinary `CUDAShaderObject` into an interface-type field of a `CUDAEntryPointShaderObject` we end up needing to perform a device->host memory copy, because the bytes of the value will have already been written to GPU memory, but need to be in GPU memory for the dispatch call. * The implementation I'm using here basically assumes that the child shader object must have been finalized before it gets plugged into the parent shader object. We haven't yet made a policy decision about that bit.
1 parent e474c4e commit 39975b2

File tree

3 files changed

+246
-137
lines changed

3 files changed

+246
-137
lines changed

‎source/slang/slang-reflection-api.cpp

+175-129
Original file line numberDiff line numberDiff line change
@@ -1172,6 +1172,35 @@ namespace Slang
11721172
}
11731173
}
11741174

1175+
SlangBindingType _calcBindingType(
1176+
LayoutResourceKind kind)
1177+
{
1178+
switch( kind )
1179+
{
1180+
default:
1181+
return SLANG_BINDING_TYPE_UNKNOWN;
1182+
1183+
// Some cases of `LayoutResourceKind` can be mapped
1184+
// directly to a `BindingType` because there is only
1185+
// one case of types that have that resource kind.
1186+
1187+
#define CASE(FROM, TO) \
1188+
case LayoutResourceKind::FROM: return SLANG_BINDING_TYPE_##TO
1189+
1190+
CASE(ConstantBuffer, CONSTANT_BUFFER);
1191+
CASE(SamplerState, SAMPLER);
1192+
CASE(VaryingInput, VARYING_INPUT);
1193+
CASE(VaryingOutput, VARYING_OUTPUT);
1194+
CASE(ExistentialObjectParam, EXISTENTIAL_VALUE);
1195+
CASE(PushConstantBuffer, PUSH_CONSTANT);
1196+
CASE(Uniform, INLINE_UNIFORM_DATA);
1197+
// TODO: register space
1198+
1199+
#undef CASE
1200+
}
1201+
}
1202+
1203+
11751204

11761205
SlangBindingType _calcBindingType(
11771206
Slang::TypeLayout* typeLayout,
@@ -1193,29 +1222,7 @@ namespace Slang
11931222
// multiple different kinds of binding, depending on where/how
11941223
// it is used (e.g., as a varying parameter, a root constant, etc.).
11951224
//
1196-
switch( kind )
1197-
{
1198-
default:
1199-
return SLANG_BINDING_TYPE_UNKNOWN;
1200-
1201-
// Some cases of `LayoutResourceKind` can be mapped
1202-
// directly to a `BindingType` because there is only
1203-
// one case of types that have that resource kind.
1204-
1205-
#define CASE(FROM, TO) \
1206-
case LayoutResourceKind::FROM: return SLANG_BINDING_TYPE_##TO
1207-
1208-
CASE(ConstantBuffer, CONSTANT_BUFFER);
1209-
CASE(SamplerState, SAMPLER);
1210-
CASE(VaryingInput, VARYING_INPUT);
1211-
CASE(VaryingOutput, VARYING_OUTPUT);
1212-
CASE(ExistentialObjectParam, EXISTENTIAL_VALUE);
1213-
CASE(PushConstantBuffer, PUSH_CONSTANT);
1214-
CASE(Uniform, INLINE_UNIFORM_DATA);
1215-
// TODO: register space
1216-
1217-
#undef CASE
1218-
}
1225+
return _calcBindingType(kind);
12191226
}
12201227

12211228
static DeclRefType* asInterfaceType(Type* type)
@@ -1538,147 +1545,186 @@ namespace Slang
15381545
}
15391546
else if(asInterfaceType(typeLayout->type))
15401547
{
1541-
// An `interface` type should introduce a sub-object range,
1542-
// with no concrete descriptor ranges to store its value
1543-
// (since we don't know until runtime what type of
1544-
// value will be plugged in).
1548+
// An `interface` type should introduce a binding range and a matching
1549+
// sub-object range.
1550+
//
1551+
// We currently do *not* allocate any descriptor ranges to represent
1552+
// an interface-type field, since the only direct storage required
1553+
// is all uniform/ordinary data.
15451554
//
1546-
1547-
LayoutResourceKind kind = LayoutResourceKind::ExistentialObjectParam;
1548-
auto count = multiplier;
1549-
auto spaceOffset = _calcSpaceOffset(path, kind);
1550-
1551-
Int descriptorSetIndex = _findOrAddDescriptorSet(spaceOffset);
1552-
auto descriptorSet = m_extendedInfo->m_descriptorSets[descriptorSetIndex];
1553-
15541555
TypeLayout::ExtendedInfo::BindingRangeInfo bindingRange;
15551556
bindingRange.leafTypeLayout = typeLayout;
15561557
bindingRange.bindingType = SLANG_BINDING_TYPE_EXISTENTIAL_VALUE;
15571558
bindingRange.count = multiplier;
1558-
bindingRange.descriptorSetIndex = descriptorSetIndex;
1559-
bindingRange.firstDescriptorRangeIndex = descriptorSet->descriptorRanges.getCount();
1560-
bindingRange.descriptorRangeCount = 1;
1559+
bindingRange.descriptorSetIndex = 0;
1560+
bindingRange.descriptorRangeCount = 0;
1561+
bindingRange.firstDescriptorRangeIndex = 0;
15611562

15621563
TypeLayout::ExtendedInfo::SubObjectRangeInfo subObjectRange;
15631564
subObjectRange.bindingRangeIndex = m_extendedInfo->m_bindingRanges.getCount();
15641565

1566+
// TODO: if we have "pending" layout information that tells us where
1567+
// data for the sub-object range has been allocated, then we need
1568+
// a way to reference that data here.
1569+
15651570
m_extendedInfo->m_bindingRanges.add(bindingRange);
15661571
m_extendedInfo->m_subObjectRanges.add(subObjectRange);
15671572
}
1568-
// TODO: We need to handle `interface` types here, because they are
1569-
// another case that introduces a "sub-object" for the purposes of
1570-
// application-side allocation.
1571-
//
1572-
// TODO: There are a few cases of "leaf" fields that might
1573-
// still result in multiple descriptors (or at least multiple
1574-
// `LayoutResourceKind`s) depending on the target.
1575-
//
1576-
// For eample, combined texture-sampler types should be treated
1577-
// as "leaf" fields for this code (since a portable engine would
1578-
// need to abstract over them), but would map to two descriptors
1579-
// on targets that don't actually support combining them.
15801573
else
15811574
{
1582-
Int resourceKindCount = typeLayout->resourceInfos.getCount();
1583-
if(resourceKindCount == 0)
1575+
// Here we have the catch-all case that handles "leaf" fields
1576+
// that should never introduce a sub-object range, but might
1577+
// need to introduce a binding range and descriptor ranges.
1578+
//
1579+
// First, we want to determine what type of binding this
1580+
// leaf field should map to, if any. We being by querying
1581+
// the type itself, since there are many distinct descriptor
1582+
// types for textures/buffers that can only be determined
1583+
// by type, rather than by a `LayoutResourceKind`.
1584+
//
1585+
auto bindingType = _calcResourceBindingType(typeLayout);
1586+
1587+
// It is possible that the type alone isn't enough to tell
1588+
// us a specific binding type, at which point we need to
1589+
// start looking at the actual resources the type layout
1590+
// consumes.
1591+
//
1592+
if(bindingType == SLANG_BINDING_TYPE_UNKNOWN)
15841593
{
1585-
// This is a field that consumes no resources, and as
1586-
// such does not need a binding or descriptor ranges
1587-
// allocated for it.
1594+
// We will search through all the resource kinds that
1595+
// the type layout consumes, to see if we can find
1596+
// one that indicates a binding type we actually
1597+
// want to reflect.
15881598
//
1589-
return;
1590-
}
1591-
else
1592-
{
1593-
// `resourceKindCount` should be 1 in most cases.
1594-
// However if this field is a buffer of existential type,
1595-
// the resourceInfo array will contain additional ExistentialTypeParam
1596-
// and ExistentialObjectParam entries. These entries doesn't affect the
1597-
// logic here, so we only need to care about the first entry.
1598-
auto& resInfo = typeLayout->resourceInfos[0];
1599-
LayoutResourceKind kind = resInfo.kind;
1600-
1601-
auto bindingType = _calcBindingType(typeLayout, kind);
1602-
if(bindingType == SLANG_BINDING_TYPE_INLINE_UNIFORM_DATA)
1599+
for( auto resInfo : typeLayout->resourceInfos )
16031600
{
1604-
// We do not consider uniform resource usage
1605-
// in the ranges we compute.
1601+
auto kind = resInfo.kind;
1602+
if(kind == LayoutResourceKind::Uniform)
1603+
continue;
1604+
1605+
auto kindBindingType = _calcBindingType(kind);
1606+
if(kindBindingType == SLANG_BINDING_TYPE_UNKNOWN)
1607+
continue;
1608+
1609+
// If we find a relevant binding type based on
1610+
// one of the resource kinds that are consumed,
1611+
// then we immediately stop the search and use
1612+
// the first one found (whether or not later
1613+
// entries might also provide something relevant).
16061614
//
1607-
// TODO: We may need to revise that rule for types that
1608-
// represent resources, even when one or more targets
1609-
// map those resource types to ordinary/uniform data.
1610-
//
1611-
return;
1615+
bindingType = kindBindingType;
1616+
break;
16121617
}
1618+
}
16131619

1614-
// This leaf field will map to a single binding range and,
1615-
// if it is appropriate, a single descriptor range.
1616-
//
1617-
auto count = resInfo.count * multiplier;
1618-
auto indexOffset = _calcIndexOffset(path, kind);
1619-
auto spaceOffset = _calcSpaceOffset(path, kind);
1620+
// After we've tried to determine a binding type, if
1621+
// we have nothing to go on then we don't want to add
1622+
// a binding range.
1623+
//
1624+
if(bindingType == SLANG_BINDING_TYPE_UNKNOWN)
1625+
return;
1626+
1627+
// We now know that the leaf field will map to a single binding range,
1628+
// and zero or more descriptor ranges.
1629+
//
1630+
TypeLayout::ExtendedInfo::BindingRangeInfo bindingRange;
1631+
bindingRange.leafTypeLayout = typeLayout;
1632+
bindingRange.bindingType = bindingType;
1633+
bindingRange.count = multiplier;
1634+
bindingRange.descriptorSetIndex = 0;
1635+
bindingRange.firstDescriptorRangeIndex = 0;
1636+
bindingRange.descriptorRangeCount = 0;
16201637

1621-
Int descriptorSetIndex = -1;
1622-
Int firstDescriptorIndex = 0;
1623-
RefPtr<TypeLayout::ExtendedInfo::DescriptorSetInfo> descriptorSet;
1638+
// We will associate the binding range with a specific descriptor
1639+
// set on demand *if* we discover that it shold contain any
1640+
// descriptor ranges.
1641+
//
1642+
RefPtr<TypeLayout::ExtendedInfo::DescriptorSetInfo> descriptorSet;
1643+
1644+
1645+
// We will add a descriptor range for each relevant resource kind
1646+
// that the type layout consumes.
1647+
//
1648+
for(auto resInfo : typeLayout->resourceInfos)
1649+
{
1650+
auto kind = resInfo.kind;
16241651
switch( kind )
16251652
{
1653+
default:
1654+
break;
1655+
1656+
1657+
// There are many resource kinds that we do not want
1658+
// to expose as descriptor ranges simply because they
1659+
// do not actually allocate descriptors on our target
1660+
// APIs.
1661+
//
1662+
// Notably included here are uniform/ordinary data and
1663+
// varying input/output (including the ray-tracing cases).
1664+
//
1665+
// It is worth noting that we *do* allow root/push-constant
1666+
// ranges to be reflected as "descriptor" ranges here,
1667+
// despite the fact that they are not descriptor-bound
1668+
// under D3D12/Vulkan.
1669+
//
1670+
// In practice, even with us filtering out some cases here,
1671+
// an application/renderer layer will need to filter/translate
1672+
// or descriptor ranges into API-specific ones, and a one-to-one
1673+
// mapping should not be assumed.
1674+
//
1675+
// TODO: Make some clear decisions about what should and should
1676+
// not appear here.
1677+
//
1678+
case LayoutResourceKind::Uniform:
16261679
case LayoutResourceKind::RegisterSpace:
16271680
case LayoutResourceKind::VaryingInput:
16281681
case LayoutResourceKind::VaryingOutput:
16291682
case LayoutResourceKind::HitAttributes:
16301683
case LayoutResourceKind::RayPayload:
1631-
// Resource kinds that represent "varying" input/output
1632-
// do not manifest as entries in API descriptor tables.
1633-
//
1634-
// TODO: Neither do root constants, if we are being
1635-
// precise. This API really needs to carefully match
1636-
// the semantics of the target platform/API in terms
1637-
// of what things are descriptor-bound and which are
1638-
// not, so that a user can easily allocate the platform-specific
1639-
// descriptor sets using this info.
1640-
//
1641-
// (That said, we are purposefully *not* breaking apart
1642-
// samplers and SRV/UAV/CBV stuff for our D3D reflection
1643-
// of descriptor sets. It seems like the policy here
1644-
// really requires careful thought)
1645-
//
1646-
// TODO: Maybe the best answer is to leave decomposition
1647-
// of stuff into descriptor sets up to the application
1648-
// layer? This is especially true if a common case would
1649-
// be an application that doesn't support arbitrary manual
1650-
// binding of parameters to register/spaces.
1651-
//
1652-
break;
1684+
continue;
1685+
}
16531686

1654-
default:
1655-
{
1656-
TypeLayout::ExtendedInfo::DescriptorRangeInfo descriptorRange;
1657-
descriptorRange.kind = kind;
1658-
descriptorRange.bindingType = bindingType;
1659-
descriptorRange.count = count;
1660-
descriptorRange.indexOffset = indexOffset;
1687+
// We will prefer to use a binding type derived from the specific
1688+
// resource kind, but will fall back to information from the
1689+
// type layout when that is not available.
1690+
//
1691+
// TODO: This logic probably needs a bit more work to handle
1692+
// the case of a combined texture-sampler field that is being
1693+
// compiled for an API with separate textures and samplers.
1694+
//
1695+
auto kindBindingType = _calcBindingType(kind);
1696+
if( kindBindingType == SLANG_BINDING_TYPE_UNKNOWN )
1697+
{
1698+
kindBindingType = bindingType;
1699+
}
16611700

1662-
descriptorSetIndex = _findOrAddDescriptorSet(spaceOffset);
1663-
descriptorSet = m_extendedInfo->m_descriptorSets[descriptorSetIndex];
1701+
// We now expect to allocate a descriptor range for this
1702+
// `resInfo` representing resouce usage.
1703+
//
1704+
auto count = resInfo.count * multiplier;
1705+
auto indexOffset = _calcIndexOffset(path, kind);
1706+
auto spaceOffset = _calcSpaceOffset(path, kind);
16641707

1665-
firstDescriptorIndex = descriptorSet->descriptorRanges.getCount();
1666-
descriptorSet->descriptorRanges.add(descriptorRange);
1667-
}
1668-
break;
1669-
}
1708+
TypeLayout::ExtendedInfo::DescriptorRangeInfo descriptorRange;
1709+
descriptorRange.kind = kind;
1710+
descriptorRange.bindingType = kindBindingType;
1711+
descriptorRange.count = count;
1712+
descriptorRange.indexOffset = indexOffset;
16701713

1714+
if(!descriptorSet)
1715+
{
1716+
Int descriptorSetIndex = _findOrAddDescriptorSet(spaceOffset);
1717+
descriptorSet = m_extendedInfo->m_descriptorSets[descriptorSetIndex];
16711718

1672-
TypeLayout::ExtendedInfo::BindingRangeInfo bindingRange;
1673-
bindingRange.leafTypeLayout = typeLayout;
1674-
bindingRange.bindingType = _calcBindingType(typeLayout, kind);
1675-
bindingRange.count = count;
1676-
bindingRange.descriptorSetIndex = descriptorSetIndex;
1677-
bindingRange.firstDescriptorRangeIndex = firstDescriptorIndex;
1678-
bindingRange.descriptorRangeCount = 1;
1719+
bindingRange.descriptorSetIndex = descriptorSetIndex;
1720+
bindingRange.firstDescriptorRangeIndex = descriptorSet->descriptorRanges.getCount();
1721+
}
16791722

1680-
m_extendedInfo->m_bindingRanges.add(bindingRange);
1723+
descriptorSet->descriptorRanges.add(descriptorRange);
1724+
bindingRange.descriptorRangeCount++;
16811725
}
1726+
1727+
m_extendedInfo->m_bindingRanges.add(bindingRange);
16821728
}
16831729
}
16841730
};

0 commit comments

Comments
 (0)