[SPIR-V] Invalid SPIR-V generated where `OpStore` is being used to store to an object with `OpTypeImage`.

[manon-traverse]

I ran into a case where I attempted to use atomics on a 3D texture that is loaded in a bindless manner. When compiling to DXIL there seems to be no issue in the generated code. However, when compiling to SPIR-V I found that it generates invalid SPIR-V.

To illustrate the issue I have attached an example shader that produces the issue, as well as the generated SPIR-V.

HLSL: bin_lines.cs.hlsl.txt
SPIR-V: bin_lines.cs.hlsl#main#SPIR-V#cs_6_6.txt

The interesting bits of SPIR-V are the following:

%type_3d_image = OpTypeImage %uint 3D 2 0 0 2 R32ui
...
%_ptr_Function_type_3d_image = OpTypePointer Function %type_3d_image
...
%131 = OpAccessChain %_ptr_UniformConstant_type_3d_image %g_rwTexture3d %130
%132 = OpLoad %type_3d_image %131
...
`%69 = OpVariable %_ptr_Function_type_3d_image Function`
...
OpStore %69 %132

This contradicts Vulkan's spec, https://www.khronos.org/registry/vulkan/specs/1.1-extensions/html/vkspec.html#VUID-StandaloneSpirv-OpTypeImage-04661 :
"Objects of types OpTypeImage, OpTypeSampler, OpTypeSampledImage, and arrays of these types must not be stored to or modified"

I have also filed an issue on SPIRV-Tools, since spirv-val does not catch this issue either. KhronosGroup/SPIRV-Tools#4796

[jaebaek]

@manon-traverse Thank you for reporting this issue. We will take a look.

[manon-traverse]

@jaebaek
Hi, I was wondering if there is any progress on this issue.
This issue is preventing us from enabling some rendering systems and having this fixed would be really nice for us.

Is there something I could do to help out with tracking down and solving this? I can imagine you are pretty busy with many more issues than this one, so if I could help out in any way with this issue, let me know!

Cheers,
Manon

[s-perron]

I took a quick look. It seems like spirv-opt is not optimizing away the function scope variable like it should, but I'm not sure which pass needs to be updated. The address of the variable is passed to an OpImageTexelPointer. We need to replace the address in the instruction with the UniformConstant address that contains the same value.

[kuhar]

Hi @manon-traverse,

Thanks for bumping this issue. I tried reproducing this using the current top-of-trunk and did not see any OpStore in the final SPIR-V. Could you please provide a few more details:

1. What is the full command to invoke DXC on the example that you attached?
2. What's the version/release of DXC that you used?

[kuhar]

I can see the two OpStores when compiled with: dxc -T cs_6_6  -spirv t.hlsl -E main -HV 2021 -fspv-target-env=vulkan1.2. Thanks, @s-perron!

[cassiebeckley]

I believe I've managed to reduce this to a minimal reproducible example:

RWTexture3D<uint> g_rwTexture3d[100000];

[numthreads(256, 1, 1)] void main() {
    RWTexture3D<uint> voxels = g_rwTexture3d[0];
    uint bucketOffset;
    InterlockedAdd(voxels[uint3(1,2,3)], 1, bucketOffset);
}

Which when compiled with dxc -T cs_6_6 -E main -HV 2021 -fspv-target-env=vulkan1.2 -spirv min.hlsl results in:

; SPIR-V
; Version: 1.5
; Generator: Google spiregg; 0
; Bound: 27
; Schema: 0
               OpCapability Shader
               OpMemoryModel Logical GLSL450
               OpEntryPoint GLCompute %main "main" %g_rwTexture3d
               OpExecutionMode %main LocalSize 256 1 1
               OpSource HLSL 660
               OpName %type_3d_image "type.3d.image"
               OpName %g_rwTexture3d "g_rwTexture3d"
               OpName %main "main"
               OpDecorate %g_rwTexture3d DescriptorSet 0
               OpDecorate %g_rwTexture3d Binding 0
        %int = OpTypeInt 32 1
      %int_0 = OpConstant %int 0
       %uint = OpTypeInt 32 0
     %uint_0 = OpConstant %uint 0
     %uint_1 = OpConstant %uint 1
     %uint_2 = OpConstant %uint 2
     %uint_3 = OpConstant %uint 3
     %v3uint = OpTypeVector %uint 3
         %12 = OpConstantComposite %v3uint %uint_1 %uint_2 %uint_3
%uint_100000 = OpConstant %uint 100000
%type_3d_image = OpTypeImage %uint 3D 2 0 0 2 R32ui
%_arr_type_3d_image_uint_100000 = OpTypeArray %type_3d_image %uint_100000
%_ptr_UniformConstant__arr_type_3d_image_uint_100000 = OpTypePointer UniformConstant %_arr_type_3d_image_uint_100000
       %void = OpTypeVoid
         %17 = OpTypeFunction %void
%_ptr_Function_type_3d_image = OpTypePointer Function %type_3d_image
%_ptr_UniformConstant_type_3d_image = OpTypePointer UniformConstant %type_3d_image
%_ptr_Image_uint = OpTypePointer Image %uint
%g_rwTexture3d = OpVariable %_ptr_UniformConstant__arr_type_3d_image_uint_100000 UniformConstant
       %main = OpFunction %void None %17
         %21 = OpLabel
         %22 = OpVariable %_ptr_Function_type_3d_image Function
         %23 = OpAccessChain %_ptr_UniformConstant_type_3d_image %g_rwTexture3d %int_0
         %24 = OpLoad %type_3d_image %23
               OpStore %22 %24
         %25 = OpImageTexelPointer %_ptr_Image_uint %22 %12 %uint_0
         %26 = OpAtomicIAdd %uint %25 %uint_1 %uint_0 %uint_1
               OpReturn
               OpFunctionEnd

@s-perron could you confirm whether this has the error? I believe it's OpStore %22 %24 right after the OpLoad on the previous line.

[s-perron]

Yes, this contains the invalid code. The spir-v is the code that I would expect the FE to generate. There is a pass is spir-v tools that is suppose to replace the %22 operand in the OpImageTexelPointer by %23. I cannot remember which pass I placed that in.

We can discuss the next step, but I would change g_rwTexture3d so that is is not an array. Then I would run the resulting spir-v through spirv-opt with --print-all to see which pass does the preplacement in the OpImageTexelPointer.