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.2 requirements #1567

Closed
billhollings opened this issue Apr 9, 2022 · 26 comments
Closed

Vulkan 1.2 requirements #1567

billhollings opened this issue Apr 9, 2022 · 26 comments
Labels
Completed Issue has been fixed, or enhancement implemented. Enhancement

Comments

@billhollings
Copy link
Contributor

billhollings commented Apr 9, 2022

Appendix D of the Vulkan spec summarizes the extension list below, and adds additional Vulkan 1.2 enhancement requirements.

Associated SPIRV-Cross issue.

The following extensions are optional for Vulkan 1.2, cannot be supported by Metal, and are marked so by disabling the indicated member in VkPhysicalDeviceVulkan12Features. When crossed out below, it indicates they have been confirmed as unsupported, and set as such in MoltenVK:

Extension Feature Flag Metal Status
VK_KHR_draw_indirect_count drawIndirectCount Not supported in Metal.
VK_KHR_shader_atomic_int64 shaderBufferInt64Atomics Not supported in Metal. See #1692.
VK_KHR_vulkan_memory_model vulkanMemoryModel Although set up for it, Metal does not yet support C++ memory order operations beyond memory_order_relaxed.
VK_EXT_sampler_filter_minmax samplerFilterMinmax Not supported in Metal.

The following extensions are required (or optional) for Vulkan 1.2 and are supported by MoltenVK:

  • VK_KHR_8bit_storage
  • VK_KHR_buffer_device_address
  • VK_KHR_create_renderpass2
  • VK_KHR_depth_stencil_resolve
  • VK_KHR_driver_properties
  • VK_KHR_image_format_list
  • VK_KHR_imageless_framebuffer
  • VK_KHR_sampler_mirror_clamp_to_edge
  • VK_KHR_separate_depth_stencil_layouts
  • VK_KHR_shader_float_controls
  • VK_KHR_shader_float16_int8
  • VK_KHR_shader_subgroup_extended_types
  • VK_KHR_spirv_1_4
  • VK_KHR_timeline_semaphore
  • VK_KHR_uniform_buffer_standard_layout
  • VK_EXT_descriptor_indexing
  • VK_EXT_host_query_reset
  • VK_EXT_scalar_block_layout
  • VK_EXT_separate_stencil_usage
  • VK_EXT_shader_viewport_index_layer
  • SPIR-V version 1.5 (including the ShaderNonUniform capability) (resources in argument buffers)
@spnda
Copy link
Collaborator

spnda commented Jun 6, 2022

VK_KHR_buffer_device_address seems to now be properly supportable with Metal 3:
https://developer.apple.com/documentation/metal/mtlbuffer/3929873-gpuaddress

(not directly relevant, but) VK_NV_mesh_shaders could also now be supported as Metal 3 adds support for Mesh shaders:
https://developer.apple.com/documentation/metal/mtlmeshrenderpipelinedescriptor

@billhollings
Copy link
Contributor Author

billhollings commented Jun 8, 2022

VK_KHR_buffer_device_address seems to now be properly supportable with Metal 3: https://developer.apple.com/documentation/metal/mtlbuffer/3929873-gpuaddress

(not directly relevant, but) VK_NV_mesh_shaders could also now be supported as Metal 3 adds support for Mesh shaders: https://developer.apple.com/documentation/metal/mtlmeshrenderpipelinedescriptor

Consolidating in discussion #1616. Thanks for noticing the relatively subtle MTLBuffer.gpuAddress!

@spnda
Copy link
Collaborator

spnda commented Jul 6, 2022

Please note that not all extensions that were promoted to core 1.2 are actually required for the implementation to conform to 1.2. A good few of them are actually optional.

  • VK_KHR_draw_indirect_count is optional.
  • VK_KHR_shader_atomic_int64 is optional.
  • VK_KHR_vulkan_memory_model is optional.
  • VK_EXT_sampler_filter_minmax is optional.

If the VK_KHR_vulkan_memory_model extension is not supported, support for the vulkanMemoryModel feature is optional.

If the VK_KHR_shader_atomic_int64 extension is not supported, support for the shaderBufferInt64Atomics feature is optional.

If the VK_KHR_draw_indirect_count extension is not supported, support for the entry points vkCmdDrawIndirectCount and vkCmdDrawIndexedIndirectCount is optional.

samplerFilterMinmax, if the VK_EXT_sampler_filter_minmax extension is supported.

Note: There is no Differences relative to VK_EXT_sampler_filter_minmax in the Version 1.2 chapter of Appendix D, which I think is an issue in the spec, as the quote above is from section 41.1 listing feature requirements and it does not require Vulkan 1.2. Otherwise, it would have been written like this:

uniformBufferStandardLayout, if Vulkan 1.2 or the VK_KHR_uniform_buffer_standard_layout extension is supported.

