search

KhronosGroup / Vulkan-Portability

Apache License 2.0
40 stars 4 forks source link

Add VkPhysicalDevicePortabilitySubsetFeaturesKHR:: shaderSharedVariableReleaseAcquire capability #57

Open billhollings opened 2 years ago

billhollings commented 2 years ago

Some CTS tests fail on shader releasing and acquiring shared variables.

This is most likely fixable with effort within the implementations.

In the meantime, these use cases can be disabled in CTS and reported as Not Supported, by adding a VkPhysicalDevicePortabilitySubsetFeaturesKHR:: shaderSharedVariableReleaseAcquire capability.

This affects 2 CTS tests:

dEQP-VK.memory_model.message_passing.permuted_index.release_acquire
dEQP-VK.memory_model.message_passing.permuted_index.release_acquire_atomic_payload
  Fail (Fail)
kvark commented 2 years ago

Isn't supporting Vulkan Memory model optional? The model wasn't formalized at Vk-1.0 time. So a Vulkan-1.0 portability implementation shouldn't need to pass the memory model tests at all.

cdavis5e commented 2 years ago

Isn't supporting Vulkan Memory model optional?

Not all the tests under dEQP-VK.memory_model concern VK_KHR_vulkan_memory_model. (I actually pointed this concern out when Bill started working on fixing these tests, and that was his reply.) Believe it or not, the permuted_index tests are among those that don't require it.

The problem AIUI is that Metal atomics currently only support relaxed ordering, when SPIR-V specifies stronger orderings acquire, release, acq_rel, and seq_cst.

kvark commented 2 years ago

Interesting. Should this flag be called shaderAtomicOrdering then (trying to come up with something closer to the semantics)?