[SPIR-V] [Proposal 0011] SPIR-V Type for a PhysicalStorageBuffer class OpTypePointer fails to compile because of missing offsets for a struct

[devshgraphicsprogramming]

Description This was already solved for vk::RawBufferLoad<Composite> however it rears its head again when we start using vk::SpirvOpaqueType to declare the same poitner type RawBufferLoad uses under the hood.

Related to this probably: #4924 and #2411

Steps to Reproduce This Godbolt: https://godbolt.org/z/MjdfrGjPz

Actual Behavior Every time an

 vk::SpirvOpaqueType<
        /* OpTypePointer */ 32,
        /* PhysicalStorageBuffer */ vk::Literal<vk::integral_constant<uint,5349> >,
        T
    >;

gets instantiated where T is a composite, we get the following error

fatal error: generated SPIR-V is invalid: Structure id 7 decorated as Block for variable in PhysicalStorageBuffer storage class must follow relaxed storage buffer layout rules: member 0 is missing an Offset decoration

Environment

• DXC version : Latest trunk on Godbolt
• Host Operating System : Linux I guess?

[devshgraphicsprogramming]

Its been over a year since I've checkd, but seems that still, contrary to the docs

struct Test
{
    // The vk::ext_decorate don't seem to work or emit `OpMemberDecorate` at all
    [[vk::ext_decorate(/*Offset*/ 35,0)]] float4 mem1;
    [[vk::ext_decorate(/*Offset*/ 35,16)]] float mem2;
    [[vk::ext_decorate(/*Offset*/ 35,20)]] int mem3;
};

There are no tests for this functionality in the DXC repo at all

https://godbolt.org/z/x7sx4Po9Y

[sudonatalie]

Thanks for reporting @devshgraphicsprogramming. You've piqued my interest with https://github.com/microsoft/DirectXShaderCompiler/issues/6489#issuecomment-2058755134 so we'll try to prioritize this.

[devshgraphicsprogramming]

Thanks for reporting @devshgraphicsprogramming. You've piqued my interest with #6489 (comment) so we'll try to prioritize this.

I can't conform to Proposal 0010 100% but I can get something that has all the functionality.

One reason is that there's no reflection neither in C++ or HLSL so for structs I'd be basically defining them throuhg a macro.

[sudonatalie]

We'll want a dedicated implementation anyways, but I'm still keen to see full features done successfully in inline SPIR-V.

[devshgraphicsprogramming]

The vk::ext_decorate seems really flaky, I'm trying to decorate my pointer variable as AliasedPointer and seems like some SPIR-V Optimization rips it away

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

NVM: got sniped by https://github.com/KhronosGroup/SPIRV-Registry/issues/140#issuecomment-1143104775

[s-perron]

This seems like an issue with the layout rules not getting passed around correctly for the vk::SpirvType. Here is another case that fails. It fails differently, but it probably a related issues: https://godbolt.org/z/3vq3xWY7v.

[s-perron]

I'm starting to understand the problem, and I'll need to think about how to fix it. The problem in the smaller example is that the SPIR-V backend does not know how to propagate the layout rule for the vk::ext_instruction functions.

In general, the layout rule for a return value in a function is Void. When DXC generates the callee, it will make sure the return value has layout rule Void. In this case, the load function is a single OpLoad instruction, which will result in an object of type T that has a different layout rule.

There are a few options to fix this. In the expansion of the load function, DXC could add code to always rebuild the type with the correct layout rule. This is the best option if it can work.

Other option involve fixing up the code after wards, either in DXC or spirv-opt.

[devshgraphicsprogramming]

I'm starting to understand the problem, and I'll need to think about how to fix it. The problem in the smaller example is that the SPIR-V backend does not know how to propagate the layout rule for the vk::ext_instruction functions.

In general, the layout rule for a return value in a function is Void. When DXC generates the callee, it will make sure the return value has layout rule Void. In this case, the load function is a single OpLoad instruction, which will result in an object of type T that has a different layout rule.

There are a few options to fix this. In the expansion of the load function, DXC could add code to always rebuild the type with the correct layout rule. This is the best option if it can work.

Other option involve fixing up the code after wards, either in DXC or spirv-opt.

