diff options
| author | Tim Foley <tfoleyNV@users.noreply.github.com> | 2019-01-11 15:20:37 -0800 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2019-01-11 15:20:37 -0800 |
| commit | 35dfe37695b5703bc7d61aa03b17f195ee4c9e07 (patch) | |
| tree | 2f019db38053b6d748e088f4ea5c7f166b42effd | |
| parent | 7ba0a80cd56d7ec7f2a4bf8061134bf0d89f7673 (diff) | |
Fix some subtle bugs in D3D constant buffer layout (#771)
* Fix some subtle bugs in D3D constant buffer layout
The root of the issue here is that the D3D constant buffer layout rules require 16-byte alignment for arrays and structures, but they do *not* round up the size of an array/structure type to account for that alignment.
That means that in cases like the following:
```hlsl
cbuffer C0 { float3 a[2]; float c0; }
struct A { float4 x; float3 y; };
cbuffer C1 { A a; float c1; }
```
The `c0` and `c1` fields get an offset of 28 and not 32 like you might expect if the preceding array/structure field `a` had been padded out to match its 16-byte alignment.
The actual fix here is relatively simple, and mostly amount to shuffling around some code in `type-layout.cpp` to ensure that the D3D constant buffer layout don't inherit the logic that was rounding up array/structure sizes. Along the way I took the opportunity to clean up the inheritance hierarchy by making the GLSL-family layout rules not try to share anythign with the D3D family (not that there is very little to share), which in turn allowed for some simplification of the GLSL side of things.
Fixing this behavior changed the output of a few reflection tests. In the case of `tests/reflection/arrays.hlsl` the output confirmed that we had been producing bad reflection information in these kinds of cases. The output for `tests/reflection/matrix-layout.slang` also showed some bugs in our reflection, but these were overall more minor: we mis-reported the size of certain matrices as 64 bytes instead of 60, and as a result also computed the size of the overall constant buffer as 4 bytes bigger than needed. In all of these cases I double-checked the expected output against dxc to make sure that the new offsets/sizes are what we should have been producing in the first place.
I also updated the reflection test harness to start outputting layout information for the element type of a structured buffer, which changed the output of `tests/reflection/structured-buffer.slang`, but this didn't show any change in what we reported: it is just information that wasn't in the output to begin with.
Finally, I added two new tests around these subtle cases of buffer layout behavior (especially subtle because it varies across target APIs).
The `tests/compute/buffer-layout.slang` test simply sets up a type to ilustrate the troublesome scenarios and then embeds it in both a constant buffer and structured buffer that will be backed by memory with sequential `int` values. We then read out the value of a field as a way to probe its de facto *offset* at runtime. This test doesn't really stress the Slang compiler (except for our ability to pass through the same type declarations to downstream compilers), but it is useful to confirm our expectations about where things land in memory.
The `tests/reflection/buffer-layout.slang` test then uses the reflection test infrastructure to confirm that the same type declarations used in the compute test produce the expected offsets in our reported reflection information. Before the fixes in this change this test showed us producing dangerously incorrect results in our D3D reflection information, which has now been fixed to match the empirically-determined offsets from the compute test.
* fixups based on review feedback
| -rw-r--r-- | source/slang/type-layout.cpp | 234 | ||||
| -rw-r--r-- | source/slang/type-layout.h | 10 | ||||
| -rw-r--r-- | tests/compute/buffer-layout.slang | 131 | ||||
| -rw-r--r-- | tests/compute/buffer-layout.slang.1.expected.txt | 4 | ||||
| -rw-r--r-- | tests/compute/buffer-layout.slang.2.expected.txt | 4 | ||||
| -rw-r--r-- | tests/compute/buffer-layout.slang.expected.txt | 4 | ||||
| -rw-r--r-- | tests/reflection/arrays.hlsl.expected | 4 | ||||
| -rw-r--r-- | tests/reflection/buffer-layout.slang | 109 | ||||
| -rw-r--r-- | tests/reflection/buffer-layout.slang.1.expected | 192 | ||||
| -rw-r--r-- | tests/reflection/buffer-layout.slang.expected | 192 | ||||
| -rw-r--r-- | tests/reflection/matrix-layout.slang.1.expected | 6 | ||||
| -rw-r--r-- | tests/reflection/matrix-layout.slang.expected | 10 | ||||
| -rw-r--r-- | tests/reflection/structured-buffer.slang.expected | 40 | ||||
| -rw-r--r-- | tools/slang-reflection-test/slang-reflection-test-main.cpp | 148 |
14 files changed, 944 insertions, 144 deletions
diff --git a/source/slang/type-layout.cpp b/source/slang/type-layout.cpp index 4281829be..f5cd518b8 100644 --- a/source/slang/type-layout.cpp +++ b/source/slang/type-layout.cpp @@ -78,13 +78,36 @@ struct DefaultLayoutRulesImpl : SimpleLayoutRulesImpl SimpleArrayLayoutInfo GetArrayLayout( SimpleLayoutInfo elementInfo, LayoutSize elementCount) override { SLANG_RELEASE_ASSERT(elementInfo.size.isFinite()); - auto stride = elementInfo.size.getFiniteValue(); + auto elementSize = elementInfo.size.getFiniteValue(); + auto elementAlignment = elementInfo.alignment; + auto elementStride = RoundToAlignment(elementSize, elementAlignment); + + // An array with no elements will have zero size. + // + LayoutSize arraySize = 0; + // + // Any array with a non-zero number of elements will need + // to have space for N elements of size `elementSize`, with + // the constraints that there must be `elementStride` bytes + // between consecutive elements. + // + if( elementCount > 0 ) + { + // We can think of this as either allocating (N-1) + // chunks of size `elementStride` (for most of the elements) + // and then one final chunk of size `elementSize` for + // the last element, or equivalently as allocating + // N chunks of size `elementStride` and then "giving back" + // the final `elementStride - elementSize` bytes. + // + arraySize = (elementStride * (elementCount-1)) + elementSize; + } SimpleArrayLayoutInfo arrayInfo; arrayInfo.kind = elementInfo.kind; - arrayInfo.size = stride * elementCount; - arrayInfo.alignment = elementInfo.alignment; - arrayInfo.elementStride = stride; + arrayInfo.size = arraySize; + arrayInfo.alignment = elementAlignment; + arrayInfo.elementStride = elementStride; return arrayInfo; } @@ -125,86 +148,179 @@ struct DefaultLayoutRulesImpl : SimpleLayoutRulesImpl if(fieldInfo.size == 0) return ioStructInfo->size; + // A struct type must be at least as aligned as its most-aligned field. ioStructInfo->alignment = std::max(ioStructInfo->alignment, fieldInfo.alignment); - ioStructInfo->size = RoundToAlignment(ioStructInfo->size, fieldInfo.alignment); - LayoutSize fieldOffset = ioStructInfo->size; - ioStructInfo->size += fieldInfo.size; + + // The new field will be added to the end of the struct. + auto fieldBaseOffset = ioStructInfo->size; + + // We need to ensure that the offset for the field will respect its alignment + auto fieldOffset = RoundToAlignment(fieldBaseOffset, fieldInfo.alignment); + + // The size of the struct must be adjusted to cover the bytes consumed + // by this field. + ioStructInfo->size = fieldOffset + fieldInfo.size; + return fieldOffset; } void EndStructLayout(UniformLayoutInfo* ioStructInfo) override { + SLANG_UNUSED(ioStructInfo); + + // Note: A traditional C layout algorithm would adjust the size + // of a struct type so that it is a multiple of the alignment. + // This is a parsimonious design choice because it means that + // `sizeof(T)` can both be used when copying/allocating a single + // value of type `T` or an array of N values, without having to + // consider more details. + // + // Of course the choice also has down-sides in that wrapping things + // into a `struct` can affect layout in ways that waste space. E.g., + // the following two cases don't lay out the same: + // + // struct S0 { double d; float f; float g; }; + // + // struct X { double d; float f; } + // struct S1 { X x; float g; } + // + // Even though `S0::g` and `S1::g` have the same amount of useful + // data in front of them, they will not land at the same offset, + // and the resulting struct sizes will differ (`sizeof(S0)` will be + // 16 while `sizeof(S1)` will be 24). + // + // Slang doesn't get to be opinionated about this stuff because + // there is already precedent in both HLSL and GLSL for types + // that have a size that is not rounded up to their alignment. + // + // Our default layout rules won't implement the C-like policy, + // and instead it will be injected in the concrete implementations + // that require it. + } +}; + + /// Common behavior for GLSL-family layout. +struct GLSLBaseLayoutRulesImpl : DefaultLayoutRulesImpl +{ + typedef DefaultLayoutRulesImpl Super; + + SimpleLayoutInfo GetVectorLayout(SimpleLayoutInfo elementInfo, size_t elementCount) override + { + // The `std140` and `std430` rules require vectors to be aligned to the next power of + // two up from their size (so a `float2` is 8-byte aligned, and a `float3` is + // 16-byte aligned). + // + // Note that in this case we have a type layout where the size is *not* a multiple + // of the alignment, so it should be possible to pack a scalar after a `float3`. + // + SLANG_RELEASE_ASSERT(elementInfo.kind == LayoutResourceKind::Uniform); + SLANG_RELEASE_ASSERT(elementInfo.size.isFinite()); + + auto size = elementInfo.size.getFiniteValue() * elementCount; + SimpleLayoutInfo vectorInfo( + LayoutResourceKind::Uniform, + size, + RoundUpToPowerOfTwo(size)); + return vectorInfo; + } + + SimpleArrayLayoutInfo GetArrayLayout( SimpleLayoutInfo elementInfo, LayoutSize elementCount) override + { + // The size of an array must be rounded up to be a multiple of its alignment. + // + auto info = Super::GetArrayLayout(elementInfo, elementCount); + info.size = RoundToAlignment(info.size, info.alignment); + return info; + } + + void EndStructLayout(UniformLayoutInfo* ioStructInfo) override + { + // The size of a `struct` must be rounded up to be a multiple of its alignment. + // ioStructInfo->size = RoundToAlignment(ioStructInfo->size, ioStructInfo->alignment); } }; -// Capture common behavior betwen HLSL and GLSL (`std140`) constnat buffer rules -struct DefaultConstantBufferLayoutRulesImpl : DefaultLayoutRulesImpl + /// The GLSL `std430` layout rules. +struct Std430LayoutRulesImpl : GLSLBaseLayoutRulesImpl { - // The `std140` rules require that all array elements - // be a multiple of 16 bytes. - // - // HLSL agrees. + // These rules don't actually need any differences from our + // base/common GLSL layout rules. +}; + + /// The GLSL `std430` layout rules. +struct Std140LayoutRulesImpl : GLSLBaseLayoutRulesImpl +{ + typedef GLSLBaseLayoutRulesImpl Super; + SimpleArrayLayoutInfo GetArrayLayout(SimpleLayoutInfo elementInfo, LayoutSize elementCount) override { + // The `std140` rules require that array elements + // be aligned on 16-byte boundaries. + // if(elementInfo.kind == LayoutResourceKind::Uniform) { if (elementInfo.alignment < 16) elementInfo.alignment = 16; - elementInfo.size = RoundToAlignment(elementInfo.size, elementInfo.alignment); } - return DefaultLayoutRulesImpl::GetArrayLayout(elementInfo, elementCount); + return Super::GetArrayLayout(elementInfo, elementCount); } - // The `std140` rules require that a `struct` type be - // aligned to at least 16. - // - // HLSL agrees. UniformLayoutInfo BeginStructLayout() override { + // The `std140` rules require that a `struct` type + // be at least 16-byte aligned. + // return UniformLayoutInfo(0, 16); } }; -struct GLSLConstantBufferLayoutRulesImpl : DefaultConstantBufferLayoutRulesImpl +struct HLSLConstantBufferLayoutRulesImpl : DefaultLayoutRulesImpl { -}; + typedef DefaultLayoutRulesImpl Super; -// The `std140` and `std430` rules require vectors to be aligned to the next power of -// two up from their size (so a `float2` is 8-byte aligned, and a `float3` is -// 16-byte aligned). -// -// Note that in this case we have a type layout where the size is *not* a multiple -// of the alignment, so it should be possible to pack a scalar after a `float3`. -static SimpleLayoutInfo getGLSLVectorLayout( - SimpleLayoutInfo elementInfo, size_t elementCount) -{ - SLANG_RELEASE_ASSERT(elementInfo.kind == LayoutResourceKind::Uniform); - SLANG_RELEASE_ASSERT(elementInfo.size.isFinite()); - - auto size = elementInfo.size.getFiniteValue() * elementCount; - SimpleLayoutInfo vectorInfo( - LayoutResourceKind::Uniform, - size, - RoundUpToPowerOfTwo(size)); - return vectorInfo; -} + // Similar to GLSL `std140` rules, an HLSL constant buffer requires that + // `struct` and array types have 16-byte alignement. + // + // Unlike GLSL `std140`, the overall size of an array or `struct` type + // is *not* rounded up to the alignment, so it is possible for later + // fields to sneak into the "tail space" left behind by a preceding + // structure or array. E.g., in this example: + // + // struct S { float3 a[2]; float b; }; + // + // The stride of the array `a` is 16 bytes per element, but the size + // of `a` will only be 28 bytes (not 32), so that `b` can fit into + // the space after the last array element and the overall structure + // will have a size of 32 bytes. -// The `std140` rules combine the GLSL-specific layout for 3-vectors with the -// alignment padding for structures and arrays that is common to both HLSL -// and GLSL constant buffers. -struct Std140LayoutRulesImpl : GLSLConstantBufferLayoutRulesImpl -{ - SimpleLayoutInfo GetVectorLayout(SimpleLayoutInfo elementInfo, size_t elementCount) override + SimpleArrayLayoutInfo GetArrayLayout(SimpleLayoutInfo elementInfo, LayoutSize elementCount) override { - return getGLSLVectorLayout(elementInfo, elementCount); + if(elementInfo.kind == LayoutResourceKind::Uniform) + { + if (elementInfo.alignment < 16) + elementInfo.alignment = 16; + } + return Super::GetArrayLayout(elementInfo, elementCount); } -}; -struct HLSLConstantBufferLayoutRulesImpl : DefaultConstantBufferLayoutRulesImpl -{ - // Can't let a `struct` field straddle a register (16-byte) boundary + UniformLayoutInfo BeginStructLayout() override + { + return UniformLayoutInfo(0, 16); + } + + // HLSL layout rules do *not* impose additional alignment + // constraints on vectors (e.g., all of `float`, `float2`, + // `float3`, and `float4` have 4-byte alignment), but instead + // they impose a rule that any `struct` field must not + // "straddle" a 16-byte boundary. + // + // This has the effect of making it *look* like `float4` + // values have 16-byte alignment in practice, but the + // effects on `float2` and `float3` are more nuanched and + // lead to different result than the GLSL rules. + // LayoutSize AddStructField(UniformLayoutInfo* ioStructInfo, UniformLayoutInfo fieldInfo) override { // Skip zero-size fields @@ -234,18 +350,10 @@ struct HLSLConstantBufferLayoutRulesImpl : DefaultConstantBufferLayoutRulesImpl struct HLSLStructuredBufferLayoutRulesImpl : DefaultLayoutRulesImpl { - // TODO: customize these to be correct... -}; - -// The `std430` rules don't include the array/structure alignment padding that -// gets applied to constant buffers, but they do include the padding of 3-vectors -// to be aligned as 4-vectors. -struct Std430LayoutRulesImpl : DefaultLayoutRulesImpl -{ - SimpleLayoutInfo GetVectorLayout(SimpleLayoutInfo elementInfo, size_t elementCount) override - { - return getGLSLVectorLayout(elementInfo, elementCount); - } + // HLSL structured buffers drop the restrictions added for constant buffers, + // but retain the rules around not adjusting the size of an array or + // structure to its alignment. In this way they should match our + // default layout rules. }; struct DefaultVaryingLayoutRulesImpl : DefaultLayoutRulesImpl diff --git a/source/slang/type-layout.h b/source/slang/type-layout.h index 6f6dad055..c326ae989 100644 --- a/source/slang/type-layout.h +++ b/source/slang/type-layout.h @@ -176,6 +176,16 @@ inline LayoutSize maximum(LayoutSize left, LayoutSize right) right.getFiniteValue())); } +inline bool operator>(LayoutSize left, LayoutSize::RawValue right) +{ + return left.isInfinite() || (left.getFiniteValue() > right); +} + +inline bool operator<=(LayoutSize left, LayoutSize::RawValue right) +{ + return left.isFinite() && (left.getFiniteValue() <= right); +} + // Layout appropriate to "just memory" scenarios, // such as laying out the members of a constant buffer. struct UniformLayoutInfo diff --git a/tests/compute/buffer-layout.slang b/tests/compute/buffer-layout.slang new file mode 100644 index 000000000..145da99c8 --- /dev/null +++ b/tests/compute/buffer-layout.slang @@ -0,0 +1,131 @@ +// buffer-layout.slang + +// The goal of this test is to make sure that constant and structured +// buffers obey the rules we expect of the each target API. + +//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute +//TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -dx12 +//TEST(compute, vulkan):COMPARE_COMPUTE_EX:-vk -compute + + +//TEST_INPUT: ubuffer(data=[0 0 0 0], stride=4):dxbinding(0),glbinding(0),out +RWStructuredBuffer<int> outputBuffer; + +struct A +{ + float x; + float y; +} + +struct S +{ + // The first field in a struct isn't going to be that + // interesting, because it will always get offset zero, + // so we just use this to establish a poorly-aligned + // starting point for the next field. + // + // offset size alignment + // + // 0 4 4 + // + float z; + + // The `std140` and D3D constant buffer ruless both + // ensure a minimum of 16-byte alignment on `struct` + // types, but differ in that D3D does not round up + // the total size of a type to its alignment. + // + // The `std430` and structured buffer rules don't + // perform any over-alignment on `struct` types and + // instead align them using the "natural" rules one + // might expect of, e.g., a C compiler. + // + // offset size alignment + // + // cbuffer 16 8 16 + // std140 16 16 16 + // + // struct 4 8 4 + // std430 4 8 4 + // + A a; + + // Now we insert an ordinary `int` field just as + // a way to probe the offset so far. + // + // offset size alignment + // + // cbuffer 24 4 4 + // std140 32 4 4 + // + // struct 12 4 4 + // std430 12 4 4 + // + int b; + + // As our next stress-test case, we will insert an + // array with elements that aren't a multiple of + // 16 bytes in size. + // + // The contant/uniform buffer rules will set the + // array stride to a multiple of 16 bytes in this case. + // The only difference between D3D rules and `std140` + // here is that D3D does not round up the size to + // the alignment. + // + // The structured/std430 rules don't do anything + // to over-align an array, so it is laid out relatively + // naturally, but note that D3D still follows its rule + // of not letting a vector "straddle" a 16-byte boundary, + // even if it doesn't bump up the alignment of + // vector types. + // + // offset size alignment + // + // cbuffer 32 24 16 + // std140 48 32 32 + // + // struct 16 16 4 + // std430 16 16 8 + // + float2 c[2]; + + // Now we put in one more ordinary `int` field + // just to probe the offset computed so far. + // offset size alignment + // + // cbuffer 56 4 4 + // std140 80 4 4 + // + // struct 32 4 4 + // std430 32 4 4 + // + int d; +} + +//TEST_INPUT:cbuffer(data=[0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32]):dxbinding(0),glbinding(0) +ConstantBuffer<S> cb; + +//TEST_INPUT:ubuffer(data=[0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32],stride=4):dxbinding(0),glbinding(1) +RWStructuredBuffer<S> sb; + +int test(int val) +{ + val = val+1; + val = val*16 + cb.b; + val = val*256 + cb.d; + val = val*256 + sb[0].b; + val = val*256 + sb[0].d; + return val; +} + +[numthreads(4, 1, 1)] +void computeMain( + uint3 dispatchThreadID : SV_DispatchThreadID) +{ + uint tid = dispatchThreadID.x; + + int inVal = tid; + int outVal = test(inVal); + outputBuffer[tid] = outVal; +}
\ No newline at end of file diff --git a/tests/compute/buffer-layout.slang.1.expected.txt b/tests/compute/buffer-layout.slang.1.expected.txt new file mode 100644 index 000000000..5e8ac1dc4 --- /dev/null +++ b/tests/compute/buffer-layout.slang.1.expected.txt @@ -0,0 +1,4 @@ +160E0308 +260E0308 +360E0308 +460E0308 diff --git a/tests/compute/buffer-layout.slang.2.expected.txt b/tests/compute/buffer-layout.slang.2.expected.txt new file mode 100644 index 000000000..efeba7ec3 --- /dev/null +++ b/tests/compute/buffer-layout.slang.2.expected.txt @@ -0,0 +1,4 @@ +18140308 +28140308 +38140308 +48140308 diff --git a/tests/compute/buffer-layout.slang.expected.txt b/tests/compute/buffer-layout.slang.expected.txt new file mode 100644 index 000000000..5e8ac1dc4 --- /dev/null +++ b/tests/compute/buffer-layout.slang.expected.txt @@ -0,0 +1,4 @@ +160E0308 +260E0308 +360E0308 +460E0308 diff --git a/tests/reflection/arrays.hlsl.expected b/tests/reflection/arrays.hlsl.expected index b586e362a..f815ea88c 100644 --- a/tests/reflection/arrays.hlsl.expected +++ b/tests/reflection/arrays.hlsl.expected @@ -31,7 +31,7 @@ standard output = { }, "uniformStride": 16 }, - "binding": {"kind": "uniform", "offset": 16, "size": 160} + "binding": {"kind": "uniform", "offset": 16, "size": 148} }, { "name": "y", @@ -39,7 +39,7 @@ standard output = { "kind": "scalar", "scalarType": "float32" }, - "binding": {"kind": "uniform", "offset": 176, "size": 4} + "binding": {"kind": "uniform", "offset": 164, "size": 4} } ] } diff --git a/tests/reflection/buffer-layout.slang b/tests/reflection/buffer-layout.slang new file mode 100644 index 000000000..51b9680d1 --- /dev/null +++ b/tests/reflection/buffer-layout.slang @@ -0,0 +1,109 @@ +// buffer-layout.slang + +// This test mirrors `tests/compute/buffer-layout.slang`, and it meant +// to confirm that our reflection logic correctly reports the offsets +// that the compute test sees in practice. + +//TEST:REFLECTION:-stage compute -entry main -target hlsl +//TEST:REFLECTION:-stage compute -entry main -target spirv + +struct A +{ + float x; + float y; +} + +struct S +{ + // The first field in a struct isn't going to be that + // interesting, because it will always get offset zero, + // so we just use this to establish a poorly-aligned + // starting point for the next field. + // + // offset size alignment + // + // 0 4 4 + // + float z; + + // The `std140` and D3D constant buffer ruless both + // ensure a minimum of 16-byte alignment on `struct` + // types, but differ in that D3D does not round up + // the total size of a type to its alignment. + // + // The `std430` and structured buffer rules don't + // perform any over-alignment on `struct` types and + // instead align them using the "natural" rules one + // might expect of, e.g., a C compiler. + // + // offset size alignment + // + // cbuffer 16 8 16 + // std140 16 16 16 + // + // struct 4 8 4 + // std430 4 8 4 + // + A a; + + // Now we insert an ordinary `int` field just as + // a way to probe the offset so far. + // + // offset size alignment + // + // cbuffer 24 4 4 + // std140 32 4 4 + // + // struct 12 4 4 + // std430 12 4 4 + // + int b; + + // As our next stress-test case, we will insert an + // array with elements that aren't a multiple of + // 16 bytes in size. + // + // The contant/uniform buffer rules will set the + // array stride to a multiple of 16 bytes in this case. + // The only difference between D3D rules and `std140` + // here is that D3D does not round up the size to + // the alignment. + // + // The structured/std430 rules don't do anything + // to over-align an array, so it is laid out relatively + // naturally, but note that D3D still follows its rule + // of not letting a vector "straddle" a 16-byte boundary, + // even if it doesn't bump up the alignment of + // vector types. + // + // offset size alignment + // + // cbuffer 32 24 16 + // std140 48 32 32 + // + // struct 16 16 4 + // std430 16 16 8 + // + float2 c[2]; + + // Now we put in one more ordinary `int` field + // just to probe the offset computed so far. + // offset size alignment + // + // cbuffer 56 4 4 + // std140 80 4 4 + // + // struct 32 4 4 + // std430 32 4 4 + // + int d; +} + +ConstantBuffer<S> cb; +RWStructuredBuffer<S> sb; + +[numthreads(1, 1, 1)] +void main( + uint3 dispatchThreadID : SV_DispatchThreadID) +{ +}
\ No newline at end of file diff --git a/tests/reflection/buffer-layout.slang.1.expected b/tests/reflection/buffer-layout.slang.1.expected new file mode 100644 index 000000000..890f31481 --- /dev/null +++ b/tests/reflection/buffer-layout.slang.1.expected @@ -0,0 +1,192 @@ +result code = 0 +standard error = { +} +standard output = { +{ + "parameters": [ + { + "name": "cb", + "binding": {"kind": "descriptorTableSlot", "index": 0}, + "type": { + "kind": "constantBuffer", + "elementType": { + "kind": "struct", + "name": "S", + "fields": [ + { + "name": "z", + "type": { + "kind": "scalar", + "scalarType": "float32" + }, + "binding": {"kind": "uniform", "offset": 0, "size": 4} + }, + { + "name": "a", + "type": { + "kind": "struct", + "name": "A", + "fields": [ + { + "name": "x", + "type": { + "kind": "scalar", + "scalarType": "float32" + }, + "binding": {"kind": "uniform", "offset": 0, "size": 4} + }, + { + "name": "y", + "type": { + "kind": "scalar", + "scalarType": "float32" + }, + "binding": {"kind": "uniform", "offset": 4, "size": 4} + } + ] + }, + "binding": {"kind": "uniform", "offset": 16, "size": 16} + }, + { + "name": "b", + "type": { + "kind": "scalar", + "scalarType": "int32" + }, + "binding": {"kind": "uniform", "offset": 32, "size": 4} + }, + { + "name": "c", + "type": { + "kind": "array", + "elementCount": 2, + "elementType": { + "kind": "vector", + "elementCount": 2, + "elementType": { + "kind": "scalar", + "scalarType": "float32" + } + }, + "uniformStride": 16 + }, + "binding": {"kind": "uniform", "offset": 48, "size": 32} + }, + { + "name": "d", + "type": { + "kind": "scalar", + "scalarType": "int32" + }, + "binding": {"kind": "uniform", "offset": 80, "size": 4} + } + ] + } + } + }, + { + "name": "sb", + "binding": {"kind": "descriptorTableSlot", "index": 1}, + "type": { + "kind": "resource", + "baseShape": "structuredBuffer", + "access": "readWrite", + "resultType": { + "kind": "struct", + "name": "S", + "fields": [ + { + "name": "z", + "type": { + "kind": "scalar", + "scalarType": "float32" + }, + "binding": {"kind": "uniform", "offset": 0, "size": 4} + }, + { + "name": "a", + "type": { + "kind": "struct", + "name": "A", + "fields": [ + { + "name": "x", + "type": { + "kind": "scalar", + "scalarType": "float32" + }, + "binding": {"kind": "uniform", "offset": 0, "size": 4} + }, + { + "name": "y", + "type": { + "kind": "scalar", + "scalarType": "float32" + }, + "binding": {"kind": "uniform", "offset": 4, "size": 4} + } + ] + }, + "binding": {"kind": "uniform", "offset": 4, "size": 8} + }, + { + "name": "b", + "type": { + "kind": "scalar", + "scalarType": "int32" + }, + "binding": {"kind": "uniform", "offset": 12, "size": 4} + }, + { + "name": "c", + "type": { + "kind": "array", + "elementCount": 2, + "elementType": { + "kind": "vector", + "elementCount": 2, + "elementType": { + "kind": "scalar", + "scalarType": "float32" + } + }, + "uniformStride": 8 + }, + "binding": {"kind": "uniform", "offset": 16, "size": 16} + }, + { + "name": "d", + "type": { + "kind": "scalar", + "scalarType": "int32" + }, + "binding": {"kind": "uniform", "offset": 32, "size": 4} + } + ] + } + } + } + ], + "entryPoints": [ + { + "name": "main", + "stage:": "compute", + "parameters": [ + { + "name": "dispatchThreadID", + "semanticName": "SV_DISPATCHTHREADID", + "type": { + "kind": "vector", + "elementCount": 3, + "elementType": { + "kind": "scalar", + "scalarType": "uint32" + } + } + } + ], + "threadGroupSize": [1, 1, 1] + } + ] +} +} diff --git a/tests/reflection/buffer-layout.slang.expected b/tests/reflection/buffer-layout.slang.expected new file mode 100644 index 000000000..25a41bc6d --- /dev/null +++ b/tests/reflection/buffer-layout.slang.expected @@ -0,0 +1,192 @@ +result code = 0 +standard error = { +} +standard output = { +{ + "parameters": [ + { + "name": "cb", + "binding": {"kind": "constantBuffer", "index": 0}, + "type": { + "kind": "constantBuffer", + "elementType": { + "kind": "struct", + "name": "S", + "fields": [ + { + "name": "z", + "type": { + "kind": "scalar", + "scalarType": "float32" + }, + "binding": {"kind": "uniform", "offset": 0, "size": 4} + }, + { + "name": "a", + "type": { + "kind": "struct", + "name": "A", + "fields": [ + { + "name": "x", + "type": { + "kind": "scalar", + "scalarType": "float32" + }, + "binding": {"kind": "uniform", "offset": 0, "size": 4} + }, + { + "name": "y", + "type": { + "kind": "scalar", + "scalarType": "float32" + }, + "binding": {"kind": "uniform", "offset": 4, "size": 4} + } + ] + }, + "binding": {"kind": "uniform", "offset": 16, "size": 8} + }, + { + "name": "b", + "type": { + "kind": "scalar", + "scalarType": "int32" + }, + "binding": {"kind": "uniform", "offset": 24, "size": 4} + }, + { + "name": "c", + "type": { + "kind": "array", + "elementCount": 2, + "elementType": { + "kind": "vector", + "elementCount": 2, + "elementType": { + "kind": "scalar", + "scalarType": "float32" + } + }, + "uniformStride": 16 + }, + "binding": {"kind": "uniform", "offset": 32, "size": 24} + }, + { + "name": "d", + "type": { + "kind": "scalar", + "scalarType": "int32" + }, + "binding": {"kind": "uniform", "offset": 56, "size": 4} + } + ] + } + } + }, + { + "name": "sb", + "binding": {"kind": "unorderedAccess", "index": 0}, + "type": { + "kind": "resource", + "baseShape": "structuredBuffer", + "access": "readWrite", + "resultType": { + "kind": "struct", + "name": "S", + "fields": [ + { + "name": "z", + "type": { + "kind": "scalar", + "scalarType": "float32" + }, + "binding": {"kind": "uniform", "offset": 0, "size": 4} + }, + { + "name": "a", + "type": { + "kind": "struct", + "name": "A", + "fields": [ + { + "name": "x", + "type": { + "kind": "scalar", + "scalarType": "float32" + }, + "binding": {"kind": "uniform", "offset": 0, "size": 4} + }, + { + "name": "y", + "type": { + "kind": "scalar", + "scalarType": "float32" + }, + "binding": {"kind": "uniform", "offset": 4, "size": 4} + } + ] + }, + "binding": {"kind": "uniform", "offset": 4, "size": 8} + }, + { + "name": "b", + "type": { + "kind": "scalar", + "scalarType": "int32" + }, + "binding": {"kind": "uniform", "offset": 12, "size": 4} + }, + { + "name": "c", + "type": { + "kind": "array", + "elementCount": 2, + "elementType": { + "kind": "vector", + "elementCount": 2, + "elementType": { + "kind": "scalar", + "scalarType": "float32" + } + }, + "uniformStride": 8 + }, + "binding": {"kind": "uniform", "offset": 16, "size": 16} + }, + { + "name": "d", + "type": { + "kind": "scalar", + "scalarType": "int32" + }, + "binding": {"kind": "uniform", "offset": 32, "size": 4} + } + ] + } + } + } + ], + "entryPoints": [ + { + "name": "main", + "stage:": "compute", + "parameters": [ + { + "name": "dispatchThreadID", + "semanticName": "SV_DISPATCHTHREADID", + "type": { + "kind": "vector", + "elementCount": 3, + "elementType": { + "kind": "scalar", + "scalarType": "uint32" + } + } + } + ], + "threadGroupSize": [1, 1, 1] + } + ] +} +} diff --git a/tests/reflection/matrix-layout.slang.1.expected b/tests/reflection/matrix-layout.slang.1.expected index 221b96a0b..57fd29c6d 100644 --- a/tests/reflection/matrix-layout.slang.1.expected +++ b/tests/reflection/matrix-layout.slang.1.expected @@ -49,7 +49,7 @@ standard output = { "scalarType": "float32" } }, - "binding": {"kind": "uniform", "offset": 96, "size": 64} + "binding": {"kind": "uniform", "offset": 96, "size": 60} } ] } @@ -106,11 +106,11 @@ standard output = { "scalarType": "float32" } }, - "binding": {"kind": "uniform", "offset": 96, "size": 64} + "binding": {"kind": "uniform", "offset": 96, "size": 60} } ] }, - "binding": {"kind": "uniform", "offset": 0, "size": 160} + "binding": {"kind": "uniform", "offset": 0, "size": 156} } ] } diff --git a/tests/reflection/matrix-layout.slang.expected b/tests/reflection/matrix-layout.slang.expected index 04c861ed3..c8aeb2ae1 100644 --- a/tests/reflection/matrix-layout.slang.expected +++ b/tests/reflection/matrix-layout.slang.expected @@ -23,7 +23,7 @@ standard output = { "scalarType": "float32" } }, - "binding": {"kind": "uniform", "offset": 0, "size": 64} + "binding": {"kind": "uniform", "offset": 0, "size": 60} }, { "name": "ab", @@ -49,7 +49,7 @@ standard output = { "scalarType": "float32" } }, - "binding": {"kind": "uniform", "offset": 112, "size": 64} + "binding": {"kind": "uniform", "offset": 112, "size": 60} } ] } @@ -80,7 +80,7 @@ standard output = { "scalarType": "float32" } }, - "binding": {"kind": "uniform", "offset": 0, "size": 64} + "binding": {"kind": "uniform", "offset": 0, "size": 60} }, { "name": "bb", @@ -106,11 +106,11 @@ standard output = { "scalarType": "float32" } }, - "binding": {"kind": "uniform", "offset": 112, "size": 64} + "binding": {"kind": "uniform", "offset": 112, "size": 60} } ] }, - "binding": {"kind": "uniform", "offset": 0, "size": 176} + "binding": {"kind": "uniform", "offset": 0, "size": 172} } ] } diff --git a/tests/reflection/structured-buffer.slang.expected b/tests/reflection/structured-buffer.slang.expected index 70ceb64f2..3e79fd1c5 100644 --- a/tests/reflection/structured-buffer.slang.expected +++ b/tests/reflection/structured-buffer.slang.expected @@ -40,25 +40,35 @@ standard output = { "baseShape": "structuredBuffer", "resultType": { "kind": "struct", + "name": "S", "fields": [ - "name": "a", - "type": { - "kind": "vector", - "elementCount": 2, - "elementType": { + { + "name": "a", + "type": { + "kind": "vector", + "elementCount": 2, + "elementType": { + "kind": "scalar", + "scalarType": "float32" + } + }, + "binding": {"kind": "uniform", "offset": 0, "size": 8} + }, + { + "name": "b", + "type": { "kind": "scalar", "scalarType": "float32" - } - }, - "name": "b", - "type": { - "kind": "scalar", - "scalarType": "float32" + }, + "binding": {"kind": "uniform", "offset": 8, "size": 4} }, - "name": "c", - "type": { - "kind": "scalar", - "scalarType": "uint32" + { + "name": "c", + "type": { + "kind": "scalar", + "scalarType": "uint32" + }, + "binding": {"kind": "uniform", "offset": 12, "size": 4} } ] } diff --git a/tools/slang-reflection-test/slang-reflection-test-main.cpp b/tools/slang-reflection-test/slang-reflection-test-main.cpp index 071a15cd4..3ae614b22 100644 --- a/tools/slang-reflection-test/slang-reflection-test-main.cpp +++ b/tools/slang-reflection-test/slang-reflection-test-main.cpp @@ -308,6 +308,66 @@ static void emitReflectionScalarTypeInfoJSON( write(writer, "\""); } +static void emitReflectionResourceTypeBaseInfoJSON( + PrettyWriter& writer, + slang::TypeReflection* type) +{ + auto shape = type->getResourceShape(); + auto access = type->getResourceAccess(); + write(writer, "\"kind\": \"resource\""); + write(writer, ",\n"); + write(writer, "\"baseShape\": \""); + switch (shape & SLANG_RESOURCE_BASE_SHAPE_MASK) + { + default: + write(writer, "unknown"); + assert(!"unhandled case"); + break; + +#define CASE(SHAPE, NAME) case SLANG_##SHAPE: write(writer, #NAME); break + CASE(TEXTURE_1D, texture1D); + CASE(TEXTURE_2D, texture2D); + CASE(TEXTURE_3D, texture3D); + CASE(TEXTURE_CUBE, textureCube); + CASE(TEXTURE_BUFFER, textureBuffer); + CASE(STRUCTURED_BUFFER, structuredBuffer); + CASE(BYTE_ADDRESS_BUFFER, byteAddressBuffer); +#undef CASE + } + write(writer, "\""); + if (shape & SLANG_TEXTURE_ARRAY_FLAG) + { + write(writer, ",\n"); + write(writer, "\"array\": true"); + } + if (shape & SLANG_TEXTURE_MULTISAMPLE_FLAG) + { + write(writer, ",\n"); + write(writer, "\"multisample\": true"); + } + + if( access != SLANG_RESOURCE_ACCESS_READ ) + { + write(writer, ",\n\"access\": \""); + switch(access) + { + default: + write(writer, "unknown"); + assert(!"unhandled case"); + break; + + case SLANG_RESOURCE_ACCESS_READ: + break; + + case SLANG_RESOURCE_ACCESS_READ_WRITE: write(writer, "readWrite"); break; + case SLANG_RESOURCE_ACCESS_RASTER_ORDERED: write(writer, "rasterOrdered"); break; + case SLANG_RESOURCE_ACCESS_APPEND: write(writer, "append"); break; + case SLANG_RESOURCE_ACCESS_CONSUME: write(writer, "consume"); break; + } + write(writer, "\""); + } +} + static void emitReflectionTypeInfoJSON( PrettyWriter& writer, slang::TypeReflection* type) @@ -321,65 +381,13 @@ static void emitReflectionTypeInfoJSON( case slang::TypeReflection::Kind::Resource: { - auto shape = type->getResourceShape(); - auto access = type->getResourceAccess(); - write(writer, "\"kind\": \"resource\""); - write(writer, ",\n"); - write(writer, "\"baseShape\": \""); - switch (shape & SLANG_RESOURCE_BASE_SHAPE_MASK) - { - default: - write(writer, "unknown"); - assert(!"unhandled case"); - break; - -#define CASE(SHAPE, NAME) case SLANG_##SHAPE: write(writer, #NAME); break - CASE(TEXTURE_1D, texture1D); - CASE(TEXTURE_2D, texture2D); - CASE(TEXTURE_3D, texture3D); - CASE(TEXTURE_CUBE, textureCube); - CASE(TEXTURE_BUFFER, textureBuffer); - CASE(STRUCTURED_BUFFER, structuredBuffer); - CASE(BYTE_ADDRESS_BUFFER, byteAddressBuffer); -#undef CASE - } - write(writer, "\""); - if (shape & SLANG_TEXTURE_ARRAY_FLAG) - { - write(writer, ",\n"); - write(writer, "\"array\": true"); - } - if (shape & SLANG_TEXTURE_MULTISAMPLE_FLAG) - { - write(writer, ",\n"); - write(writer, "\"multisample\": true"); - } - - if( access != SLANG_RESOURCE_ACCESS_READ ) - { - write(writer, ",\n\"access\": \""); - switch(access) - { - default: - write(writer, "unknown"); - assert(!"unhandled case"); - break; - - case SLANG_RESOURCE_ACCESS_READ: - break; - - case SLANG_RESOURCE_ACCESS_READ_WRITE: write(writer, "readWrite"); break; - case SLANG_RESOURCE_ACCESS_RASTER_ORDERED: write(writer, "rasterOrdered"); break; - case SLANG_RESOURCE_ACCESS_APPEND: write(writer, "append"); break; - case SLANG_RESOURCE_ACCESS_CONSUME: write(writer, "consume"); break; - } - write(writer, "\""); - } + emitReflectionResourceTypeBaseInfoJSON(writer, type); // TODO: We should really print the result type for all resource // types, but current test output depends on the old behavior, so // we only add result type output for structured buffers at first. // + auto shape = type->getResourceShape(); switch (shape & SLANG_RESOURCE_BASE_SHAPE_MASK) { default: @@ -620,9 +628,37 @@ static void emitReflectionTypeLayoutInfoJSON( write(writer, ",\n"); emitReflectionNameInfoJSON(writer, typeLayout->getName()); break; - } - // TODO: emit size info for types + case slang::TypeReflection::Kind::Resource: + { + // Some resource types (notably structured buffers) + // encode layout information for their result/element + // type, but others don't. We need to check for + // the relevant cases here. + // + auto type = typeLayout->getType(); + auto shape = type->getResourceShape(); + + if( (shape & SLANG_RESOURCE_BASE_SHAPE_MASK) == SLANG_STRUCTURED_BUFFER ) + { + emitReflectionResourceTypeBaseInfoJSON(writer, type); + + if( auto resultTypeLayout = typeLayout->getElementTypeLayout() ) + { + write(writer, ",\n"); + write(writer, "\"resultType\": "); + emitReflectionTypeLayoutJSON( + writer, + resultTypeLayout); + } + } + else + { + emitReflectionTypeInfoJSON(writer, typeLayout->getType()); + } + } + break; + } } static void emitReflectionTypeLayoutJSON( |
