Add resource chapter documenting buffers and bindings #344

pow2clk · 2024-10-22T16:48:31Z

This adds a resource chapter that documents typed and raw buffers including their methods and compatible operators along with short descriptions of their functionality. It also adds a description of binding annotations for all types. Stub sections are included for constant buffers and samplers, but no information is provided as yet.

Fixes llvm/wg-hlsl#55
Fixes llvm/wg-hlsl#56

This adds a resource chapter that documents typed and raw buffers including their methods and compatible operators along with short descriptions of their functionality. It also adds a description of binding annotations for all types. Stub sections are included for constant buffers and samplers, but no information is provided as yet. Fixes llvm/wg-hlsl#55 Fixes llvm/wg-hlsl#56

damyanp

Some initial thoughts.

specs/language/resources.tex

Strip mention of UAVs and SRVs Remove some legacy text mistakenly left in as notes, mischaracterization of tiled memory, leftover sentences from moved or deleted segments, and elements of code examples. Better summarize typed buffers upfront. Add description of UDT register annotations.

llvm-beanz · 2024-10-30T15:46:45Z

specs/language/expressions.tex

-\texttt{T[]}, or an object of type \texttt{T} where \texttt{T} provides an
+the form \texttt{E1[E2]}, \texttt{E1} must either be a variable of array,
+vector, matrix, typed buffer (\ref{Resources.tybuf}), or structured buffer
+(\ref{Resources.stbufs}), with elements of type \texttt{T[]} or an


Resource types have overloads for operator[] even in DXC. The original language did miss vector and matrix types, but the addition of resources is redundant.

llvm-beanz · 2024-10-30T15:48:13Z

specs/language/resources.tex

+   // element access
+   T operator[](uint pos);
+   T Load(in int pos);
+   T Load(in int pos, out uint status);


Some (maybe all) of these methods should probably be const.

llvm-beanz · 2024-10-30T15:49:14Z

specs/language/resources.tex

