Fix #12711: skip void fields in byte-address struct load/store - #12724
Draft
nv-slang-bot[bot] wants to merge 1 commit into
Draft
Fix #12711: skip void fields in byte-address struct load/store#12724nv-slang-bot[bot] wants to merge 1 commit into
nv-slang-bot[bot] wants to merge 1 commit into
Conversation
Contributor
|
Automated notice (PR board sync) — do not reply to this comment. Auto-assigned @jkwak-work as shepherd for this Bot PR. FYI for maintainers: committer signal on the changed files is higher for expipiplus1 than for the auto-requested reviewer (jkwak-work). They were not auto-requested; a human may optionally add them as a reviewer. |
1 similar comment
Contributor
|
Automated notice (PR board sync) — do not reply to this comment. Auto-assigned @jkwak-work as shepherd for this Bot PR. FYI for maintainers: committer signal on the changed files is higher for expipiplus1 than for the auto-requested reviewer (jkwak-work). They were not auto-requested; a human may optionally add them as a reviewer. |
An empty/zero-sized struct field is turned into a void-typed field upstream by legalizeResourceTypes to keep field-count alignment. The byte-address-buffer legalization pass had no skip for it, so it emitted a per-field access of the void field: a Load<void> on HLSL/DXIL (invalid intrinsic) and, for SPIR-V/GLSL, offset / stride where stride(void) == 0, i.e. a divide by zero (E30002). Skip void-typed fields in emitLegalLoad and emitLegalStore before recursing, so no load/store is emitted for them and they are omitted from the reconstructed makeStruct; the later cleanUpVoidType pass drops the matching void field from the struct type, keeping operand and field counts consistent. Mirrors the existing void-field skip in slang-ir-lower-buffer-element-type.cpp and slang-ir-cleanup-void.cpp.
nv-slang-bot
Bot
force-pushed
the
fix/issue-12711
branch
from
August 25, 2026 03:36
66cd358 to
1218c6b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Loading a struct through
ByteAddressBuffer.Load<T>(offset)fails to compile whenT(recursively) contains an empty / zero-sized field. Consider this shader (based on the reporter's minimal
case in #12711):
error[E30002]: divide by zero.(g_buffer_0).Load<void >(4U);for the empty field, which DXCrejects with "External function used in non-library profile" (there is no
Load<void>intrinsic).
The store direction (
RWByteAddressBuffer.Store<Item>) is broken the same way. ReplacingEmpty bwith an ordinary field (
float c) compiles cleanly, confirming the empty field is the trigger.Proposed solution
Skip the empty field when the byte-address-buffer legalization pass decomposes a struct load/store
field-by-field. This is the principled fix because it sits exactly where the offending
per-field access is generated, and it matches an already-established in-tree pattern for the same
void-field shape (see the process report).Change summary
source/slang/slang-ir-byte-address-legalize.cppemitLegalLoadandemitLegalStore, skip avoid-typed struct field before recursing, so no per-field load/store is emitted for it and it is omitted from the reconstructedmakeStruct.tests/bugs/byte-address-load-empty-field.slang-target spirv,-target hlsl, and-target glsl(GLSL shares SPIR-V's divide-by-stride path). It covers the empty field in first position (ItemFirst { Empty a; float b; }) and middle position (ItemMid { float a; Empty b; float c; }); the middle case'sLoad<float >(20U)assertion proves the following real field keeps its natural offset across the skip. The forbiddenLoad<void>is checked by a dedicatedHLSLNOVOIDprefix whose only directive is aCHECK-NOT(so it scans the whole output and a first-position regression cannot escape). SPIR-V/GLSL assert codegen reaches entry-point emission (both abort withE30002onmaster).Concepts and vocabulary
slang-ir-byte-address-legalize.cpp): a pre-emit IR passthat turns a single
ByteAddressBuffer.Load<T>/.Store<T>into per-field accesses at computedbyte offsets. For GLSL/SPIR-V it further rewrites those into structured-buffer accesses, turning
a byte offset into an element index via
index = offset / stride.void-typed field: the representation an empty/zero-sized field is given before this passruns (see below). It is intentional, not malformed.
cleanUpVoidType: a later IR pass (slang-ir-cleanup-void.cpp) that removesvoidfieldsfrom struct types and the matching
voidoperands frommakeStruct/call.Process report
Root cause / producer→consumer trace. An empty field does not reach byte-address legalization
as an empty struct — it reaches it as a
void-typed field.legalizeResourceTypes(
slang-emit.cppinvokes it before byte-address legalization) rebuilds the struct and, inslang-legalize-types.cpp, deliberately assignsgetVoidType()to a field that legalized tonothing "to keep field count aligned". So when
emitLegalLoad(
slang-ir-byte-address-legalize.cpp, struct branch) walksItem's fields, fieldbhas typevoid. The field loop had no skip for it, so it recursed and bottomed out atemitSimpleLoad,which emits the access with the field's own (void) type:
emitSimpleLoadproduces aByteAddressBufferLoadwhose result type isvoid, and theC-like emitter prints
emitType(...)→.Load<void >(4U).translateToStructuredBufferOps), it computesindex = offset / getNaturalSizeAndAlignment(void).getStride(). The natural stride ofvoidis0, so this is a divide-by-zero; SCCP constant-folds theDivand raisesE30002.The fix and why it is at the right layer. Before recursing into a field in
emitLegalLoadandemitLegalStore, we nowcontinuepast any field whose typeas<IRVoidType>is non-null. A voidfield carries no data, so emitting no load/store for it — and omitting it from the reconstructed
makeStruct— is correct.Input-shape check (per the methodology): the
voidfield is valid, intentional input, not amalformed shape, so the consumer-side skip is principled rather than a band-aid:
legalizeResourceTypes→slang-legalize-types.cpp) to preserve field-count alignment for layout, and that alignment isrelied on elsewhere — so eliminating the field at the producer would be a much larger,
layout-affecting change (see the second alternative under Alternatives considered below, deferred).
voidfield as "skip me" with the identicalpattern:
slang-ir-lower-buffer-element-type.cppskipsas<IRVoidType>fields when building itspack/unpack functions, and
slang-ir-cleanup-void.cppremovesvoidfields from struct types.This PR simply applies the same, established treatment at the one byte-address site that was
missing it.
Why
as<IRVoidType>specifically (and not "natural size == 0"). The producer's output is avoid-typed field, and the latercleanUpVoidTypepass reconciles themakeStructoperand countagainst the struct's field count by dropping precisely the
voidoperands andvoidfields. Skipping by
voidtype keeps our emittedmakeStructconsistent with what that pass willdo. A retained empty-
structfield (a different shape) is already handled correctly by theexisting recursion — it enters the struct branch, its zero-field loop emits nothing, and
makeStructwith zero args yields an empty value — so it needs no special case here and must notbe dropped (doing so would leave a permanent operand/field-count mismatch that
cleanUpVoidTypewould not fix).
Alternatives considered.
HLSL/DXIL
Load<void>(a different symptom of the same missing skip) broken, so it isinsufficient as the fix. It could be added as cheap defense-in-depth if desired.
earlier). Most representation-pure, but the
voidfield exists specifically to keep field-countalignment for layout, so moving its elimination earlier is a broad, layout-affecting
pass-ordering change with a much larger blast radius. Flagged to maintainers as a possible
follow-up; not pursued here.
Testing.
tests/bugs/byte-address-load-empty-field.slangloads and stores two structs —ItemFirst { Empty a; float b; }andItemMid { float a; Empty b; float c; }— through aByteAddressBuffer/RWByteAddressBufferfor-target spirv,-target hlsl, and-target glsl.The empty field's position matters: because a
CHECK-NOTbounded by positiveCHECKs only scansthe region around them, a first-position regressed
Load<void>would be emitted before any positivematch and could escape an interleaved guard. So the forbidden load is checked by a dedicated
HLSLNOVOIDprefix whose only directive isHLSLNOVOID-NOT: Load<void, which scans the entireoutput. (There is no
Store<void>form to forbid — a byte-address store emits.Store(offset, value)with no template argument — so the store path is covered by successful compilation plus the positive
Store(check.)ItemMid'sLoad<float >(20U)assertion proves the field after the skipped emptyfield keeps its natural offset. SPIR-V and GLSL assert codegen reaches entry-point emission (both
abort with
E30002: divide by zeroonmaster, and HLSL withinternal error E99999). All fourdirectives fail on
masterand pass with this change; locallyslang-test tests/bugs/byte-address-load-empty-field.slangreports100% of tests passed (4/4),and re-running the existing
tests/bugs/*byte*,tests/spirv/*byte*,tests/bugs/empty-struct*,and
tests/hlsl-intrinsic/byte-address-buffer/tests shows no regressions. A must-fail mutationdrill (disabling only the load-path skip, rebuilding, re-running) confirms the checks are
non-vacuous: with that one skip removed the test drops to 1/4 — the
HLSLNOVOIDprefix fails onthe re-emitted first-position
Load<void>, and theSPIRV/GLSLprefixes fail because thezero-stride load division aborts codegen again (only the positive
HLSLprefix still passes, sincethe real fields are still loaded/stored) — and it returns to 4/4 once the skip is restored.
Closes #12711.
🤖 Generated by an automated Slang coworker — may be inaccurate. A human maintainer should verify.