Skip to content

Commit ee7fd60

Browse files
ramang-unityramang-unity
committed
PR Review:
- Split the uniform-usage pass's parent-binding space into two fields: parentSpace (RegisterSpace offset, what the Uniform location query matches) and parentBindingSpace (includes the descriptor set, what the reflection emitter matches via getBindingSpace). - Fix usedByteRanges silently dropping off a constant buffer bound to a non-zero Vulkan descriptor set; isParameterLocationUsed and loose-global used reporting are unchanged. - Add used-uniforms-vulkan-cb-nonzero-set reflection test to pin the non-zero-set case.
1 parent b4ff7f3 commit ee7fd60

7 files changed

Lines changed: 227 additions & 20 deletions

‎source/compiler-core/slang-artifact-associated.h‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,14 +126,26 @@ struct ShaderBindingRange;
126126

127127
struct UniformParamUsage
128128
{
129-
// Parent CB or parameter block binding identity. Together
130-
// (parentSpace, parentBindingIndex) uniquely identifies which
131-
// uniform bearing parameter the byte ranges below belong to, so
132-
// multiple CBs in the same register space stay disambiguated.
129+
// Parent CB or parameter block binding identity. Two spaces are
130+
// recorded because the byte query and the reflection emitter key off
131+
// different ones, and they diverge for a CB bound to a non zero
132+
// descriptor set (see ByteGranularityParameterUsageInfo for the full
133+
// rationale):
134+
// parentSpace - the space the Uniform category reports
135+
// (RegisterSpace offset only); what
136+
// isParameterLocationUsed(Uniform, space) is
137+
// queried with. Each usedRanges entry carries
138+
// this as its spaceIndex.
139+
// parentBindingSpace - the parent binding's own space (includes the
140+
// descriptor set); what the emitter matches via
141+
// getBindingSpace. Together with
142+
// parentBindingIndex it disambiguates multiple
143+
// CBs in a shared register space.
133144
// Each entry in usedRanges has category=Uniform,
134145
// spaceIndex=parentSpace, registerIndex=byte offset within the
135146
// parent, and registerCount=byte size.
136147
UInt parentSpace;
148+
UInt parentBindingSpace;
137149
UInt parentBindingIndex;
138150
List<ShaderBindingRange> usedRanges;
139151
// True when the IR pass deliberately did not analyze this param

‎source/slang/slang-ir-byte-granularity-usage.cpp‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -364,32 +364,41 @@ collectApproximateByteGranularityUsageInformationForParameterGroups(const IRModu
364364
info.parentSpace = 0;
365365
if (auto spaceAttr = varLayout->findOffsetAttr(LayoutResourceKind::RegisterSpace))
366366
info.parentSpace = spaceAttr->getOffset();
367-
// Capture the parent CB or parameter block binding index. For most
367+
// Capture the parent CB or parameter block binding. For most
368368
// targets a parameter group has a ConstantBuffer offset (D3D b
369369
// register) or a DescriptorTableSlot offset (Vulkan/SPIR-V
370370
// descriptor binding). Some target configurations (cooperative
371371
// type metadata shaders, certain Metal/WGSL paths) yield a group
372372
// with neither attr. In that case we have no parent identity to
373-
// scope per byte ranges against, so emit an untracked entry in
374-
// the param's space. Queries in that space then return
375-
// SLANG_E_NOT_AVAILABLE, which is safe over reporting rather
376-
// than a silent collision with binding zero.
373+
// scope per byte ranges against, so emit an untracked entry.
374+
// Queries then return SLANG_E_NOT_AVAILABLE, which is safe over
375+
// reporting rather than a silent collision with binding zero.
377376
auto cbAttr = varLayout->findOffsetAttr(LayoutResourceKind::ConstantBuffer);
378377
auto dtAttr = varLayout->findOffsetAttr(LayoutResourceKind::DescriptorTableSlot);
378+
// parentBindingSpace must match what the reflection emitter sees
379+
// via getBindingSpace: the category attr's own space plus the
380+
// RegisterSpace offset. A descriptor set can live in either source
381+
// depending on binding style (vk::binding(N, set) puts it in the
382+
// descriptor attr's space; -fvk-bind-globals puts it in
383+
// RegisterSpace), so both are summed. parentSpace stays the
384+
// RegisterSpace offset alone, matching the space a Uniform query
385+
// carries.
379386
UniformParentBinding parent = selectUniformParentBinding(
380387
cbAttr != nullptr,
381-
info.parentSpace,
388+
info.parentSpace + (cbAttr ? cbAttr->getSpace() : 0),
382389
cbAttr ? cbAttr->getOffset() : 0,
383390
dtAttr != nullptr,
384-
info.parentSpace,
391+
info.parentSpace + (dtAttr ? dtAttr->getSpace() : 0),
385392
dtAttr ? dtAttr->getOffset() : 0);
386393
if (!parent.found)
387394
{
395+
info.parentBindingSpace = info.parentSpace;
388396
info.parentBindingIndex = 0;
389397
info.isUntracked = true;
390398
result.add(_Move(info));
391399
continue;
392400
}
401+
info.parentBindingSpace = parent.space;
393402
info.parentBindingIndex = parent.bindingIndex;
394403
info.isUntracked = false;
395404

‎source/slang/slang-ir-byte-granularity-usage.h‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,37 @@ struct ByteGranularityUsageRange
1616
};
1717

1818
// Per parameter usage record. param is the global shader parameter.
19-
// (parentSpace, parentBindingIndex) identify the param's primary CB
20-
// or parameter block binding so multiple CBs in a shared register
21-
// space stay disambiguated downstream. ranges holds whatever byte
22-
// intervals reachable code touched. isUntracked flags params we
23-
// declined to analyze (e.g. unbounded uniform element type), in which
24-
// case ranges is empty and the consumer should record an untracked
25-
// marker so per byte queries return SLANG_E_NOT_AVAILABLE instead of
26-
// a misleading false answer.
19+
// ranges holds whatever byte intervals reachable code touched.
20+
// isUntracked flags params we declined to analyze (e.g. unbounded
21+
// uniform element type), in which case ranges is empty and the consumer
22+
// should record an untracked marker so per byte queries return
23+
// SLANG_E_NOT_AVAILABLE instead of a misleading false answer.
24+
//
25+
// Two spaces, because two downstream consumers key off different ones
26+
// and they diverge for a constant buffer bound to a non zero descriptor
27+
// set:
28+
//
29+
// parentSpace - the space the Uniform parameter category
30+
// reports (RegisterSpace offset only). This is
31+
// what spIsParameterLocationUsed(Uniform, space)
32+
// is queried with, since a loose uniform leaf has
33+
// no descriptor set of its own and reports space
34+
// zero. The recorded ranges carry this space too.
35+
// parentBindingSpace - the space of the parent CB or parameter block
36+
// binding itself (RegisterSpace offset plus the
37+
// ConstantBuffer/DescriptorTableSlot category's
38+
// own space). This is what the reflection emitter
39+
// matches against via getBindingSpace, so it must
40+
// include the descriptor set.
41+
//
42+
// Together with parentBindingIndex, parentBindingSpace identifies the
43+
// parent binding so multiple CBs in a shared register space stay
44+
// disambiguated downstream.
2745
struct ByteGranularityParameterUsageInfo
2846
{
2947
IRGlobalParam* param;
3048
UInt parentSpace;
49+
UInt parentBindingSpace;
3150
UInt parentBindingIndex;
3251
List<ByteGranularityUsageRange> ranges;
3352
bool isUntracked;

‎source/slang/slang-ir-metadata.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,7 @@ void collectMetadata(const IRModule* irModule, ArtifactPostEmitMetadata& outMeta
222222
{
223223
UniformParamUsage entry;
224224
entry.parentSpace = info.parentSpace;
225+
entry.parentBindingSpace = info.parentBindingSpace;
225226
entry.parentBindingIndex = info.parentBindingIndex;
226227
entry.isUntracked = info.isUntracked;
227228
for (auto& r : info.ranges)

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1154,7 +1154,10 @@ static void emitEntryPointParamJSON(
11541154
{
11551155
for (const auto& entry : postEmit->getUniformParamUsage())
11561156
{
1157-
if (entry.parentSpace != spaceIndex ||
1157+
// Match the parent binding's own space (which includes
1158+
// the descriptor set), since spaceIndex here came from
1159+
// getBindingSpace on the parent CB or parameter block.
1160+
if (entry.parentBindingSpace != spaceIndex ||
11581161
entry.parentBindingIndex != parentBindingIndex)
11591162
continue;
11601163
if (entry.isUntracked)

‎tests/reflection/used-uniforms-vulkan-cb-nonzero-set.slang‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// used-uniforms-vulkan-cb-nonzero-set.slang
2+
3+
// Regression test for per uniform field tracking on a constant buffer
4+
// bound to a non zero Vulkan descriptor set. The sibling test
5+
// used-uniforms-vulkan-cb.slang uses set 0, so the set never appears in
6+
// the parent binding space and a pass that dropped the set would still
7+
// match. Here the explicit vk::binding puts the CB in set 2, so the
8+
// recorded parent binding space must include the set or the emitter's
9+
// getBindingSpace lookup misses and usedByteRanges silently drops off
10+
// the parameter. used0 and used2 are read; unused1 and unused3 are not.
11+
12+
//TEST:REFLECTION:-stage compute -entry main -target spirv
13+
14+
struct Data
15+
{
16+
float used0;
17+
float unused1;
18+
float used2;
19+
float unused3;
20+
}
21+
22+
[[vk::binding(3, 2)]] ConstantBuffer<Data> c;
23+
24+
RWStructuredBuffer<float> outBuf;
25+
26+
[numthreads(1, 1, 1)]
27+
void main()
28+
{
29+
outBuf[0] = c.used0 + c.used2;
30+
}

‎tests/reflection/used-uniforms-vulkan-cb-nonzero-set.slang.expected‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
result code = 0
2+
standard error = {
3+
}
4+
standard output = {
5+
{
6+
"parameters": [
7+
{
8+
"name": "c",
9+
"binding": {"kind": "descriptorTableSlot", "space": 2, "index": 3},
10+
"type": {
11+
"kind": "constantBuffer",
12+
"elementType": {
13+
"kind": "struct",
14+
"name": "Data",
15+
"fields": [
16+
{
17+
"name": "used0",
18+
"type": {
19+
"kind": "scalar",
20+
"scalarType": "float32"
21+
},
22+
"binding": {"kind": "uniform", "offset": 0, "size": 4, "elementStride": 0}
23+
},
24+
{
25+
"name": "unused1",
26+
"type": {
27+
"kind": "scalar",
28+
"scalarType": "float32"
29+
},
30+
"binding": {"kind": "uniform", "offset": 4, "size": 4, "elementStride": 0}
31+
},
32+
{
33+
"name": "used2",
34+
"type": {
35+
"kind": "scalar",
36+
"scalarType": "float32"
37+
},
38+
"binding": {"kind": "uniform", "offset": 8, "size": 4, "elementStride": 0}
39+
},
40+
{
41+
"name": "unused3",
42+
"type": {
43+
"kind": "scalar",
44+
"scalarType": "float32"
45+
},
46+
"binding": {"kind": "uniform", "offset": 12, "size": 4, "elementStride": 0}
47+
}
48+
]
49+
},
50+
"containerVarLayout": {
51+
"binding": {"kind": "descriptorTableSlot", "index": 0}
52+
},
53+
"elementVarLayout": {
54+
"type": {
55+
"kind": "struct",
56+
"name": "Data",
57+
"fields": [
58+
{
59+
"name": "used0",
60+
"type": {
61+
"kind": "scalar",
62+
"scalarType": "float32"
63+
},
64+
"binding": {"kind": "uniform", "offset": 0, "size": 4, "elementStride": 0}
65+
},
66+
{
67+
"name": "unused1",
68+
"type": {
69+
"kind": "scalar",
70+
"scalarType": "float32"
71+
},
72+
"binding": {"kind": "uniform", "offset": 4, "size": 4, "elementStride": 0}
73+
},
74+
{
75+
"name": "used2",
76+
"type": {
77+
"kind": "scalar",
78+
"scalarType": "float32"
79+
},
80+
"binding": {"kind": "uniform", "offset": 8, "size": 4, "elementStride": 0}
81+
},
82+
{
83+
"name": "unused3",
84+
"type": {
85+
"kind": "scalar",
86+
"scalarType": "float32"
87+
},
88+
"binding": {"kind": "uniform", "offset": 12, "size": 4, "elementStride": 0}
89+
}
90+
]
91+
},
92+
"binding": {"kind": "uniform", "offset": 0, "size": 16, "elementStride": 0}
93+
}
94+
}
95+
},
96+
{
97+
"name": "outBuf",
98+
"binding": {"kind": "descriptorTableSlot", "index": 0},
99+
"type": {
100+
"kind": "resource",
101+
"baseShape": "structuredBuffer",
102+
"access": "readWrite",
103+
"resultType": {
104+
"kind": "scalar",
105+
"scalarType": "float32"
106+
}
107+
}
108+
}
109+
],
110+
"entryPoints": [
111+
{
112+
"name": "main",
113+
"stage": "compute",
114+
"threadGroupSize": [1, 1, 1],
115+
"bindings": [
116+
{
117+
"name": "c",
118+
"binding": {"kind": "descriptorTableSlot", "space": 2, "index": 3},
119+
"usedByteRanges": [
120+
{"offset": 0, "size": 4},
121+
{"offset": 8, "size": 4}
122+
]
123+
},
124+
{
125+
"name": "outBuf",
126+
"binding": {"kind": "descriptorTableSlot", "index": 0, "used": 1}
127+
}
128+
]
129+
}
130+
],
131+
"bindlessSpaceIndex": 1
132+
}
133+
}

0 commit comments

Comments
 (0)