IIRC what happens is that for a canonical type T DXC on-demand makes T_ubo, T_ssbo depending on how you use them with different SPIR-V decorations.

btw I always compile my code with -fvk-use-scalar-layout since its fairly ubiquitous across still supported GPUs and many people are already in that boat.

Surely Scalar Layout must make this a lot easier?

[s-perron]

As I have thought about this more, DXC does not know the correct layout for the result of a vk::ext_instruction function. It cannot generate code that does to or from the correct type without knowing about the instruction. This is only a problem if the result type is a struct or array, and the instruction is dealing with externally visible memory.

For now, I think I will write a spirv-opt pass that will fix up the result type for an OpLoad and the object type for an OpStore. I suspect that this pass will almost never have to be updated for new instructions given the specific cases it will be needed.

EDIT I looked at the original test case. The problem with that one is the BitCast. The result of the vk::ext_instruction function is the SpirvType, and it get assigned void for the layout. This is not something that spirv-opt can fix because it does not know how to add offset decorations. I don't want to add that to spirv-opt. I'll have to think about this more, and come back to it later.

[devshgraphicsprogramming]

As I have thought about this more, DXC does not know the correct layout for the result of a vk::ext_instruction function. It cannot generate code that does to or from the correct type without knowing about the instruction. This is only a problem if the result type is a struct or array, and the instruction is dealing with externally visible memory.

For now, I think I will write a spirv-opt pass that will fix up the result type for an OpLoad and the object type for an OpStore. I suspect that this pass will almost never have to be updated for new instructions given the specific cases it will be needed.

EDIT I looked at the original test case. The problem with that one is the BitCast. The result of the vk::ext_instruction function is the SpirvType, and it get assigned void for the layout. This is not something that spirv-opt can fix because it does not know how to add offset decorations. I don't want to add that to spirv-opt. I'll have to think about this more, and come back to it later.

These were my thoughts/musings related to this:

• https://github.com/microsoft/DirectXShaderCompiler/issues/6554#issuecomment-2091132462
• https://github.com/microsoft/DirectXShaderCompiler/issues/6578

[s-perron]

We are able to deal with workgroup pointers, and we are adding this to the HLSL standard headers: https://github.com/microsoft/DirectXShaderCompiler/pull/6873.

I am coming to believe that the layout issue with storage classes that require a layout is insurmountable. We might be able to do some targeted fixes for pointer. If we have a SpirvOpqaueType tha that is a pointer, then try to deduce the layout based on the storage class. However, it would still fail for instructions that do a load or store type instruction using the pointers.

There is no way to specify that the result of an vk::ext_instruction function must have a particular layout. We may need to add an attribute to indicate the layout of the result. I am not planning on doing that until it becomes something we need for the HLSL standard headers that we writing.

I will close this issue, and open a new one in HLSL spec when we decide that we need the layout attribute.

[devshgraphicsprogramming]

Wouldn't OpLogicalCopy be enough to copy between "related" structs but with different layouts?

[s-perron]

Opcopylogical only helps a little. It would not translate the data pointed to by a pointer. You would need a load. It also doesn't help with the result type for a vk::ext_instruction function. The result type is invalid even if an opcopylogical is applied afterwards.

[s-perron]

Its been over a year since I've checkd, but seems that still, contrary to the docs

struct Test
{
    // The vk::ext_decorate don't seem to work or emit `OpMemberDecorate` at all
    [[vk::ext_decorate(/*Offset*/ 35,0)]] float4 mem1;
    [[vk::ext_decorate(/*Offset*/ 35,16)]] float mem2;
    [[vk::ext_decorate(/*Offset*/ 35,20)]] int mem3;
};

There are no tests for this functionality in the DXC repo at all

https://godbolt.org/z/x7sx4Po9Y

We have https://github.com/microsoft/DirectXShaderCompiler/issues/4195 to cover this issue.

[devshgraphicsprogramming]

I'm starting to understand the problem, and I'll need to think about how to fix it. The problem in the smaller example is that the SPIR-V backend does not know how to propagate the layout rule for the vk::ext_instruction functions.

