Skip to content

Add support for resource arrays to the offload test suite#302

Merged
hekota merged 14 commits into
llvm:mainfrom
hekota:resource-arrays-support
Jul 31, 2025
Merged

Add support for resource arrays to the offload test suite#302
hekota merged 14 commits into
llvm:mainfrom
hekota:resource-arrays-support

Conversation

@hekota

@hekota hekota commented Jul 28, 2025

Copy link
Copy Markdown
Member

Introduces support for resource arrays across the offload test suite, including:

  • Data input and output handling
  • Comparison of expected results
  • Integration with both DirectX and Vulkan runtimes (Metal is not supported yet)

A new property ArraySize has been added to the Buffer definition. If ArraySize is 1 (the default), the Data field of the Buffer remains unchanged:

Data: [0, 1, 2, 3]

If ArraySize is greater than 1, the Data field must be specified as a list of arrays:

Data:
  - [0, 1, 2, 3]
  - [4, 5, 6, 7]
  - [8, 9, 10, 11]

If the resource array includes resources with counters, their values will be output using the Counters field:

Counters: [1, 2, 3]

Closes llvm/wg-hlsl#291

hekota added 6 commits July 28, 2025 16:41
Introduces support for resource arrays across the offload test suite, including:
- Data input and output handling
- Comparison of expected results
- Integration with both DirectX and Vulkan runtimes
- Metal runtime is not yet supported

A new property `ArraySize` has been added to the Buffer definition.

If ArraySize is 1 (the default), the Data field remains unchanged:

```
Data: [0, 1, 2, 3]
```

If `ArraySize` is greater than `1`, the `Data` field must be specified as a list of arrays:
```
   Data:
     - [0, 1, 2, 3]
     - [4, 5, 6, 7]
     - [8, 9, 10, 11]
```

If the resource array includes resources with counters, their values will be output using the Counters field:
```
Counters: [1, 2, 3]
```
- update Vulkan code to support to arrays again, including arrays of textures
- update code that converts buffer to text to support arrays
- add tests to cover the new additions

@llvm-beanz llvm-beanz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question I have from a design perspective is should the array-ness be part of the buffer, or part of the resource.

This PR makes them part of the buffer, my inclination had been to make it part of the resource. The reason I had thought to attach it to the resource is to allow an array to be comprised of buffers of different sizes or underlying data types. Curious for thoughts on whether or not that is important to support.

Comment thread lib/API/VK/Device.cpp Outdated
Comment thread lib/Support/Pipeline.cpp
Comment thread test/Feature/ResourceArrays/array-global.test
Comment thread test/Feature/StructuredBuffer/inc_counter_array.test
Comment thread test/Feature/Textures/SRVToUAV-array.test
@hekota

hekota commented Jul 29, 2025

Copy link
Copy Markdown
Member Author

One question I have from a design perspective is should the array-ness be part of the buffer, or part of the resource.

This PR makes them part of the buffer, my inclination had been to make it part of the resource. The reason I had thought to attach it to the resource is to allow an array to be comprised of buffers of different sizes or underlying data types. Curious for thoughts on whether or not that is important to support.

I considered making the array-nest part of the Resource, but since all elements in a resource array must share the same type, stride, and other characteristics, it seemed more appropriate to place the array-nesting on the Buffer. Moving it to the resource level would make the buffer definitions repetitive and require additional validation to ensure consistency across elements.
That said, I acknowledge it’s a bit odd to have a Buffer definition that effectively encapsulates multiple buffers.

Currently, we don’t support varying sizes for individual resources in an array, but that could be added if needed.

@llvm-beanz

Copy link
Copy Markdown
Collaborator

I considered making the array-nest part of the Resource, but since all elements in a resource array must share the same type, stride, and other characteristics, it seemed more appropriate to place the array-nesting on the Buffer. Moving it to the resource level would make the buffer definitions repetitive and require additional validation to ensure consistency across elements.

This makes sense to me. I'm glad I wasn't alone in thinking the other way around.

@llvm-beanz llvm-beanz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments, but this mostly looks good to me.

Comment thread test/Feature/StructuredBuffer/inc_counter_array.test Outdated
Comment thread test/Feature/StructuredBuffer/inc_counter_array.test Outdated
@@ -86,6 +88,10 @@ DescriptorSets:
#--- end

# UNSUPPORTED: Clang

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spall Did we decide on using XFAIL for things that aren't yet supported but are planned?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we were talking about introducing UNIMPLEMENTED: but it had not been done yet. I'll at least change the UNSUPPORTEDs in this change to XFAILs.

@hekota hekota merged commit 6b18c2d into llvm:main Jul 31, 2025
9 checks passed
hekota added a commit that referenced this pull request Aug 4, 2025
Add tests for arrays of resources to the offload test suite, including
arrays declared locally or used as function arguments, multi-dimensional
arrays, or subsets of multi-dimensional arrays. One test for global
arrays was already added in #302 when support for resource arrays was
added to the offload test suite.

Closes llvm/wg-hlsl#292
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for resources arrays to offload test suite

4 participants