KhronosGroup / SPIRV-Cross

SPIRV-Cross is a practical tool and library for performing reflection on SPIR-V and disassembling SPIR-V back to high level languages.
Apache License 2.0
2.01k stars 556 forks source link

Support for generating (RW)StructuredBuffers #600

Open mellinoe opened 6 years ago

mellinoe commented 6 years ago

SPIR-V compiled from GLSL like the following:

layout(std140, set = 1, binding = 0) readonly buffer ExampleData
{
    vec4 Data[];
};

will produce HLSL using ByteAddressBuffer (or RWByteAddressBuffer if it's mutable).

ByteAddressBuffer _196 : register(t0);

Is there any support for generating (RW)StructuredBuffer? For cases like the above, where the SSBO is just a simple, single-type array, it seems like a StructuredBuffer could be used. The graphics library I am intending to use SPIRV-Cross with does not (at the moment) support raw buffers in the D3D11 backend, and I would like to avoid supporting that.

HansKristian-Work commented 6 years ago

So, ByteAddressBuffer vs StructuredBuffer is actually a different buffer type in DX? Wow :| Picking StructuredBuffer vs ByteAddressBuffer at whim sounds like it would be even more painful for an application to deal with.

The only workable solution to this I think would be to have an option which forces structured buffers for specific buffers, and will fail to compile if it encounters anything which does not conform to that. Atomics might also get funky.

Adding support for this in SPIRV-Cross and maintaining it indefinitely sounds like a far bigger chunk of work than just adding support for ByteAddressBuffer to a backend.

mellinoe commented 6 years ago

So, ByteAddressBuffer vs StructuredBuffer is actually a different buffer type in DX? Wow :|

Yeah, it's rather annoying. On top of that, it's not possible to create a buffer that is both "structured" and "raw":

You can't combine the D3D11_RESOURCE_MISC_BUFFER_ALLOW_RAW_VIEWS flag with D3D11_RESOURCE_MISC_BUFFER_STRUCTURED.

https://msdn.microsoft.com/en-us/library/windows/desktop/ff476900(v=vs.85).aspx#Raw_Buffer_Views

It wouldn't be very hard to support raw buffers in my graphics library, but I'm trying to avoid adding in concepts that are unnecessarily confusing or D3D-specific.

The only workable solution to this I think would be to have an option which forces structured buffers for specific buffers, and will fail to compile if it encounters anything which does not conform to that.

That was exactly the sort of thing I was imagining. I would be fine getting an error saying "Resource A cannot be compiled into a StructuredBuffer because XYZ". I'm already going through and processing all of the resources, so it's easy for me to just mark them with an extra flag.

HansKristian-Work commented 6 years ago

Right, but you still have to plumb through if a buffer is UAV capable at some point, so picking STRUCTURED or RAW_VIEWS shouldn't be that much different?

It seems to me like a very specific workaround for a very specific case, so I'm not convinced this should be in SPIRV-Cross.

mellinoe commented 6 years ago

Right, but you still have to plumb through if a buffer is UAV capable at some point, so picking STRUCTURED or RAW_VIEWS shouldn't be that much different?

Yeah, it wouldn't be hard to allow that. Users would just have to choose between Structured or Raw buffers up-front. I'm just trying to avoid muddying the waters by introducing a D3D11-specific concept with no analog in other API's, and trying to keep things simpler in general. It's certainly debatable whether that decision is worth it, though.

HansKristian-Work commented 6 years ago

Right, but if you control which shaders go in (i.e. SPIR-V), you know you will only see ByteAddressBuffer, so Structured should not be needed, or?

mellinoe commented 6 years ago

I don't have a tight coupling with SPIRV-Cross, so it's hard to make that assumption, although it would make this part simpler (if more restrictive). Users are free to produce the HLSL bytecode however they want -- directly from HLSL with FXC, through some other shader translation framework, etc.

HansKristian-Work commented 6 years ago

Do you need complex types in that structured buffer, e.g. a struct or matrix? Restricting the structured buffer hack to these types would make it easier:

For structs, I would need to add a whole new buffer alignment validation scheme for StructuredBuffer which I'm not going to do (a lot of work and there is no spec other than trying and failing with FXC for hours). Matrices also have the fun time with row-major vs. column-major. vec3 types have 12 byte stride for some reason, so that's not going to work.

mellinoe commented 6 years ago

I think it would need to include custom structs to be worth including -- it would be too limited otherwise.

I would need to add a whole new buffer alignment validation scheme for StructuredBuffer

What sort of validation are you referring to?

HansKristian-Work commented 6 years ago

The byte layout of a struct in a structured buffer will need to exactly match the layout which is defined in the SPIR-V. This needs to be validated by SPIRV-Cross or subtly wrong HLSL code can be generated. ByteAddressBuffer gets around this by using the offsets from the SPIR-V directly.

mellinoe commented 6 years ago

I see; that is indeed important. Doesn't similar validation need to be done for things like uniform buffers containing structs?

HansKristian-Work commented 6 years ago

Yes, and there is validation for it, but the validation rules are a bit simpler for cbuffer. I don't know where the StructuredBuffer rules are found, if any exists. I have seen some cases where it diverges significantly from cbuffer.

mellinoe commented 6 years ago

Interesting, do you have some examples? I had thought the alignment/size requirements for StructuredBuffers were pretty lenient compared to SSBO's. This does seem like an area where a lot of subtle, hard to track issues could pop up, though, so it makes sense to be very careful.

HansKristian-Work commented 6 years ago

One example I think is float3, which would have 12 byte stride (?!), but SPIR-V will most certainly specify 16 bytes. I don't know of a way to force 16 bytes stride here, so that wouldn't work for example.

Basically, to use StructuredBuffer, the rules FXC apply must exactly match with the SPIR-V. If not, we're out of luck, and we have to use ByteAddressBuffer. It can get really shaky to make sure whatever input you get can always map cleanly to a StructuredBuffer. This assumption will only hold for trivial cases where there is no question about how to pack data.

mellinoe commented 6 years ago

I think that (u/i)vec3 are the only types that have these unusual alignment requirements in GLSL. At least, those are the only types that I've special-cased when I did similar validation in the past. When I did that, though, I validated uniform buffers in the same way. Wouldn't this analysis need to be applied if your uniform buffer contained, for example, an array of custom structs?

HansKristian-Work commented 6 years ago

Yes, structs and arrays in cbuffer are indeed validated (cbuffer aligns to float4 in general, similar to std140), so the logical leap to validate for StructuredBuffer as well isn't that large. See: https://github.com/KhronosGroup/SPIRV-Cross/blob/master/spirv_glsl.cpp#L1103