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

Vulkan 1.3 requirements #1930

Open
billhollings opened this issue Jun 1, 2023 · 14 comments
Open

Vulkan 1.3 requirements #1930

billhollings opened this issue Jun 1, 2023 · 14 comments

Comments

@billhollings
Copy link
Contributor

billhollings commented Jun 1, 2023

Appendix D of the Vulkan spec and the VK_VERSION_1_3 man page summarize the extension list below, and adds additional Vulkan 1.3 enhancement requirements.

Extension MoltenVK
Support
Status Notes
VK_KHR_copy_commands2
VK_KHR_dynamic_rendering
VK_KHR_format_feature_flags2 Added (#2108)
VK_KHR_maintenance4 In Dev (#2116)
VK_KHR_shader_integer_dot_product Added (#2119)
VK_KHR_shader_non_semantic_info
VK_KHR_shader_terminate_invocation
VK_KHR_synchronization2 Added (#2021)
VK_KHR_zero_initialize_workgroup_memory
VK_EXT_4444_formats
VK_EXT_extended_dynamic_state Added (#2036)
VK_EXT_extended_dynamic_state2 Added (#2036)
VK_EXT_image_robustness
VK_EXT_inline_uniform_block
VK_EXT_pipeline_creation_cache_control
VK_EXT_pipeline_creation_feedback
VK_EXT_private_data
VK_EXT_shader_demote_to_helper_invocation
VK_EXT_subgroup_size_control
VK_EXT_texel_buffer_alignment
VK_EXT_texture_compression_astc_hdr
VK_EXT_tooling_info
VK_EXT_ycbcr_2plane_444_formats

In addition to the promoted extensions described above, Vulkan 1.3 added required support for:

  • SPIR-V version 1.6
    • SPIR-V 1.6 deprecates (but does not remove) the WorkgroupSize decoration.
  • The bufferDeviceAddress feature which indicates support for accessing memory in shaders as storage buffers via vkGetBufferDeviceAddress.
  • The vulkanMemoryModel and vulkanMemoryModelDeviceScope features, which indicate support for the corresponding Vulkan Memory Model capabilities.
  • The maxInlineUniformTotalSize limit is added to provide the total size of all inline uniform block bindings in a pipeline layout.

Additional details of functionality and information that may need to be included in a 1.3 implementation can be found here.

@CarterLi
Copy link

CarterLi commented Jun 5, 2023

Ref: haasn/libplacebo#174

@rcaridade145
Copy link

In regards to VK_KHR_shader_integer_dot_product

I came across

gpuweb/gpuweb#2677

Which led to me.

-https://gitlab.freedesktop.org/mesa/mesa/-/blob/main/src/compiler/nir/nir_opcodes.py#L1367

Hopefully this can be useful.

@cdavis5e
Copy link
Collaborator

cdavis5e commented Jun 9, 2023

In regards to VK_KHR_shader_integer_dot_product

I came across

gpuweb/gpuweb#2677

Which led to me.

-https://gitlab.freedesktop.org/mesa/mesa/-/blob/main/src/compiler/nir/nir_opcodes.py#L1367

Hopefully this can be useful.

That bit of code you highlighted is for a packed saturating vertical add. That's only part of three of the new instructions added by that extension. Metal already has that (as addsat()). For a dot product, we need a vertical multiply followed by a horizontal add (i.e. add all elements of the product vector together). AFAIK, Metal doesn't have either an overload of dot() that accepts integral input or any sort of horizontal add. We'd have to expand the final addition manually, which would be painful.

@billhollings
Copy link
Contributor Author

We'd have to expand the final addition manually, which would be painful.

By that, you mean adding the required synthetic functions to SPIRV-Cross?

@cdavis5e
Copy link
Collaborator

We'd have to expand the final addition manually, which would be painful.

By that, you mean adding the required synthetic functions to SPIRV-Cross?

Yes.

@rcaridade145
Copy link

gfx-rs/naga#1689 People of webgpu seem to have dealt with dot product with integers on Metal.

@rcaridade145
Copy link

Been studying spirv-cross code and other alternatives like naga and tint (really nice code) .

Tint - https://dawn.googlesource.com/tint/+/refs/heads/main/src/tint/writer/msl/generator_impl.cc#681

It should be implemented something like that (to support some integer dot) here no?

https://github.com/KhronosGroup/SPIRV-Cross/blob/main/spirv_msl.cpp#L5414

@devshgraphicsprogramming

btw vulkanMemoryModel starts to be quite important when SPV_KHR_physical_storage_buffer gets used because you start needing to use it on OpLoad and OpStore if you want Buffer Device Addresses to volatile/coherent variables

@AndrewTriesToCode
Copy link

AndrewTriesToCode commented Jan 22, 2024

Hi, just want to confirm that until this is supported only 1.x DXVK releases are compatible with MoltenVK. Do I understand that right?

Thanks yall are awesome.

@cdavis5e
Copy link
Collaborator

Hi, just want to confirm that until this is supported only 1.x DXVK releases are compatible with MoltenVK. Do I understand that right?

Yes, that is correct. Hopefully, you won't have to wait much longer...

@ForestCSharp
Copy link

ForestCSharp commented Mar 8, 2024

Should I be able to use dynamic rendering without the Ext function pointers?
I'm currently getting a segfault calling vkCmdBeginRendering/vkCmdBeginRendering, though I can access the same functionality via
pfn_begin_rendering = (PFN_vkCmdBeginRenderingKHR)vkGetDeviceProcAddr(device, "vkCmdBeginRenderingKHR"); pfn_begin_rendering(vk_command_buffer, &vk_rendering_info);
but would prefer to have one codepath for Windows + Mac using the 1.3 functions

@spnda
Copy link
Collaborator

spnda commented Mar 8, 2024

Should I be able to use dynamic rendering without the Ext function pointers?

No. That's invalid usage of Vulkan, as 1.3 is not advertised.

Your problem is easily avoidable anyway, just use an if to switch between loading the core function and the extension function, based on the physical device API version. The functions have the same signature, and the structures didn't change from the promotion, so its all backwards compatible.

@spnda
Copy link
Collaborator

spnda commented Apr 28, 2024

I just by accident found another extension which was made core with Vulkan 1.3, but was not listed in the Appendix you linked in the original post.

VK_ATTACHMENT_STORE_OP_NONE was added with 1.3, meaning VK_EXT_load_store_op_none was partially made core. The spec says this:

VK_ATTACHMENT_STORE_OP_NONE specifies the contents within the render area are not accessed by the store operation as long as no values are written to the attachment during the render pass. If values are written during the render pass, this behaves identically to VK_ATTACHMENT_STORE_OP_DONT_CARE and with matching access semantics.

Seems like this could just be emulated with MTLStoreActionDontCare. I'll look into a quick implementation of that op and the two extensions providing it. But it should probably still be listed in the original post.

Also, perhaps we missed some other things found here that need to be supported.

@billhollings
Copy link
Contributor Author

VK_ATTACHMENT_STORE_OP_NONE was added with 1.3, meaning VK_EXT_load_store_op_none was partially made core.

Actually, this was also added via VK_KHR_dynamic_rendering, which is why it also ended up in 1.3.

Since MoltenVK already supports VK_KHR_dynamic_rendering, it should also support VK_ATTACHMENT_STORE_OP_NONE, which it doesn't deliberately, but since it defaults to STORE, it does successfully use it.

Seems like this could just be emulated with MTLStoreActionDontCare.

Hmmm...as part of the description above, the spec also includes:

VK_ATTACHMENT_STORE_OP_DONT_CARE can cause contents generated during previous render passes to be discarded before reaching memory, even if no write to the attachment occurs during the current render pass.

With this in mind, my interpretation of the difference between VK_ATTACHMENT_STORE_OP_NONE and VK_ATTACHMENT_STORE_OP_DONT_CARE, is that NONE indicates that the attachment is not written to in this render pass, so it doesn't need to be stored, but it needs to be retained for further render passes.

And I read the "If values are written..." part to mean, "If you write when you said you wouldn't, we're not going to save the changes, and you've messed it up already, so we're not going to bother retaining it".

With all that in mind, I think MTLStoreActionStore would be appropriate, since we want to ensure the content is not discarded. MoltenVK does this in mvkMTLStoreActionFromVkAttachmentStoreOpInObj() through the default, but we should fix MoltenVK to do this officially, to avoid the error message, particularly since VK_KHR_dynamic_rendering is supported now.

Also, perhaps we missed some other things found here that need to be supported.

Good catch. I've added this to the main section above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants