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] Atomic counter emitted when combining RWStructuredBuffer and RayQuery #3287

Closed
MarijnS95 opened this issue Nov 30, 2020 · 3 comments · Fixed by KhronosGroup/SPIRV-Tools#4047
Assignees
Labels
spirv Work related to SPIR-V

Comments

@MarijnS95
Copy link
Contributor

A very simple shader using RWStructuredBuffer or RayQuery seems to compile fine and no counter binding is emitted unless {Increment,Decrement}Counter is used. However, combine them in an unrelated way like below:

RWStructuredBuffer<uint> test : register(u0, space0);

[numthreads(1, 1, 1)]
void main(uint3 DTid: SV_DispatchThreadID) {
    test[0] = 1;
    RayQuery<RAY_FLAG_ACCEPT_FIRST_HIT_AND_END_SEARCH |
             RAY_FLAG_FORCE_OPAQUE> q;
}

Compiled with: dxc -E main -T cs_6_5 structuredbuffer_counter.hlsl -spirv -fspv-target-env=vulkan1.2 this results in:

; SPIR-V
; Version: 1.5
; Generator: Google spiregg; 0
; Bound: 29
; Schema: 0
               OpCapability Shader
               OpCapability RayQueryKHR
               OpExtension "SPV_KHR_ray_query"
               OpMemoryModel Logical GLSL450
               OpEntryPoint GLCompute %main "main" %test %counter_var_test %gl_GlobalInvocationID
               OpExecutionMode %main LocalSize 1 1 1
               OpSource HLSL 650
               OpName %type_RWStructuredBuffer_uint "type.RWStructuredBuffer.uint"
               OpName %test "test"
               OpName %type_ACSBuffer_counter "type.ACSBuffer.counter"
               OpMemberName %type_ACSBuffer_counter 0 "counter"
               OpName %counter_var_test "counter.var.test"
               OpName %main "main"
               OpName %param_var_DTid "param.var.DTid"
               OpName %rayQueryKHR "rayQueryKHR"
               OpDecorate %gl_GlobalInvocationID BuiltIn GlobalInvocationId
               OpDecorate %test DescriptorSet 0
               OpDecorate %test Binding 0
               OpDecorate %counter_var_test DescriptorSet 0
               OpDecorate %counter_var_test Binding 1
               OpDecorate %_runtimearr_uint ArrayStride 4
               OpMemberDecorate %type_RWStructuredBuffer_uint 0 Offset 0
               OpDecorate %type_RWStructuredBuffer_uint Block
               OpMemberDecorate %type_ACSBuffer_counter 0 Offset 0
               OpDecorate %type_ACSBuffer_counter Block
       %uint = OpTypeInt 32 0
     %uint_1 = OpConstant %uint 1
        %int = OpTypeInt 32 1
      %int_0 = OpConstant %int 0
     %uint_0 = OpConstant %uint 0
%_runtimearr_uint = OpTypeRuntimeArray %uint
%type_RWStructuredBuffer_uint = OpTypeStruct %_runtimearr_uint
%_ptr_StorageBuffer_type_RWStructuredBuffer_uint = OpTypePointer StorageBuffer %type_RWStructuredBuffer_uint
%type_ACSBuffer_counter = OpTypeStruct %int
%_ptr_StorageBuffer_type_ACSBuffer_counter = OpTypePointer StorageBuffer %type_ACSBuffer_counter
     %v3uint = OpTypeVector %uint 3
%_ptr_Input_v3uint = OpTypePointer Input %v3uint
       %void = OpTypeVoid
         %20 = OpTypeFunction %void
%_ptr_Function_v3uint = OpTypePointer Function %v3uint
         %22 = OpTypeFunction %void %_ptr_Function_v3uint
%rayQueryKHR = OpTypeRayQueryKHR
%_ptr_Function_rayQueryKHR = OpTypePointer Function %rayQueryKHR
%_ptr_StorageBuffer_uint = OpTypePointer StorageBuffer %uint
       %test = OpVariable %_ptr_StorageBuffer_type_RWStructuredBuffer_uint StorageBuffer
%counter_var_test = OpVariable %_ptr_StorageBuffer_type_ACSBuffer_counter StorageBuffer
%gl_GlobalInvocationID = OpVariable %_ptr_Input_v3uint Input
       %main = OpFunction %void None %20
         %25 = OpLabel
         %26 = OpVariable %_ptr_Function_rayQueryKHR Function
%param_var_DTid = OpVariable %_ptr_Function_v3uint Function
         %27 = OpLoad %v3uint %gl_GlobalInvocationID
         %28 = OpAccessChain %_ptr_StorageBuffer_uint %test %int_0 %uint_0
               OpStore %28 %uint_1
               OpReturn
               OpFunctionEnd

In particular:

               OpName %counter_var_test "counter.var.test"
[...]
%type_ACSBuffer_counter = OpTypeStruct %int
%_ptr_StorageBuffer_type_ACSBuffer_counter = OpTypePointer StorageBuffer %type_ACSBuffer_counter
     %v3uint = OpTypeVector %uint 3
[...]
               OpDecorate %counter_var_test DescriptorSet 0
               OpDecorate %counter_var_test Binding 1
[...]
%counter_var_test = OpVariable %_ptr_StorageBuffer_type_ACSBuffer_counter StorageBuffer

The counter_var_test variable/binding should not be there, any idea why it shows up?

When compiling for DXIL the Dim stays at r/w unless {Increment,Decrement}Counter is added, then it turns into r/w+cnt as expected.

@ehsannas ehsannas added the spirv Work related to SPIR-V label Nov 30, 2020
@ehsannas
Copy link
Contributor

ehsannas commented Dec 2, 2020

Thanks for reporting this @MarijnS95 . I have traced this to be a bug in SPIRV-Tools (spirv-opt should be optimizing that counter away, but it's not doing it).

@MarijnS95
Copy link
Contributor Author

Awesome @ehsannas, simple fix for a simple issue, the resulting SPIR-V is clean again now 👍

At what pace are submodule hashes generally updated?

@ehsannas
Copy link
Contributor

ehsannas commented Dec 2, 2020

Usually once a month, but since this is an important fix for raytracing/query to work, we will pull it in soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spirv Work related to SPIR-V
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants