Require narrow StorageBuffer{8,16}BitAccess for storage-buffer 8/16-bit access (#9910)

[nv-slang-bot]

Motivation

An 8-bit type used only through a storage buffer makes Slang emit the broad SPIR-V capability
UniformAndStorageBuffer8BitAccess where the narrower StorageBuffer8BitAccess is sufficient.
Consider the reporter's shader:

[shader("compute")]
[numthreads(512, 1, 1)]
void _name(uniform uint N, uniform uint scalar, RWStructuredBuffer<uint8_t> in_out, uint gid: SV_DispatchThreadID) {
    if (gid >= N) return;
    in_out[gid] = uint8_t(scalar);
}

Compiled with -target spirv, this emits OpCapability UniformAndStorageBuffer8BitAccess. That
capability requires the Vulkan feature uniformAndStorageBuffer8BitAccess, which many devices (e.g.
the reporter's RTX 3090) do not advertise even though they do expose storageBuffer8BitAccess. The
module is therefore rejected although only storage-buffer 8-bit access is actually used. The 16-bit
analog (UniformAndStorageBuffer16BitAccess vs StorageBuffer16BitAccess) has the identical bug.
This is a gap in #8194: for the Uniform/StorageBuffer pair it wired only the broad variants (it did
separately, and correctly, split the PushConstant and Input/Output capabilities) — not a regression.

Proposed solution

The storage class already arrives distinct at the decision site — a RWStructuredBuffer<uint8_t> is
correctly SpvStorageClassStorageBuffer (addressSpaceToStorageClass), a genuine uniform/constant
buffer is SpvStorageClassUniform. The information to pick the narrow capability was present and
simply discarded by a shared switch arm.

requireCapabilitiesForType in source/slang/slang-emit-spirv.cpp coalesces
SpvStorageClassUniform and SpvStorageClassStorageBuffer into one case in both its switches
(the flag-collection switch and the require switch). This change splits SpvStorageClassStorageBuffer
into its own arm that requires the narrow StorageBuffer{8,16}BitAccess, while SpvStorageClassUniform
keeps requiring the broad UniformAndStorageBuffer{8,16}BitAccess. This mirrors the
SpvStorageClassPushConstant arm in the same function, which was already correctly split
(StoragePushConstant8/16).

The one subtlety is that the same struct type can be used in both a uniform buffer and a storage
buffer in one module, so both arms can fire (in either order). The broad capability is a strict
superset of the narrow one, so the fix maintains a single invariant: the module carries the minimal
capability set covering all uses — never under-requires (which would be invalid SPIR-V), never
carries both the narrow and the broad form. This is enforced with subsumption logic:

• The storage-buffer arm skips the narrow capability when the broad one is already present.
• The uniform arm always adds the broad capability and removes a previously-added narrow one
  (an upgrade), so ordering never leaves both in the module.

Change summary

File	Change
source/slang/slang-emit-spirv.cpp (requireCapabilitiesForType)	Split SpvStorageClassStorageBuffer from SpvStorageClassUniform in both switches; require narrow StorageBuffer{8,16}BitAccess for storage buffers, broad for uniform buffers; add subsumption (skip narrow when broad present; upgrade — add broad and remove narrow — from the uniform arm).
source/slang/slang-emit-spirv.cpp (requireSPIRVCapability / removeSPIRVCapability)	Add m_capabilityInsts (capability → emitted OpCapability inst) and a symmetric removeSPIRVCapability so a subsumed capability can be unlinked from the module.
source/slang/slang-emit-spirv.cpp (SpvInstParent::addInst)	Set inst->parent when adding a section's first child. It was previously left null (only the subsequent-child path set it), which the capability-removal path's sibling assertion relies on.
tests/spirv/capability-uniform-and-storage.slang	Rework the fixture: uniform buffer (IN_*) asserts the broad capability; storage buffer (OUT_*) asserts the narrow one and -NOT the broad; an independent-size mixed case (broad-16 + narrow-8 coexist); and a SHARED case binding one Shared struct type through both a ConstantBuffer and a RWStructuredBuffer for 8- and 16-bit (broad subsumes narrow → broad only).

Concepts and vocabulary

• Narrow vs broad 8/16-bit storage capability. StorageBuffer8BitAccess (4448) /
  StorageBuffer16BitAccess (4433) permit 8/16-bit access through the StorageBuffer storage class
  only. UniformAndStorageBuffer8BitAccess (4449) / ...16BitAccess (4434) additionally permit
  8/16-bit access through the Uniform (constant/UBO) storage class, and thus subsume the narrow
  form. All four enumerants are already vendored in external/spirv — no capdef/header change.
• requireCapabilitiesForType(IRType*, SpvStorageClass). Emitter helper called during pointer-type
  emission. Phase 1 builds a TypeNeedsStorageFlags mask of which element sizes to search for (based
  on which capabilities are still missing for this storage class); phase 2, after walking the type,
  requires the corresponding capabilities.
• requireSPIRVCapability / eager emission. Requiring a capability appends its OpCapability
  instruction immediately, on first add. removeSPIRVCapability reverses that by unlinking the
  tracked instruction — safe because OpCapability has no result id and is referenced by nothing.

Process report

Split of the coalesced arm (both switches). The input shape here — the SpvStorageClass value —
is correct and principled: addressSpaceToStorageClass faithfully maps a RWStructuredBuffer to
SpvStorageClassStorageBuffer and a uniform/constant buffer to SpvStorageClassUniform, and both
callers (kIROp_SPIRVUntypedPtrType and the kIROp_PtrType family) pass it through unchanged. So
the producer is right; the bug was purely that the consumer collapsed two distinct classes into one
case and hard-coded the broad capability. The fix therefore belongs exactly here, at the consumer,
and does not paper over any upstream representation. Without the split, the test's OUT_UINT8
storage-buffer case emits UniformAndStorageBuffer8BitAccess (reproduced on top-of-tree) instead of
the correct StorageBuffer8BitAccess.

Subsumption / removeSPIRVCapability + m_capabilityInsts. These exist to hold the invariant
"minimal covering capability set, never both narrow and broad." Consider a module where the same
8-bit-containing struct is bound as both a ConstantBuffer (→ SpvStorageClassUniform) and a
RWStructuredBuffer (→ SpvStorageClassStorageBuffer). requireCapabilitiesForType runs once per
pointer-type emission, so both arms fire, and pointer-type emission order is not fixed. Two orderings:

• Storage first, then uniform: the storage arm adds StorageBuffer8BitAccess; then the uniform arm
  adds UniformAndStorageBuffer8BitAccess and calls removeSPIRVCapability(StorageBuffer8BitAccess),
  unlinking the now-subsumed narrow OpCapability. Result: broad only.
• Uniform first, then storage: the uniform arm adds the broad capability; then the storage arm's
  guard sees the broad capability already present and adds nothing. Result: broad only.

Either way the module ends with the single broad capability — never both, and never only the narrow
one when a uniform buffer also needs 8-bit (which would be invalid SPIR-V). The
m_capabilityInsts map is the minimal state needed to reverse an eager OpCapability emission; it is
populated in the one place capabilities managed by requireSPIRVCapability are emitted, and read only
by its symmetric removeSPIRVCapability, so there is a single source of truth. (The unconditional
Shader capability in emitFrontMatter is emitted directly via emitOpCapability and is deliberately
outside this bookkeeping — it is never a subsumption target.) This is a new mechanism, not a duplicate
representation: capability set membership stays in m_capabilities, and the map only records the
instruction to unlink.

SpvInstParent::addInst — first-child parent fix. Exercising removeSPIRVCapability uncovered a
latent bug. addInst sets inst->parent = this only on the subsequent-child path; when adding a
section's first child it set m_firstChild = m_lastChild = inst and returned without setting
parent. So the first OpCapability added to the Capabilities section had parent == nullptr. That
first capability is whichever one is required first during emission — emitFrontMatter (which emits
Shader) runs late in emitSPIRVModule (after emitSPIRVAnyCapabilities), so the storage caps
required while emitting types/entry points populate the section first. The bug then bites two ways:
removing a narrow capability that happens to be the section's first child is a silent no-op via
removeFromParent's if (!oldParent) return; (leaving both narrow and broad in the module); and
removing a later capability whose previous sibling is the null-parent first child trips
SLANG_ASSERT(pp->parent == oldParent) — a hard crash, which is what the same-size mixed UBO+SSBO case
hit. This is a genuine representation bug — the linked-list invariant "every child of a parent has its
parent set" was simply not established for the first child — so it is fixed at the producer
(addInst), not worked around in removeFromParent. No consumer used parent == nullptr as an
"is-first-child" sentinel (the only other reader, the forward-declared-pointer re-add at the end of
emission, reads spvPtrType->parent and re-adds it, which is now correct for a first child too), and
removeFromParent's if (!oldParent) return; guard still handles a genuinely unparented instruction.

A note on the test's storage-buffer cases not pinning -profile spirv_1_3. The pre-existing test
pinned SPIR-V 1.3, which is why it never caught this bug: at 1.3 a RWStructuredBuffer is emitted with
the legacy BufferBlock/Uniform encoding (storage class Uniform), for which the broad capability
is genuinely correct. The bug only manifests with the modern StorageBuffer storage class
(SPIR-V 1.4+/default), so the storage-buffer (OUT_*, SHARED, MIX) cases use the default profile;
the uniform (IN_*) cases keep -profile spirv_1_3 since a constant buffer is Uniform in both
encodings.

Closes #9910.

_{🤖 Generated by an automated Slang coworker — may be inaccurate. A human maintainer should verify.}

[jhelferty-nv]

Automated notice (PR board sync) — do not reply to this comment.

Auto-assigned @zangold-nv as shepherd for this Bot PR.

FYI for maintainers: committer signal on the changed files is highest for pdeayton-nv among collaborators other than the assignee. They were not auto-requested; a human may optionally add them as a reviewer.

[jhelferty-nv]

Automated notice (PR board sync) — do not reply to this comment.

Auto-assigned @zangold-nv as shepherd for this Bot PR.

FYI for maintainers: committer signal on the changed files is highest for pdeayton-nv among collaborators other than the assignee. They were not auto-requested; a human may optionally add them as a reviewer.