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] invalid OpImageWrite instruction generated if vk::image_format is not specified. #4773

Closed
Keenuts opened this issue Nov 7, 2022 · 0 comments · Fixed by #4868
Closed
Labels
bug Bug, regression, crash spirv Work related to SPIR-V

Comments

@Keenuts
Copy link
Collaborator

Keenuts commented Nov 7, 2022

Issue derived from (#4772)

HLSL has attributes to specify the texture format (https://github.com/microsoft/DirectXShaderCompiler/blob/main/docs/SPIR-V.rst#textures).

When not specified, the fallback is Rgba32f (first type of the list).
Given this sample:

RWTexture2D<float3> uavs[2];

[numthreads(1,1,1)]
void maincomp(uint3 threadID : SV_DispatchThreadID)
{
  uavs[threadID.z][threadID.xy] = float3(0.5, 0.2, 1.0);
}

and this command:

dxc repro.hlsl -E maincomp -T cs_6_0 -spirv -O0

The generated code is invalid (breaks VUID-RuntimeSpirv-OpImageWrite-07112)

OpImageWrite to any Image whose Image Format is not Unknown must have the Texel operand contain at least as many components as the corresponding [VkFormat](https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkFormat.html) as given in the [SPIR-V Image Format compatibility table](https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#spirvenv-image-formats)

Reason is we default the image format to RGBA32F instead of unknown.
A quick inspection in DXC showed we set it to unknown at first, but then the information is ignored when the type is lowered, and the image format is selected depending on the sampled type (float3), hence RGBA32F, since RGB32F doesn't exists.

Since we emit programs with the shader capability, we could enable StorageImageReadWithoutFormat and keep the format as unknown.

@Keenuts Keenuts added bug Bug, regression, crash spirv Work related to SPIR-V labels Nov 7, 2022
Keenuts added a commit to Keenuts/DirectXShaderCompiler that referenced this issue Dec 12, 2022
This commit aim to fix microsoft#4773. In SPIR-V, image type must be specified,
and GLSL has the syntax for it. In HLSL, we don't, until microsoft#3395.
When this attribute is not specified, we decided to guess the type, not
using Unknown anymore.
But the type guessing was a bit too optimistic, and thus allowed a 3
component layout to be upgraded to 4 component layout.

Changing the guessing function to either give a close type, or return
Unknown, adding the capability on load/store to the variable.
cassiebeckley pushed a commit that referenced this issue Jan 12, 2023
This commit aim to fix #4773. In SPIR-V, image type must be specified,
and GLSL has the syntax for it. In HLSL, we don't, until #3395.
When this attribute is not specified, we decided to guess the type, not
using Unknown anymore.
But the type guessing was a bit too optimistic, and thus allowed a 3
component layout to be upgraded to 4 component layout.

Changing the guessing function to either give a close type, or return
Unknown, adding the capability on load/store to the variable.
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
None yet
Development

Successfully merging a pull request may close this issue.

1 participant