[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.