-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposal for mapping resource attributes to DXIL and SPIR-V #76
base: main
Are you sure you want to change the base?
Conversation
57e9383
to
c412b60
Compare
c412b60
to
d0b6a34
Compare
| | Dim | Depth | Arrayed | MS | Sampled | Access | | ||
|---------------------------------|--------|-------------|---------|-------|---------|--------| | ||
| Texture1D | 1D | Unknown (2) | false | false | r/o (1) | r/o | | ||
| Texture1DArray | 1D | Unknown (2) | true | false | r/o (1) | r/o | | ||
| Texture2D | 2D | Unknown (2) | false | false | r/o (1) | r/o | | ||
| Texture2DArray | 2D | Unknown (2) | true | false | r/o (1) | r/o | | ||
| Texture2DMS | 2D | Unknown (2) | false | true | r/o (1) | r/o | | ||
| Texture2DMSArray | 2D | Unknown (2) | true | true | r/o (1) | r/o | | ||
| Texture3D | 3D | Unknown (2) | false | false | r/o (1) | r/o | | ||
| TextureCUBE | Cube | Unknown (2) | false | false | r/o (1) | r/o | | ||
| TextureCUBEArray | Cube | Unknown (2) | true | false | r/o (1) | r/o | | ||
| RWTexture1D | 1D | Unknown (2) | false | false | r/w (2) | r/w | | ||
| RWTexture1DArray | 1D | Unknown (2) | true | false | r/w (2) | r/w | | ||
| RWTexture2D | 2D | Unknown (2) | false | false | r/w (2) | r/w | | ||
| RWTexture2DArray | 2D | Unknown (2) | true | false | r/w (2) | r/w | | ||
| RWTexture2DMS | 2D | Unknown (2) | false | true | r/w (2) | r/w | | ||
| RWTexture2DMSArray | 2D | Unknown (2) | true | true | r/w (2) | r/w | | ||
| RWTexture3D | 3D | Unknown (2) | false | false | r/w (2) | r/w | | ||
| Buffer | Buffer | Unknown (2) | false | false | r/o (1) | r/o | | ||
| RWBuffer | Buffer | Unknown (2) | false | false | r/w (2) | r/w | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Access Qualifier is for Kernel (think OpenCL) only. The value after "Sampled" is the Image Format.
DXC lowers [Constant/Texture/Structured/Byte Buffers] to [OpTypeStruct] in | ||
SPIR-V with a uniform storage class and various layout decorations. DXC tracks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The storage class should depend on the version of Vulkan that is targeted. Starting with Vulkan 1.2 the storage class is StorageBuffer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add a section discussing the "Image format" for spir-v. This has been discussed a lot in DXC over the years. The Image format indicates the format of the underlying data. I don't know what the expectations are for DXIL, but many Vulkan users want flexibility. The most flexibility is to use Unknown
as much as possible bey default. However for Vulkan we cannot always use Unknown
.
There are two feature bits that we care about: shaderStorageImageReadWithoutFormat and shaderStorageImageWriteWithoutFormat. I'm trying to check how available these feature are. However, many devices are still running Vulkan 1.1, and probably do not support it.
DXC guesses at the image format based on the sampled type, and developers can use the vk::image_format
attribute to set it to something else. We should document what we will be doing.
Vulkan documentation:
https://docs.vulkan.org/spec/latest/appendices/spirvenv.html#spirvenv-format-type-matching
https://docs.vulkan.org/spec/latest/appendices/spirvenv.html#spirvenv-image-formats
DXC Issues:
microsoft/DirectXShaderCompiler#4941
microsoft/DirectXShaderCompiler#4773
microsoft/DirectXShaderCompiler#2498
microsoft/DirectXShaderCompiler#3395
microsoft/DirectXShaderCompiler#4868
The "RasterizerOrdered" (or ROV) resources in HLSL don't map directly to | ||
SPIR-V. In DXC, we emulate ROV by treating these the same as their non-ROV | ||
counterparts but wrapping accesses to the resource in critical sections using | ||
[SPV_EXT_fragment_shader_interlock]. We will probably need to extend the SPIR-V | ||
target extension types to represent these unless we want to emit the critical | ||
sections awkwardly early in the compiler. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is even harder than just wrapping the accesses in a critical section.
If the entry point has any of the interlock ordering execution modes, it must dynamically execute each of OpBeginInvocationInterlockEXT and OpEndInvocationInterlockEXT, in that program order, exactly once.
So DXC generates a different begin and end instructions around each of the accesses, then a spirv-opt pass finds the small block of code that will mean the requirements above.
At first glance, I agree that we should somehow add to the type that it is an ROV. However, we will need to look in deatail a how we choose between the different executions mode, but all of that feels like a backend thing to me.
| Texture1D | SRV | vec4 | - | 1D | - | - | - | - | - | | ||
| Texture1DArray | SRV | vec4 | - | 1D | - | - | yes | - | - | | ||
| Texture2D | SRV | vec4 | - | 2D | - | - | - | - | - | | ||
| Texture2DArray | SRV | vec4 | - | 2D | - | - | yes | - | - | | ||
| Texture2DMS | SRV | vec4 | - | 2D | yes | - | - | - | - | | ||
| Texture2DMSArray | SRV | vec4 | - | 2D | yes | - | yes | - | - | | ||
| Texture3D | SRV | vec4 | - | 3D | - | - | - | - | - | | ||
| TextureCUBE | SRV | vec4 | - | Cube | - | - | - | - | - | | ||
| TextureCUBEArray | SRV | vec4 | - | Cube | - | - | yes | - | - | | ||
| RWTexture1D | UAV | vec4 | - | 1D | - | - | - | - | - | | ||
| RWTexture1DArray | UAV | vec4 | - | 1D | - | - | yes | - | - | | ||
| RWTexture2D | UAV | vec4 | - | 2D | - | - | - | - | - | | ||
| RWTexture2DArray | UAV | vec4 | - | 2D | - | - | yes | - | - | | ||
| RWTexture2DMS | UAV | vec4 | - | 2D | yes | - | - | - | - | | ||
| RWTexture2DMSArray | UAV | vec4 | - | 2D | yes | - | yes | - | - | | ||
| RWTexture3D | UAV | vec4 | - | 3D | - | - | - | - | - | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These look good. The individual attributes map nicely to individual operands to spirv.Image.
need to keep track of that information in new SPIR-V target extension types in | ||
our implementation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree we will need a new target type.
I've been thinking about this a bit. SPIR-V does not have a "handle" for structured buffers. There is a global variable, which is accessed using the same instructions used to access function scope variables.
In Vulkan SPIR-V, a structured buffer StructuredBuffer<T>
is represented as struct { T[] }
. We could make the handle a pointer to the struct or a pointer to the array. In any case, you want it to be a pointer type.
One option is to have CreateHandleFromBinding
return a pointer. Then use use standard llvm-ir instruction to access it. The problem with that is that we will have to expose the layout of T, and the type information may be hard to recover. That is why I agree we need a target type.
We should be able to define a target type to represent a spir-v pointer.
| ByteAddressBuffer | SRV | - | - | - | - | - | - | yes | - | | ||
| RWByteAddressBuffer | UAV | - | - | - | - | - | - | yes | - | | ||
| RasterizerOrderedByteAddressBuffer | UAV | - | yes | - | - | - | - | yes | - | | ||
| StructuredBuffer | SRV | struct | - | - | - | - | - | yes | - | | ||
| RWStructuredBuffer | UAV | struct | - | - | - | - | - | yes | - | | ||
| RasterizerOrderedStructuredBuffer | UAV | struct | yes | - | - | - | - | yes | - | | ||
| AppendStructuredBuffer | UAV | struct | - | - | - | - | - | yes | - | | ||
| ConsumeStructuredBuffer | UAV | struct | - | - | - | - | - | yes | - | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To generate or use these types, the SPIR-V backend will have to know the layout of any structs. In DXC, we have a few different layout options. I feel like it should be the FE's job to specify the layout. Let me know what you think.
| ByteAddressBuffer | SRV | - | - | - | - | - | - | yes | - | | ||
| RWByteAddressBuffer | UAV | - | - | - | - | - | - | yes | - | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ByteAddressBuffers are hard in SPIR-V. Without untyped pointers, we cannot cast from one pointer type to another. This make it very hard to do some thing, and impossible to do others. For, example, it is impossible to have both 32-bit atomic operations and 64-bit atomic operations.
For now, we cannot have a common implementation for all of the raw buffer resources.
View), CBV (Constant buffer view), or Sampler. The only difference between | ||
SRV and UAV is whether the object is writeable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only difference between SRV and UAV is whether the object is writeable.
The resource class also has implications on the register type that the resource can be bound to. I think I'd expect to see some reference to that (maybe just a link to the other documentation we have about it).
I almost wonder if it would be fair to say that the only reason we track this resource class is to know which register types it belongs to. It's notably absent from any discussion about the SPIR-V implementation, where SPIR-V doesn't distinguish between different types of register.
No description provided.