In general, the layout rule for a return value in a function is Void. When DXC generates the callee, it will make sure the return value has layout rule Void. In this case, the load function is a single OpLoad instruction, which will result in an object of type T that has a different layout rule.

There are a few options to fix this. In the expansion of the load function, DXC could add code to always rebuild the type with the correct layout rule. This is the best option if it can work.

Other option involve fixing up the code after wards, either in DXC or spirv-opt.

Shouldn't this PR merge https://github.com/microsoft/DirectXShaderCompiler/pull/6873 eliminate the need for you to pass the -Vd commandline option?

[devshgraphicsprogramming]

We are able to deal with workgroup pointers, and we are adding this to the HLSL standard headers: #6873.

I am coming to believe that the layout issue with storage classes that require a layout is insurmountable. We might be able to do some targeted fixes for pointer. If we have a SpirvOpqaueType tha that is a pointer, then try to deduce the layout based on the storage class. However, it would still fail for instructions that do a load or store type instruction using the pointers.

There is no way to specify that the result of an vk::ext_instruction function must have a particular layout. We may need to add an attribute to indicate the layout of the result. I am not planning on doing that until it becomes something we need for the HLSL standard headers that we writing.

I will close this issue, and open a new one in HLSL spec when we decide that we need the layout attribute.

doesn't the -fvk-use-scalar-layout option turn EVERY struct into one that has a scalar layout instead of the std140 and std430 shenanigans? (SSBO, UBO and BDA)

[s-perron]

No it does not. In SPIR-V there are certain storage class that never have a data layout (workgroup, private, function). If you do an OpLoad the layout of the result type has to match the layout of the pointer type. In the normal case, this is easy. DXC knows when it is generating an OpLoad, so it looks at the pointer type to see which layout the result needs.

The problem in your case is that the instruction that does the load is opaque to the code generator, which decides the layout. So the code generator does not know how to deduce the layout type. It just opts for no layout. It works for some cases, and not for others.

It is a big change to fix the way that DXC handles layouts in general. This is too big of a change to go into DXC at this point.

[devshgraphicsprogramming]

No it does not. In SPIR-V there are certain storage class that never have a data layout (workgroup, private, function).

Are there any holes in:

• UBO, SSBO, BDA -> scalar layout
• groupshared, private, function -> no layout for your layout resolution?

This is pure unambiguous mapping of Storage Class to Layout, DXC never respected the attribute offset annotation decoration for inline spirv.

[s-perron]

Yes, there is a hole. What should the layout be for a return value from a function? We chose to make it no "no layout". That is the calling convention. However, if the function call represents a single SPIR-V instruction like an OpLoad, then there is no fixed layout. The layout depends on the input. This ends up breaking the calling convention. Any solution to this is very invasive.

It might also be DXC specific because I'm not sure how we will deal with this in Clang.

[devshgraphicsprogramming]

Post C++11, don't functions always return r-value references when returning structs by value? so one could "walk" eventually find the layout?

Second question, more to the point, this maybe would be solved if the layout was inherently baked into the type.

For every offset decoration combo I can see that DXC codegen emits a separate SPIR-V struct declaration.

Would treating a plain T as scalar-layout (with explicit offset attributes overriding) as the canonical layout and then letting private, groupshared, function be actually separate types ___layoutless<T> with implicit conversions to/from T work?

Taking a step back, the whole problem for my BDA use case seems to stem from the fact that my vk::SpirvType for a pointer doesn't know which concrete SPIR-V struct declaration to hash-cons to. I'd argue that its not really the OpLoad or any other intrinsic that needs to know anything about layouts, its simply my vk::SpirvType<32,... .

So maybe the only thing that needs to be fixed is how vk::SpirvType and vk::SpirvOpaqueType handles template parameters which are types and not vk::integral_constant, maybe vk::laid_out<T,enum layout> ?

P.S. are you sure groupshared has no memory layout, there's the KHR_workgroup_explicit_layout extension. https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_KHR_workgroup_memory_explicit_layout.html https://htmlpreview.github.io/?https://github.com/KhronosGroup/SPIRV-Registry/blob/main/extensions/KHR/SPV_KHR_workgroup_memory_explicit_layout.html