+\begin{HLSL}
+template <typename T = float4>
+ class Buffer {
+   Buffer();


We also need a copy constructor.

Should the copy constructors take references?

Yeah, I was hesitant to suggest references since we don't make them visible to the users, but I suppose that's the way the are in C++ and that's the metaphor we're using here even in cases where it doesn't reflect the actual implementation.

We use references elsewhere and when we do add references to the language I think it'll mean that this documentation just becomes correct.

I think that the copy constructors should take const references though.

specs/language/resources.tex

pow2clk

Responded to Damyan's comments

specs/language/resources.tex

consolidate typed, byte-address, and structured buffers into common sections sharing documentation of their common elements and using inheritance and variant-specific sections to describe differences. Make const all methods that should be Add copy constructors to all buffer types. Revise expressions changes.

pow2clk

Respond to @llvm-beanz comments. The extend of the changes merging many of the buffer descriptions made me unable to respond to some of the comments directly. I used the class inheritance metaphor to represent the relation between some buffer types.

pow2clk · 2024-10-31T22:51:58Z

specs/language/resources.tex

+\begin{HLSL}
+template <typename T = float4>
+ class Buffer {
+   Buffer();


pow2clk · 2024-10-31T22:52:39Z

specs/language/resources.tex

+   // element access
+   T operator[](uint pos);
+   T Load(in int pos);
+   T Load(in int pos, out uint status);


damyanp

I haven't fully read this yet, but I had a look through the changes that were directly in response to my previous feedback (thanks!)

specs/language/resources.tex

In response to Damyan's comments Corrected some mistaken syntax in buffer declarations. Revise the description of CheckAccessFullyMapped, giving it its own section that other parts refer to. correct sign of parameters to the subscript operator

pow2clk · 2024-11-04T21:12:14Z

specs/language/resources.tex

+   T operator[](int pos) const;
+   T Load(in int pos) const;
+   T Load(in int pos, out uint status) const;


Yup, I was suggesting that we should change the spec here, only call out that this somewhat surprising definition is actually what is intended and isn't a mistake.

Somehow I was unable to respond to this comment from @damyanp about similar lines of text directly. Probably a consequence of the consolidation I did.

I suspect perhaps the intent was to say "I wasn't suggesting that we should change the spec here"? That seems more consistent with the rest of the sentence. I could be persuaded either way, but I've brought all subscript ops and loads into line with a signed integer parameter for now.

Oops, yes, your interpretation is correct.

@bogner - do the parameter types in this spec match what you've/'re implementing?

damyanp · 2024-11-07T00:50:24Z

specs/language/resources.tex

+\begin{HLSL}
+template <typename T = float4>
+ class Buffer {
+   Buffer();


Should the copy constructors take references?

damyanp · 2024-11-07T00:51:26Z

specs/language/resources.tex

+   T operator[](int pos) const;
+   T Load(in int pos) const;
+   T Load(in int pos, out uint status) const;


Oops, yes, your interpretation is correct.

@bogner - do the parameter types in this spec match what you've/'re implementing?

specs/language/resources.tex

damyanp · 2024-11-07T00:59:51Z

specs/language/resources.tex

+When defined at local scope, ByteAccessBuffers represent local references
+that can be associated with global ByteAccessBuffers when assigned,
+but must be resolvable to a unique global buffer.


must be resolvable to a unique global buffer

Just checking this is the case. So I couldn't do something like:

ByteAddressBuffer myBuf = globalBuffersArray[threadId];

Resource arrays are different. If you can unambiguously map to the same resource array, that's fine. It's if you cross over into a separate resource entirely that you have problems:

https://godbolt.org/z/9Whzssxr1

ByteAddressBuffer globalBuffersArray[10]; ByteAddressBuffer globalBuffer; RWBuffer<float4> outbuf; [numthreads(8,1,1)] void main(uint threadId: SV_GroupThreadID) { ByteAddressBuffer myBuf; myBuf = globalBuffersArray[threadId]; // This is fine if (threadId > 10) myBuf = globalBuffer; // This is not outbuf[threadId] = myBuf.Load<float4>(0); }

I'll try to come up with some wording that makes this clear and maybe add a note for the specific case

specs/language/resources.tex

damyanp · 2024-11-07T01:55:17Z

specs/language/resources.tex

+  RWBuffer<float> herbuf : register(u0, space1);
+\end{HLSL}
+
+Resource aggregates structs containing multiple resources types or arrays of resource types or structs containing one or more resource types.


Resource aggregates structs containing multiple resources types or arrays of resource types or structs containing one or more resource types.

I might be tired, but I'm having a hard time parsing this one.

I refactored it into a self-referential list of what constitutes an aggregate resource type. I think it's much clearer now.

Corrected a few parameter names and types This led to significant rewording of the atomic operation descriptions. Clarified unique global resource mapping. Refactored description of aggregate resource types for purposes of binding

pow2clk

Thanks Damyan! Responses in relation to the latest commit.

pow2clk · 2024-11-07T19:27:05Z

specs/language/resources.tex

+\begin{HLSL}
+template <typename T = float4>
+ class Buffer {
+   Buffer();


Yeah, I was hesitant to suggest references since we don't make them visible to the users, but I suppose that's the way the are in C++ and that's the metaphor we're using here even in cases where it doesn't reflect the actual implementation.

specs/language/resources.tex

pow2clk · 2024-11-07T19:32:16Z

specs/language/resources.tex

+When defined at local scope, ByteAccessBuffers represent local references
+that can be associated with global ByteAccessBuffers when assigned,
+but must be resolvable to a unique global buffer.


Resource arrays are different. If you can unambiguously map to the same resource array, that's fine. It's if you cross over into a separate resource entirely that you have problems:

https://godbolt.org/z/9Whzssxr1

ByteAddressBuffer globalBuffersArray[10]; ByteAddressBuffer globalBuffer; RWBuffer<float4> outbuf; [numthreads(8,1,1)] void main(uint threadId: SV_GroupThreadID) { ByteAddressBuffer myBuf; myBuf = globalBuffersArray[threadId]; // This is fine if (threadId > 10) myBuf = globalBuffer; // This is not outbuf[threadId] = myBuf.Load<float4>(0); }

I'll try to come up with some wording that makes this clear and maybe add a note for the specific case

specs/language/resources.tex

pow2clk · 2024-11-07T19:34:14Z

specs/language/resources.tex

+  RWBuffer<float> herbuf : register(u0, space1);
+\end{HLSL}
+
+Resource aggregates structs containing multiple resources types or arrays of resource types or structs containing one or more resource types.


I refactored it into a self-referential list of what constitutes an aggregate resource type. I think it's much clearer now.

damyanp

LGTM - a few nits in the comments, but I think this is pretty much ready to go in.

damyanp · 2024-11-22T19:31:34Z

specs/language/resources.tex

+  RWBuffer<float> herbuf : register(u0, space1);
+\end{HLSL}
+
+A aggregate resource type can be:


Suggested change

A aggregate resource type can be:

An aggregate resource type can be:

damyanp · 2024-11-22T19:32:52Z

specs/language/resources.tex

+\end{itemize}
+
+When aggregate resource types have register annotations,
+they occupy the first register they specify as well as however many additional sequential registers


Suggested change

they occupy the first register they specify as well as however many additional sequential registers

they occupy the first register they specify, as well as however many additional sequential registers

damyanp · 2024-11-22T19:34:32Z

specs/language/resources.tex

+
+\begin{note}
+Resource types contained in structs are only allocated registers when they are explicitly used.
+This includes elements of arrays of resources as such array elements must be indexed with literals.


Suggested change

This includes elements of arrays of resources as such array elements must be indexed with literals.

This includes elements of arrays of resources, as such array elements must be indexed with literals.

damyanp · 2024-11-22T19:37:19Z

specs/language/resources.tex

+\begin{HLSL}
+template <typename T = float4>
+ class Buffer {
+   Buffer();


We use references elsewhere and when we do add references to the language I think it'll mean that this documentation just becomes correct.

I think that the copy constructors should take const references though.

damyanp requested a review from llvm-beanz October 22, 2024 16:56

damyanp reviewed Oct 22, 2024

View reviewed changes

llvm-beanz reviewed Oct 30, 2024

View reviewed changes

pow2clk commented Oct 30, 2024

View reviewed changes

specs/language/resources.tex Outdated Show resolved Hide resolved

specs/language/resources.tex Outdated Show resolved Hide resolved

specs/language/resources.tex Outdated Show resolved Hide resolved

specs/language/resources.tex Show resolved Hide resolved

pow2clk commented Oct 31, 2024

View reviewed changes

damyanp reviewed Nov 1, 2024

View reviewed changes

specs/language/resources.tex Outdated Show resolved Hide resolved

specs/language/resources.tex Outdated Show resolved Hide resolved

specs/language/resources.tex Outdated Show resolved Hide resolved

pow2clk added 2 commits November 1, 2024 16:14

Make headings more consistent, add binding section refs

d5604d8

pow2clk commented Nov 4, 2024

View reviewed changes

damyanp reviewed Nov 7, 2024

View reviewed changes

Respond to Damyan's feedback

93bbd1c

Corrected a few parameter names and types This led to significant rewording of the atomic operation descriptions. Clarified unique global resource mapping. Refactored description of aggregate resource types for purposes of binding

pow2clk commented Nov 7, 2024

View reviewed changes

llvm-beanz added the Design Meeting Agenda item for the design meeting label Nov 12, 2024

damyanp approved these changes Nov 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add resource chapter documenting buffers and bindings #344

Add resource chapter documenting buffers and bindings #344

pow2clk commented Oct 22, 2024

damyanp left a comment

llvm-beanz Oct 30, 2024

llvm-beanz Oct 30, 2024

pow2clk Oct 31, 2024

llvm-beanz Oct 30, 2024

pow2clk Oct 31, 2024

damyanp Nov 7, 2024

pow2clk Nov 7, 2024

damyanp Nov 22, 2024

pow2clk left a comment

pow2clk left a comment

pow2clk Oct 31, 2024

pow2clk Oct 31, 2024

damyanp left a comment

pow2clk Nov 4, 2024

damyanp Nov 7, 2024

damyanp Nov 7, 2024

damyanp Nov 7, 2024

damyanp Nov 7, 2024

pow2clk Nov 7, 2024

damyanp Nov 7, 2024

pow2clk Nov 7, 2024

pow2clk left a comment

pow2clk Nov 7, 2024

pow2clk Nov 7, 2024

pow2clk Nov 7, 2024

damyanp left a comment

damyanp Nov 22, 2024

damyanp Nov 22, 2024

damyanp Nov 22, 2024

damyanp Nov 22, 2024

	A aggregate resource type can be:
	An aggregate resource type can be:

	they occupy the first register they specify as well as however many additional sequential registers
	they occupy the first register they specify, as well as however many additional sequential registers

	This includes elements of arrays of resources as such array elements must be indexed with literals.
	This includes elements of arrays of resources, as such array elements must be indexed with literals.

Add resource chapter documenting buffers and bindings #344

Are you sure you want to change the base?

Add resource chapter documenting buffers and bindings #344

Conversation

pow2clk commented Oct 22, 2024

damyanp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pow2clk left a comment

Choose a reason for hiding this comment

pow2clk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damyanp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pow2clk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damyanp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment