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

[Val] Validator incorrectly validates Vulkan workgroupsize requirements in the universal env #5939

Open
s-perron opened this issue Jan 14, 2025 · 3 comments · Fixed by #5940
Assignees

Comments

@s-perron
Copy link
Collaborator

The SPIR-V does not specific the type for the WorkgroupSize builtin. All it says is "See the client API specification for more detail.". Many of the issues like whether or not the type is a vector of 3 32-bit ints should not be checked in the universal environment.

See https://godbolt.org/z/5s7oa9znz.

; SPIR-V
; Version: 1.4
; Generator: LLVM LLVM SPIR-V Backend; 20
; Bound: 27
; Schema: 0
               OpCapability Kernel
               OpCapability Addresses
               OpCapability Int64
               OpCapability Linkage
          %1 = OpExtInstImport "OpenCL.std"
               OpMemoryModel Physical64 OpenCL
               OpEntryPoint Kernel %test "test" %__spirv_BuiltInWorkgroupSize
               OpExecutionMode %test ContractionOff
               OpSource OpenCL_CPP 100000
               OpName %test "test"
               OpName %__spirv_BuiltInWorkgroupSize "__spirv_BuiltInWorkgroupSize"
               OpName %entry "entry"
               OpDecorate %__spirv_BuiltInWorkgroupSize Constant
               OpDecorate %__spirv_BuiltInWorkgroupSize LinkageAttributes "__spirv_BuiltInWorkgroupSize" Import
               OpDecorate %__spirv_BuiltInWorkgroupSize BuiltIn WorkgroupSize
       %void = OpTypeVoid
      %ulong = OpTypeInt 64 0
       %uint = OpTypeInt 32 0
    %v3ulong = OpTypeVector %ulong 3
%_ptr_Input_v3ulong = OpTypePointer Input %v3ulong
          %8 = OpTypeFunction %void
%__spirv_BuiltInWorkgroupSize = OpVariable %_ptr_Input_v3ulong Input
       %test = OpFunction %void None %8
      %entry = OpLabel
         %11 = OpLoad %v3ulong %__spirv_BuiltInWorkgroupSize Aligned 1
         %12 = OpCompositeExtract %ulong %11 0
               OpReturn
               OpFunctionEnd
@s-perron
Copy link
Collaborator Author

See more discussion on #5407.

@s-perron
Copy link
Collaborator Author

I reopened the issue to follow up on #5940 (comment).

@spencer-lunarg
Copy link
Contributor

To add to @s-perron - Historically, there was no validation around BuiltIns in OpenCL (all was just in Vulkan)

#4801 showed that WorkgroupSize can't have a product of zero (which in an old SPIR-V call we agreed is universal to SPIR-V)

In Vulkan you can only have a 32 bit int vec3 for WorkgroupSize, but for OpenCL (as shown in #5407 comments) can be 64 bit int vec3 as well

Where we are at now is the code still assumes a vec3 and need to understand what is actually allowed in OpenCL/universal here, should it be just int vec3 or can it be something else (if it is, we need test coverage around it)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In progress
Development

Successfully merging a pull request may close this issue.

2 participants