This greatly reduces the amount of extensions still pending. See the list of feature requirements here at section 41.1 of the Vulkan spec: https://www.khronos.org/registry/vulkan/specs/1.2-extensions/html/vkspec.html#features-requirements, or look at the Differences blocks in the Appendix D chapter. This leaves us with only two extensions before Vulkan 1.2 can be officially supported, and they're both essentially just new SPIR-V capabilities.

  • VK_KHR_shader_float_controls
  • VK_KHR_spirv_1_4

@oscarbg
Copy link

oscarbg commented Jul 11, 2022

unrelated, but sadly&interestingly DXVK-Macos project shows on:
https://github.com/Gcenx/DXVK-macOS/releases/tag/v1.10.1-125-gf95f5418
"This will be the final release of DXVK-macOS until MoltenVK supports Vulkan-1.2"

@billhollings
Copy link
Contributor Author

not all extensions that were promoted to core 1.2 are actually required for the implementation to conform to 1.2 ...[clip]... This greatly reduces the amount of extensions still pending.

Thanks for researching and highlighting this! We'll make use of this where appropriate.

@billhollings
Copy link
Contributor Author

This will be the final release of DXVK-macOS until MoltenVK supports Vulkan-1.2

Thanks. Good motivation! We're actively working on this!

@saschasc
Copy link

@billhollings Any ETA when Vulkan 1.2 will be supported by MoltenVK? The progress sounds great so far!

@billhollings
Copy link
Contributor Author

Any ETA when Vulkan 1.2 will be supported by MoltenVK? The progress sounds great so far!

Vulkan 1.2 support is one of the main things we are working on. I'm hoping to knock off the remaining extensions above within the next couple of months.

@spnda
Copy link
Collaborator

spnda commented Aug 16, 2022

From what I can see the only requirement is VK_KHR_shader_float_controls which is mostly just SPIRV-Cross stuff. MVK only needs to advertise their support and update the SPIRV-Cross commit for building. VK_KHR_spirv_1_4 also only implements the aforementioned extension into SPIR-V core. Not sure if the SPIR-V capabilities can be properly translated into MSL though.

@billhollings
Copy link
Contributor Author

billhollings commented Aug 16, 2022

From what I can see the only requirement is VK_KHR_shader_float_controls which is mostly just SPIRV-Cross stuff.

I'm going to start working on VK_KHR_shader_float_controls, including the SPIRV-Cross stuff, per #1684.

@Gcenx
Copy link

Gcenx commented Aug 16, 2022

unrelated, but sadly&interestingly DXVK-Macos project shows on:
https://github.com/Gcenx/DXVK-macOS/releases/tag/v1.10.1-125-gf95f5418
"This will be the final release of DXVK-macOS until MoltenVK supports Vulkan-1.2"

Welp it’s the last version based on master, I’d swapped to using the 1.10.x branch for the base until MoltenVK supports the now required extensions.

I did list some of the newly required extensions, I’m sure master now uses even more.

@oscarbg
Copy link

oscarbg commented Aug 19, 2022

@Gcenx where is this list of newly required extensions?

@billhollings @spnda
just became interested on VK_KHR_shader_atomic_int64 seeing code from HPG2022 best paper
"Software Rasterization of 2 Billion Points in Real-Time" using compute shaders..
https://github.com/m-schuetz/compute_rasterizer
code is using OpenGL so requires GL_NV_shader_atomic_int64 (one render mode uses atomicMin on uint64_t buffer (SSBO))
https://gitlab.freedesktop.org/mesa/mesa/-/issues/7087
but eventually porting to Vulkan on Mac will require VK_KHR_shader_atomic_int64..

seeing https://developer.apple.com/metal/Metal-Shading-Language-Specification.pdf:
at first seemed to me that should be supported since Metal 2.4..
"atomic_ulong A type of alias of atomic for OSes that support Metal 2.4 and later"
but upon further inspection there a two main limitations/issues: first Metal only supports unsigned int64 vs vulkan supports signed or unsigned 64 bit ints..
also later seem to find that atomic_ulong only supports atomic min, max ops in section 6.15.2.6 but sadly Vulkan extension needs
OpAtomicMin, OpAtomicMax, OpAtomicAnd, OpAtomicOr, OpAtomicXor, OpAtomicAdd, OpAtomicExchange, and OpAtomicCompareExchange.

so conclusion is Metal can't support VK_KHR_shader_atomic_int64 "fully" right now for all ops, if documentation is correct, but only a subset (Min,Max) which luckily is what a port of the algorithm will need..
question is can this subset of VK_KHR_shader_atomic_int64 be implemented and only exposed through some enviroment variable like VK_MVK_enable_partial_extensions or some idea like #1670..

probably better if opened as separate issue..

while I'm at it I give a quick lock of atomic Floats(32) and seems luckily all supported Vulkan atomic float variants (OpAtomicFAddEXT, OpAtomicExchange, OpAtomicLoad and OpAtomicStore) are supported on Metal 3.0..

@Gcenx
Copy link

Gcenx commented Aug 19, 2022

@oscarbg this being an incomplete list as I’ve not looked at master recently due to missing required features. And no tear can’t be faked or bypassed or DXVK will crash crash!

Trying to make a list of required extensions post DXVK-1.10.1-125

@spnda
Copy link
Collaborator

spnda commented Aug 19, 2022

@Gcenx VK_KHR_dynamic_rendering is fully supported with MVK. VK_KHR_create_renderpass2 is also already supported. However, VK_EXT_extended_dynamic_state is not supportable out of the box with the API that Metal offers due to the lack of commands for configuring dynamic depth/stencil state. It could be possible to pre-build various combinations of depth states, though im not sure how reliable and performant that is. I'm not sure which commands are required by the spec to advertise support for the extension, so perhaps excluding those few is valid. I have already implemented the rest of the commands on a branch of mine.

@Gcenx
Copy link

Gcenx commented Aug 19, 2022

@spnda the problem is the dynamic extensions are also required, if stoped listing anything else after these considering those are hard requirements then more extensions are required.

@rcaridade145
Copy link

There is a pull request on dxvk doitsujin/dxvk#2826 define in a json the required extensions. If i understand correctly VK_EXT_extended_dynamic_state is only required on master (to be 2.0) not the 1.10.x branch.

@Gcenx
Copy link

Gcenx commented Aug 19, 2022

There is a pull request on dxvk doitsujin/dxvk#2826 define in a json the required extensions. If i understand correctly VK_EXT_extended_dynamic_state is only required on master (to be 2.0) not the 1.10.x branch.

Yes the (lightly incomplete list from me) is what's required for master branch I'd also said this, I have a 1.10.x branch already, however as DXVK master now requires Vulkan 1.2 features it's good to have them listed here.

The json isn't very in depth so lightly it won't be too useful.

@billhollings
Copy link
Contributor Author

so conclusion is Metal can't support VK_KHR_shader_atomic_int64 "fully" right now for all ops, if documentation is correct, but only a subset (Min,Max) which luckily is what a port of the algorithm will need.. question is can this subset of VK_KHR_shader_atomic_int64 be implemented and only exposed through some enviroment variable like VK_MVK_enable_partial_extensions or some idea like #1670..

probably better if opened as separate issue..

Opened in #1692, and I am looking into it.

@billhollings
Copy link
Contributor Author

Opened in #1692, and I am looking into it.

Unfortunately, it's not going to be possible to implement any subset of VK_KHR_shader_atomic_int64 with current Metal functionality (Metal 3).

Sigh. So close. 🤦🏻‍♂️

@spnda
Copy link
Collaborator

spnda commented Sep 25, 2022

So, with #1724 merged, what's left? Looks to me as if Vulkan 1.2 core has been reached.

@billhollings
Copy link
Contributor Author

billhollings commented Sep 25, 2022

So, with #1724 merged, what's left? Looks to me as if Vulkan 1.2 core has been reached.

Yeah. I think we're good.

I'm actually working on a 1.2 commit now, and when I post it as a PR, we can discuss if it covers everything.

@billhollings
Copy link
Contributor Author

Taking an inventory, it looks like we're not using the following structures at all. But since the extensions that provide them are already active, perhaps we can investigate and add their functionality if needed after the 1.2 release:

VK_KHR_imageless_framebuffer:
- VkFramebufferAttachmentsCreateInfo
- VkFramebufferAttachmentImageInfo

VK_KHR_separate_depth_stencil_layouts:
- VkAttachmentDescriptionStencilLayout
- VkAttachmentReferenceStencilLayout

VK_KHR_buffer_device_address:
- VkBufferOpaqueCaptureAddressCreateInfo
- VkMemoryOpaqueCaptureAddressAllocateInfo

I also want to run a 1.2 build against CTS to see if it has any effect on the tests beyond the results of the equivalent extensions being enabled.

@billhollings billhollings added the Completed Issue has been fixed, or enhancement implemented. label Sep 26, 2022
@billhollings
Copy link
Contributor Author

Implemented by PR #1726. Outstanding question above about unused extension structs has been moved to a separate issue #1727.

Closing this now.

@EwoutH
Copy link
Contributor

EwoutH commented Oct 18, 2022

@billhollings congratulations on getting all the Vulkan 1.2 extensions in and released!

A quick note, you can format a nice to-do list in GitHub markdown using tickboxes:

  • Done
  • Not done

The syntax for this is very simple:

- [x] Done
- [ ] Not done

Might also be nice for a next tracking issue for Vulkan 1.3! :)

@billhollings
Copy link
Contributor Author

@EwoutH Thanks for the suggestion. Makes sense. I will be creating a 1.3 task list soon, and we can use checkmarks instead of strike-out text.

@billhollings
Copy link
Contributor Author

Might also be nice for a next tracking issue for Vulkan 1.3! :)

Issue #1930 is now tracking Vulkan 1.3 requirements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Completed Issue has been fixed, or enhancement implemented. Enhancement
Projects
None yet
Development

No branches or pull requests

7 participants