DirectXShaderCompiler
DirectXShaderCompiler copied to clipboard
microsoft
[SPIR-V] arrays of RW/append/consume structured buffers unsupported
When targetting SPIR-V the following code generates a compile error:
struct PSInput
{
float4 color : COLOR;
};
StructuredBuffer<uint> g_buffer[] : register(t0, space1);
RWStructuredBuffer<uint> g_rwbuffer[] : register(u0, space2);
float4 PSMain(PSInput input) : SV_TARGET
{
return input.color;
}
However, in DXIL mode this is perfectly fine - and it's how you would go about implementing bindless UAVs.
It's a little hard to grep for but the relevant error is from here https://github.com/microsoft/DirectXShaderCompiler/blob/4ced9bdf05ba26c6d9c01258ad8914f892e3620a/tools/clang/lib/SPIRV/SpirvEmitter.cpp#L1357
I suspect the check should just be isAppendStructuredBuffer(type) || isConsumeStructuredBuffer(type) and allow RWStructuredBuffers to pass through.
I think the support for this is not trivial (and it's probably why it was not done initially). The reason is that RWStructuredBuffers, AppendStructuredBuffers, and ConsumeStructuredBuffers have associated counters, and when you have an array of these resources, the bindings become quite complicated. For instance how do you decide what's the proper descriptor set & binding number of the counter of a certain element in the array (does the counter binding of the first element come before the resource binding of the second element?).
I think there might be some work involved in flattening these arrays in spirv-opt as well.
I think the support for this is not trivial (and it's probably why it was not done initially). The reason is that RWStructuredBuffers, AppendStructuredBuffers, and ConsumeStructuredBuffers have associated counters, and when you have an array of these resources, the bindings become quite complicated.
I think RWStructuredBuffers can have counters, but they don't when used as an array like this, though it would be good to get some insight from Microsoft for this.
AppendStructuredBuffer and ConsumeStructuredBuffer definitely would have a counter - but I think they're separate.
Actually - if you look here: http://shader-playground.timjones.io/b3a96b7be434c99a86553a91b4f517ea you'll see that the RWStructuredBuffer array gets reflected as Dim r/w, whereas a ConsumeStructuredBuffer gets reflected as r/w+cnt: http://shader-playground.timjones.io/132922e3b32b3202f782883d9c53aded (at which point it becomes unclear how to layout the descriptors).
Similarly - for RWStructuredBuffer one would only require a UAV counter technically if IncrementCounter or DecrementCounter is called on it. Extending this check above to be based on if those functions are called would already vastly improve the usefulness there.
Hello, has there been any update on this? We just recently hit this issue, and it's preventing us from adopting full bindless for our engine's Vulkan path unless we switch all of these buffers to another type.
Is there any update? I am writing a vulkan abstraction right now and want to use full bindless and hlsl. Not having buffer device adress or arrays of RWStructuredBuffers makes this impossible for dxc with vulkan. This is a big problem for me, as it would force me to rewrite all shaders and drop hlsl entirely.
My suggestion would be to simply not allow calls to Increment and Decrement when RWStructured buffers are bound in an array descriptor, like Jasper suggested. This would be strange syntax wise, but would enable us to use hlsl with vulkan fully bindless.
according to D3D docs, when creating a buffer in D3D and you want to have these counters, you need to have the flag D3D11_BUFFER_UAV_FLAG_COUNTER set, which is optional and obviously is not applicable to Vulkan/SPIR-V. So RW/Append/Consume buffers probably just should NOT have counters when targeting SPIR-V, and so it should not be an issue.
Thanks. @Ipotrick @Jasper-Bekkers it may be a while until we can get to this, sorry. We are pretty resource constrained at the moment :(. I will take a look in the coming days.
@dnovillo Hello! I've been working with Ipotrick on a project, and we're using DXC. I took a look at the source and from my little bit of digging, here's what I found. (disclaimer: I'm not familiar with the codebase at all)
first thing I did was make a simple contrived shader example to test with:
template <typename T> RWStructuredBuffer<T> get_RWStructuredBuffer(uint buffer_id);
template <typename T> StructuredBuffer<T> get_StructuredBuffer(uint buffer_id);
struct Globals {
int x;
void update() { x++; }
};
[[vk::binding(0, 0)]] RWStructuredBuffer<Globals> RWStructuredBufferViewGlobals[];
template <> RWStructuredBuffer<Globals> get_RWStructuredBuffer(uint buffer_id) {
return RWStructuredBufferViewGlobals[buffer_id];
}
[[vk::binding(0, 0)]] StructuredBuffer<Globals> StructuredBufferViewGlobals[];
template <> StructuredBuffer<Globals> get_StructuredBuffer(uint buffer_id) {
return StructuredBufferViewGlobals[buffer_id];
}
[numthreads(1, 1, 1)] void main() {
RWStructuredBuffer<Globals> globals = get_RWStructuredBuffer<Globals>(0);
globals[0].update();
}
and so I'd build this HLSL with the following parameters:
dxc ./test.hlsl -spirv -fspv-target-env="vulkan1.1" -E main -T cs_6_6 -HV 2021
and then I got DXC building in VisualStudio and started playing around. I figured out that if I comment out this if block in spirvemitter.cpp as well as this one, the error prohibiting using a bindless pattern as in this example code goes away, and it appears to successfully generate the correct SPIR-V code.
The only difference between the resulting SPIR-V would be the removal of a single line that OpMemberDecorates the Globals StructuredBuffer as NonWritable
OpMemberDecorate %type_StructuredBuffer_Globals 0 NonWritable
which I would (though I have limited knowledge of SPIR-V) assume was just to indicate the StructuredBuffer is readonly. And since the RWSB is not, it removes this line.
Obviously just a starting off point, but would love to see this issue fixed. In the previous version of DXC, it seemed to actually just allow us to call member functions on the "read only" StructuredBuffer that in-turn modified the state, though yesterday I updated the version we were using and it broke this, which is why we're here now. I went back to the December 2021 release for now, but thanks for any effort you put into this!
I have pushed a commit which fixes the related issue #4569.
My code defers creating the counter for a single RWStructuredBuffer until Inc/DecrementCounter methods are invoked. If the methods are not invoked, the counter is not created.
I have not tested if my code fixes this issue. I suspect it does not completly, but I suspect the solution for this issue can build on my code. I am imagining that the solution here could be to allow arrays of RWStructuredBuffer as long as no Inc/DecrementCounter is called. While not perfect/complete, it seems like it would enable everyone on this issue.
Indeed it does not fix the array descriptors for RWStructuredBuffer completely. Thank you very much for the effort, this is probably allready a big step in fixing this issue.
I've opened a PR that attempts to address this issue (#4663). I'd appreciate any feedback you all might have.
@cassiebeckley Sorry to bother you, is there any progress here? I recently hit this issue too.
With will be handled under #5440. I want all of the issues in a central place with a fresh conversation.
