Fix dropped alignment operand in byteAddressBufferLoad specialization#1
Open
stramit wants to merge 1 commit into
Open
Fix dropped alignment operand in byteAddressBufferLoad specialization#1stramit wants to merge 1 commit into
stramit wants to merge 1 commit into
Conversation
specializeFuncsForBufferLoadArgs lumped kIROp_ByteAddressBufferLoad in
with the generic two-operand "element access" instructions (GetElement,
StructuredBufferLoad, etc.) in both getCallInfoForArg and
getSpecializedValueForArg. byteAddressBufferLoad actually has three
operands -- (buffer, offset, alignment) -- so the rewritten load inside
the specialized callee was constructed with only two operands.
Downstream, legalizeByteAddressBufferOps::processLoad unconditionally
reads operand index 2 to recover the alignment, hitting
assert failure: slang-ir.h(710): index < getOperandCount()
during SPIR-V emission. A minimal reproducer is a struct large enough
to trip the buffer-load specialization threshold (e.g. two float4x4s
plus a padded float3) loaded via Buf.Load<T>() and passed by value into
a helper function.
Give byteAddressBufferLoad dedicated handling in both functions so the
alignment operand survives specialization, and add a regression test
under tests/optimization that exercises the failing pattern on SPIR-V.
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.
Summary
specializeFuncsForBufferLoadArgslumpedkIROp_ByteAddressBufferLoadin with the generic two-operand "element access" instructions (GetElement,StructuredBufferLoad, etc.) in bothgatherCallInfoForArgandgetSpecializedValueForArg.byteAddressBufferLoadactually has three operands —(buffer, offset, alignment)— so the rewritten load inside the specialized callee was constructed with only two operands.Downstream,
legalizeByteAddressBufferOps::processLoadunconditionally reads operand index 2 to recover the alignment, hitting:during SPIR-V emission.
Repro
A struct large enough to trip the buffer-load specialization threshold, loaded via
Buf.Load<T>()and passed by value into a helper:slangc repro.slang -target spirv -O0 -fvk-use-scalar-layoutaborts with the assertion above.Fix
Give
byteAddressBufferLoaddedicated handling in both functions so the alignment operand survives specialization, and remove it fromisElementAccessInstsince it doesn't share the two-operand shape.Test plan
tests/optimization/buffer-load-specialize-byte-address.slangexercises the failing pattern on SPIR-V and checks that the specialized helper takes the offset and alignment as separateuintparameters.