Skip to content
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

[SPIR-V] RawBufferStore still trips up on missing member offsets #6554

Closed
devshgraphicsprogramming opened this issue Apr 22, 2024 · 2 comments · Fixed by #6701
Closed

[SPIR-V] RawBufferStore still trips up on missing member offsets #6554

devshgraphicsprogramming opened this issue Apr 22, 2024 · 2 comments · Fixed by #6701
Assignees
Labels
bug Bug, regression, crash spirv Work related to SPIR-V

Comments

@devshgraphicsprogramming
Copy link

devshgraphicsprogramming commented Apr 22, 2024

Description
The usage of such a struct with vk::RawBufferStore

struct emul_float64_t
{
    using storage_t = float32_t;

    emul_float64_t operator*(const emul_float64_t rhs)
    {
        emul_float64_t retval;
        retval.data = data*rhs.data;
        return retval;
    }

    storage_t data;
};

generates the following error

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

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

Environment

  • DXC version: Latest Godbolt trunk
  • Host Operating System: Linux I guess?
@devshgraphicsprogramming devshgraphicsprogramming added bug Bug, regression, crash needs-triage Awaiting triage spirv Work related to SPIR-V labels Apr 22, 2024
@devshgraphicsprogramming devshgraphicsprogramming changed the title [SPIR-V] [SPIR-V] RawBufferStore still trips up on missing member offsets Apr 22, 2024
@damyanp damyanp moved this to For Google in HLSL Triage Apr 23, 2024
@sudonatalie
Copy link
Collaborator

@devshgraphicsprogramming Thanks for reporting, we'll take a look.

@sudonatalie sudonatalie removed the needs-triage Awaiting triage label May 1, 2024
@sudonatalie sudonatalie moved this from For Google to Triaged in HLSL Triage May 1, 2024
@devshgraphicsprogramming
Copy link
Author

I tried to fix this with inline SPIR-V
https://godbolt.org/z/7sGojeEnK
but:

  1. I need to guess the %result_id% of the type declaration of my struct (in this case is appears to be 20), ties back to [Feature Request] [SPIR-V] [Proposal 0011] vk::addrof(v) to obtain an OpTypePointer to a variable and deprecate [vk::ext_reference] #6578
  2. I can't call an intrinsic from outside of a function (global scope) but some Opcodes can only live there

s-perron added a commit to s-perron/DirectXShaderCompiler that referenced this issue Jun 18, 2024
The first first fix in microsoft#5392 was not correct. It relied on the layout
rule for the address to be the correct layout rule, but that is not
always the case. The address is just an integer that could exist in any
storage class. The correct solution is to explicitly set the layout rule
for the BitCast operation when expanding the RawBuffer* functions. We
know that the result of the BitCast is a pointer to the physical storage
buffer storage class, so we know the layout need to be the storage
buffer layout.

Fixes microsoft#6554
@s-perron s-perron self-assigned this Jun 19, 2024
s-perron added a commit that referenced this issue Jun 25, 2024
The first first fix in #5392 was not correct. It relied on the layout
rule for the address to be the correct layout rule, but that is not
always the case. The address is just an integer that could exist in any
storage class. The correct solution is to explicitly set the layout rule
for the BitCast operation when expanding the RawBuffer* functions. We
know that the result of the BitCast is a pointer to the physical storage
buffer storage class, so we know the layout need to be the storage
buffer layout.

Fixes #6554
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug, regression, crash spirv Work related to SPIR-V
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants