search

llvm / wg-hlsl

HLSL Working Group documentation and task tracking.
Other
11 stars 12 forks source link

[0006] Propose the set of resource attributes that are needed to describe texture resources #42

Closed bob80905 closed 2 weeks ago

bob80905 commented 3 months ago

This PR proposes a set of attributes that will replace HLSLResourceAttr. This set will be sufficient to distinguish between any texture type, and a table is proposed below that defines which attributes should be present and what values the attributes should have for each texture kind. Certain buffers are also added for extra clarity.

damyanp commented 3 months ago

FWIW, I think we've gone too far in the "make the proposal shorter" direction :) It'd be good to get this filled out with more of the proposal stuff. Maybe you were planning this already, it is a draft after all.

A couple of examples of how and where we expect the attributes to appear would be good as well.

bob80905 commented 3 months ago

Before we can accept this I would like to see some indication of how these attributes will be used.

Take, for example, hlsl::texture_dimension - what will this be used for?

If one of the uses of it is to determine things about the operator[]'s that are available, how will this work for buffers? Are they a special case?

A bit of me suspects that it might make sense to make the texture_dimension thing be more of a resource_dimension - and so buffers will be 1D resources, like 1D textures are. 

Note though: the change I'm requesting here is to fix the entry for the buffers in the table. I think fixing that would be enough to merge this PR, but we'll need some follow up work in order to get the proposal ready to be accepted.

The texture dimension preserves the dimension, and may be used in the future to distinguish between the right load/store overloads to use for the handle. Though this use isn't currently set in stone, it was briefly proposed as an idea here: https://github.com/llvm/llvm-project/pull/90553/commits/4be43928206617ad2b9387fbd207758a4cd24de7#diff-5d235a31f5df8cd876e698f807bb6042678a30a6e1ea6f5649e879a8914504edR167

@bogner will solidify the handle creation design, but has stated before that it's important to capture all information that could be useful in constructing the handle, and dimension seems to be useful.

One advantage of keeping this attribute around, however, is to use it to distinguish between textures and buffers. If we assume this dimension attribute won't exist on buffers, we can use that information to infer the resource kind.

damyanp commented 3 months ago

I'm not sure that creating a bunch of attributes to store things that we think might be useful really counts as designing a feature. How can we evaluate the quality of the design of the attribute if we don't know what problem it is solving?

For example, how will the implementation of GetDimensions() make use of these attributes? How about the various Atomic* operations?

Will these need to have to special case something for buffers, when it could be something that falls out naturally from the definition of them? If they are special cases, what attribute does it look at to figure this out?

Maybe it won't, but at the moment this proposal tells us nothing about that.

llvm-beanz commented 2 weeks ago

Closing as subsumed by #68