Skip to content

Commit

Permalink
Add tests for multi-dim arrays and unbounded arrays (microsoft#5532)
Browse files Browse the repository at this point in the history
I added at test for unbounded arrays. This case works, so no code
changes were required.

I also added a test for multi-dimensional arrays. This case is not
handled allowed in Vulkan, so I chose to issue an error. If we need to
support it, we could try improve it later by transforming the array into
a single dimensional array.

See https://gitlab.khronos.org/spirv/SPIR-V/-/issues/743 and
https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#interfaces-resources-setandbinding.
  • Loading branch information
s-perron authored Aug 17, 2023
1 parent 15d086f commit 1d6b562
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 10 deletions.
5 changes: 3 additions & 2 deletions tools/clang/lib/SPIRV/DeclResultIdMapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1589,8 +1589,9 @@ void DeclResultIdMapper::createCounterVar(
const SpirvType *counterType = spvContext.getACSBufferCounterType();
QualType declType = decl->getType();
if (declType->isArrayType()) {
// TODO(5440): This codes does not handle multi-dimensional arrays. We need
// to look at specific example to determine the best way to do it.
// Vulkan does not support multi-dimentional arrays of resource, so we
// assume the array is a single dimensional array.
assert(!declType->getArrayElementTypeNoTypeQual()->isArrayType());
uint32_t arrayStride = 4;
if (const auto *constArrayType =
astContext.getAsConstantArrayType(declType)) {
Expand Down
21 changes: 13 additions & 8 deletions tools/clang/lib/SPIRV/SpirvEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1720,17 +1720,22 @@ void SpirvEmitter::doVarDecl(const VarDecl *decl) {

// Reject arrays of RW/append/consume structured buffers. They have assoicated
// counters, which are quite nasty to handle.
if (!spirvOptions.allowRWStructuredBufferArrays &&
decl->getType()->isArrayType()) {
auto type = decl->getType();
do {
type = type->getAsArrayTypeUnsafe()->getElementType();
} while (type->isArrayType());

if (isRWAppendConsumeSBuffer(type)) {
if (decl->getType()->isArrayType() &&
isRWAppendConsumeSBuffer(decl->getType())) {
if (!spirvOptions.allowRWStructuredBufferArrays) {
emitError("arrays of RW/append/consume structured buffers unsupported",
loc);
return;
} else if (decl->getType()
->getAsArrayTypeUnsafe()
->getElementType()
->isArrayType()) {
// See
// https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#interfaces-resources-setandbinding
emitError("Multi-dimensional arrays of RW/append/consume structured "
"buffers are unsupported in Vulkan",
loc);
return;
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// RUN: %dxc -T ps_6_6 -E main -fvk-allow-rwstructuredbuffer-arrays

struct PSInput
{
uint idx : COLOR;
};

RWStructuredBuffer<uint> g_rwbuffer[2][3] : register(u0, space2);

float4 main(PSInput input) : SV_TARGET
{
g_rwbuffer[1][input.idx].IncrementCounter();
return g_rwbuffer[input.idx][input.idx][0];
}

// CHECK: :8:26: error: Multi-dimensional arrays of RW/append/consume structured buffers are unsupported in Vulkan
// CHECK: RWStructuredBuffer<uint> g_rwbuffer[2][3] : register(u0, space2);
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// RUN: %dxc -T ps_6_6 -E main -fvk-allow-rwstructuredbuffer-arrays

struct PSInput
{
uint idx : COLOR;
};

// CHECK: OpDecorate %g_rwbuffer DescriptorSet 2
// CHECK: OpDecorate %g_rwbuffer Binding 0
// CHECK: OpDecorate %counter_var_g_rwbuffer DescriptorSet 2
// CHECK: OpDecorate %counter_var_g_rwbuffer Binding 1

// CHECK: %g_rwbuffer = OpVariable %_ptr_Uniform__runtimearr_type_RWStructuredBuffer_uint Uniform
// CHECK: %counter_var_g_rwbuffer = OpVariable %_ptr_Uniform__runtimearr_type_ACSBuffer_counter Uniform
RWStructuredBuffer<uint> g_rwbuffer[] : register(u0, space2);

float4 main(PSInput input) : SV_TARGET
{
// Correctly increment the counter.
// CHECK: [[ac1:%\w+]] = OpAccessChain %_ptr_Uniform_type_ACSBuffer_counter %counter_var_g_rwbuffer {{%\d+}}
// CHECK: [[ac2:%\w+]] = OpAccessChain %_ptr_Uniform_int [[ac1]] %uint_0
// CHECK: OpAtomicIAdd %int [[ac2]] %uint_1 %uint_0 %int_1
g_rwbuffer[input.idx].IncrementCounter();

// Correctly access the buffer.
// CHECK: [[ac1:%\w+]] = OpAccessChain %_ptr_Uniform_type_RWStructuredBuffer_uint %g_rwbuffer {{%\d+}}
// CHECK: [[ac2:%\w+]] = OpAccessChain %_ptr_Uniform_uint [[ac1]] %int_0 %uint_0
// CHECK: OpLoad %uint [[ac2]]
return g_rwbuffer[input.idx][0];
}
7 changes: 7 additions & 0 deletions tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,13 @@ TEST_F(FileTest, RWStructuredBufferArrayCounterIndirect2) {
TEST_F(FileTest, RWStructuredBufferArrayBindAttributes) {
runFileTest("type.rwstructured-buffer.array.binding.attributes.hlsl");
}
TEST_F(FileTest, RWStructuredBufferUnboundedArray) {
runFileTest("type.rwstructured-buffer.array.unbounded.counter.hlsl");
}
TEST_F(FileTest, RWStructuredBufferMultiDimentionalArray) {
runFileTest("type.rwstructured-buffer.array.multidim.counter.error.hlsl",
Expect::Failure);
}
TEST_F(FileTest, AppendStructuredBufferArrayError) {
runFileTest("type.append-structured-buffer.array.hlsl");
}
Expand Down

0 comments on commit 1d6b562

Please sign in to comment.