[PATCH 0/3] MR776: vkd3d-shader: Introduce SPIRV_ENVIRONMENT_VULKAN_1_1.
Shader Model 6 wave ops require instructions not available by extension in SPIR-V 1.0, and device support info is found in VkPhysicalDeviceSubgroupProperties, which is also has no equivalent by extension in Vulkan 1.0. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/776
From: Conor McCarthy <cmccarthy(a)codeweavers.com> The SPIR-V backend will emit SPIR-V 1.3 if Vulkan 1.1 is available. This is required for Shader Model 6 wave ops. --- include/vkd3d_shader.h | 1 + libs/vkd3d-shader/spirv.c | 17 +++++++++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index 5cc36e186..f3a57889e 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -923,6 +923,7 @@ enum vkd3d_shader_spirv_environment VKD3D_SHADER_SPIRV_ENVIRONMENT_NONE, VKD3D_SHADER_SPIRV_ENVIRONMENT_OPENGL_4_5, VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_0, /* default target */ + VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_1, VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_SPIRV_ENVIRONMENT), }; diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 61390b09c..0fbb5c47e 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -45,6 +45,8 @@ static spv_target_env spv_target_env_from_vkd3d(enum vkd3d_shader_spirv_environm return SPV_ENV_OPENGL_4_5; case VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_0: return SPV_ENV_VULKAN_1_0; + case VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_1: + return SPV_ENV_VULKAN_1_1; default: ERR("Invalid environment %#x.\n", environment); return SPV_ENV_VULKAN_1_0; @@ -228,7 +230,8 @@ static bool data_type_is_floating_point(enum vkd3d_data_type data_type) return data_type == VKD3D_DATA_HALF || data_type == VKD3D_DATA_FLOAT || data_type == VKD3D_DATA_DOUBLE; } -#define VKD3D_SPIRV_VERSION 0x00010000 +#define VKD3D_SPIRV_VERSION_1_0 0x00010000 +#define VKD3D_SPIRV_VERSION_1_3 0x00010300 #define VKD3D_SPIRV_GENERATOR_ID 18 #define VKD3D_SPIRV_GENERATOR_VERSION 11 #define VKD3D_SPIRV_GENERATOR_MAGIC vkd3d_make_u32(VKD3D_SPIRV_GENERATOR_VERSION, VKD3D_SPIRV_GENERATOR_ID) @@ -1920,7 +1923,7 @@ static void vkd3d_spirv_builder_free(struct vkd3d_spirv_builder *builder) } static bool vkd3d_spirv_compile_module(struct vkd3d_spirv_builder *builder, - struct vkd3d_shader_code *spirv, const char *entry_point) + struct vkd3d_shader_code *spirv, const char *entry_point, enum vkd3d_shader_spirv_environment environment) { uint64_t capability_mask = builder->capability_mask; struct vkd3d_spirv_stream stream; @@ -1931,7 +1934,8 @@ static bool vkd3d_spirv_compile_module(struct vkd3d_spirv_builder *builder, vkd3d_spirv_stream_init(&stream); vkd3d_spirv_build_word(&stream, SpvMagicNumber); - vkd3d_spirv_build_word(&stream, VKD3D_SPIRV_VERSION); + vkd3d_spirv_build_word(&stream, (environment == VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_1) + ? VKD3D_SPIRV_VERSION_1_3 : VKD3D_SPIRV_VERSION_1_0); vkd3d_spirv_build_word(&stream, VKD3D_SPIRV_GENERATOR_MAGIC); vkd3d_spirv_build_word(&stream, builder->current_id); /* bound */ vkd3d_spirv_build_word(&stream, 0); /* schema, reserved */ @@ -2480,6 +2484,7 @@ static struct spirv_compiler *spirv_compiler_create(const struct vsir_program *p { case VKD3D_SHADER_SPIRV_ENVIRONMENT_OPENGL_4_5: case VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_0: + case VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_1: break; default: WARN("Invalid target environment %#x.\n", target_info->environment); @@ -10157,6 +10162,7 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler, struct vkd3d_spirv_builder *builder = &compiler->spirv_builder; struct vkd3d_shader_instruction_array instructions; struct vsir_program *program = &parser->program; + enum vkd3d_shader_spirv_environment environment; enum vkd3d_result result = VKD3D_OK; unsigned int i; @@ -10241,12 +10247,12 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler, if (compiler->strip_debug) vkd3d_spirv_stream_clear(&builder->debug_stream); - if (!vkd3d_spirv_compile_module(builder, spirv, spirv_compiler_get_entry_point_name(compiler))) + environment = spirv_compiler_get_target_environment(compiler); + if (!vkd3d_spirv_compile_module(builder, spirv, spirv_compiler_get_entry_point_name(compiler), environment)) return VKD3D_ERROR; if (TRACE_ON() || parser->config_flags & VKD3D_SHADER_CONFIG_FLAG_FORCE_VALIDATION) { - enum vkd3d_shader_spirv_environment environment = spirv_compiler_get_target_environment(compiler); struct vkd3d_string_buffer buffer; if (TRACE_ON()) @@ -10274,7 +10280,6 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler, if (compile_info->target_type == VKD3D_SHADER_TARGET_SPIRV_TEXT) { struct vkd3d_shader_code text; - enum vkd3d_shader_spirv_environment environment = spirv_compiler_get_target_environment(compiler); if (vkd3d_spirv_binary_to_text(spirv, environment, compiler->formatting, &text) != VKD3D_OK) return VKD3D_ERROR; vkd3d_shader_free_shader_code(spirv); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/776
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d/device.c | 14 ++++++++++++++ libs/vkd3d/state.c | 6 +++--- libs/vkd3d/vkd3d_private.h | 2 ++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/libs/vkd3d/device.c b/libs/vkd3d/device.c index 65db8b70b..8b4c25501 100644 --- a/libs/vkd3d/device.c +++ b/libs/vkd3d/device.c @@ -558,12 +558,14 @@ static HRESULT vkd3d_instance_init(struct vkd3d_instance *instance, const struct vkd3d_optional_instance_extensions_info *optional_extensions; const struct vkd3d_application_info *vkd3d_application_info; const struct vkd3d_host_time_domain_info *time_domain_info; + PFN_vkEnumerateInstanceVersion vkEnumerateInstanceVersion; bool *user_extension_supported = NULL; VkApplicationInfo application_info; VkInstanceCreateInfo instance_info; char application_name[PATH_MAX]; uint32_t extension_count; const char **extensions; + uint32_t vk_api_version; VkInstance vk_instance; VkResult vr; HRESULT hr; @@ -616,6 +618,16 @@ static HRESULT vkd3d_instance_init(struct vkd3d_instance *instance, application_info.apiVersion = VK_API_VERSION_1_0; instance->api_version = VKD3D_API_VERSION_1_0; + /* vkEnumerateInstanceVersion was added in Vulkan 1.1, and its absence indicates only 1.0 is supported. */ + vkEnumerateInstanceVersion = (void *)vk_global_procs->vkGetInstanceProcAddr(NULL, "vkEnumerateInstanceVersion"); + if (vkEnumerateInstanceVersion && vkEnumerateInstanceVersion(&vk_api_version) >= 0 + && vk_api_version >= VK_API_VERSION_1_1) + { + TRACE("Vulkan API version 1.1 is available; requesting it.\n"); + application_info.apiVersion = VK_API_VERSION_1_1; + } + instance->vk_api_version = application_info.apiVersion; + if ((vkd3d_application_info = vkd3d_find_struct(create_info->next, APPLICATION_INFO))) { if (vkd3d_application_info->application_name) @@ -5055,6 +5067,8 @@ static HRESULT d3d12_device_init(struct d3d12_device *device, device->vk_info = instance->vk_info; device->signal_event = instance->signal_event; device->wchar_size = instance->wchar_size; + device->environment = (instance->vk_api_version >= VK_API_VERSION_1_1) + ? VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_1 : VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_0; device->adapter_luid = create_info->adapter_luid; device->removed_reason = S_OK; diff --git a/libs/vkd3d/state.c b/libs/vkd3d/state.c index 08cc110e8..da8b9b3e2 100644 --- a/libs/vkd3d/state.c +++ b/libs/vkd3d/state.c @@ -2435,7 +2435,7 @@ static HRESULT d3d12_pipeline_state_init_compute(struct d3d12_pipeline_state *st memset(&target_info, 0, sizeof(target_info)); target_info.type = VKD3D_SHADER_STRUCTURE_TYPE_SPIRV_TARGET_INFO; - target_info.environment = VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_0; + target_info.environment = device->environment; target_info.extensions = device->vk_info.shader_extensions; target_info.extension_count = device->vk_info.shader_extension_count; @@ -3156,7 +3156,7 @@ static HRESULT d3d12_pipeline_state_init_graphics(struct d3d12_pipeline_state *s ps_target_info.type = VKD3D_SHADER_STRUCTURE_TYPE_SPIRV_TARGET_INFO; ps_target_info.next = NULL; ps_target_info.entry_point = "main"; - ps_target_info.environment = VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_0; + ps_target_info.environment = device->environment; ps_target_info.extensions = vk_info->shader_extensions; ps_target_info.extension_count = vk_info->shader_extension_count; ps_target_info.parameters = ps_shader_parameters; @@ -3186,7 +3186,7 @@ static HRESULT d3d12_pipeline_state_init_graphics(struct d3d12_pipeline_state *s memset(&target_info, 0, sizeof(target_info)); target_info.type = VKD3D_SHADER_STRUCTURE_TYPE_SPIRV_TARGET_INFO; - target_info.environment = VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_0; + target_info.environment = device->environment; target_info.extensions = vk_info->shader_extensions; target_info.extension_count = vk_info->shader_extension_count; diff --git a/libs/vkd3d/vkd3d_private.h b/libs/vkd3d/vkd3d_private.h index d5e12ee3c..b3e3631fd 100644 --- a/libs/vkd3d/vkd3d_private.h +++ b/libs/vkd3d/vkd3d_private.h @@ -185,6 +185,7 @@ struct vkd3d_instance struct vkd3d_vulkan_info vk_info; struct vkd3d_vk_global_procs vk_global_procs; void *libvulkan; + uint32_t vk_api_version; uint64_t config_flags; enum vkd3d_api_version api_version; @@ -1677,6 +1678,7 @@ struct d3d12_device struct vkd3d_vk_device_procs vk_procs; PFN_vkd3d_signal_event signal_event; size_t wchar_size; + enum vkd3d_shader_spirv_environment environment; struct vkd3d_gpu_va_allocator gpu_va_allocator; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/776
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- include/vkd3d_shader.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index f3a57889e..1ffcc807a 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -215,6 +215,14 @@ enum vkd3d_shader_compile_option_feature_flags * This corresponds to the "shaderFloat64" feature in the Vulkan API, and * the "GL_ARB_gpu_shader_fp64" extension in the OpenGL API. */ VKD3D_SHADER_COMPILE_OPTION_FEATURE_FLOAT64 = 0x00000002, + /** The SPIR-V target environment supports wave operations. + * This corresponds to the following minimum requirements in + * VkPhysicalDeviceSubgroupProperties: + * subgroupSize >= 4 + * supportedOperations has BASIC, VOTE, ARITHMETIC, BALLOT, SHUFFLE and + * QUAD bits set. + * supportedStages include COMPUTE and FRAGMENT. */ + VKD3D_SHADER_COMPILE_OPTION_FEATURE_WAVE_OPS = 0x00000004, VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_COMPILE_OPTION_FEATURE_FLAGS), }; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/776
Giovanni Mascellani (@giomasce) commented about include/vkd3d_shader.h:
* This corresponds to the "shaderFloat64" feature in the Vulkan API, and * the "GL_ARB_gpu_shader_fp64" extension in the OpenGL API. */ VKD3D_SHADER_COMPILE_OPTION_FEATURE_FLOAT64 = 0x00000002, + /** The SPIR-V target environment supports wave operations. + * This corresponds to the following minimum requirements in + * VkPhysicalDeviceSubgroupProperties: + * subgroupSize >= 4 + * supportedOperations has BASIC, VOTE, ARITHMETIC, BALLOT, SHUFFLE and + * QUAD bits set. + * supportedStages include COMPUTE and FRAGMENT. */ + VKD3D_SHADER_COMPILE_OPTION_FEATURE_WAVE_OPS = 0x00000004,
Should there be a rule mandating than when you're compiling with this flag you should also specify `VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_1` as the target environment? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/776#note_67482
Along something of the same lines as Gio's comment, is VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_1 actually doing anything? We need VKD3D_SHADER_COMPILE_OPTION_FEATURE_WAVE_OPS anyway; should we just set the SPIRV version and Vulkan environment based on that? Note for review: I did double check, and this functionality is core in Vulkan 1.1 without having gone through an extension first. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/776#note_67561
Along something of the same lines as Gio's comment, is VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_1 actually doing anything? We need VKD3D_SHADER_COMPILE_OPTION_FEATURE_WAVE_OPS anyway; should we just set the SPIRV version and Vulkan environment based on that?
My understanding is that they're somewhat independent features: there might be differences between environments which we want to respect even if they're not captured by the features of the specific device we're using. If we're feeding the SPIR-V code to OpenGL vs Vulkan we might need to generate different code even if it's going to run on the same GPU (thus with the same capabilities). In abstract, at least; I don't know whether we already have an instance of that, but it makes sense for the API to be ready. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/776#note_67562
On Wed Apr 10 18:00:40 2024 +0000, Giovanni Mascellani wrote:
Along something of the same lines as Gio's comment, is VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_1 actually doing anything? We need VKD3D_SHADER_COMPILE_OPTION_FEATURE_WAVE_OPS anyway; should we just set the SPIRV version and Vulkan environment based on that? My understanding is that they're somewhat independent features: there might be differences between environments which we want to respect even if they're not captured by the features of the specific device we're using. If we're feeding the SPIR-V code to OpenGL vs Vulkan we might need to generate different code even if it's going to run on the same GPU (thus with the same capabilities). In abstract, at least; I don't know whether we already have an instance of that, but it makes sense for the API to be ready. Sure, but we already have the GL vs Vulkan environment. Is there actually a meaningful difference between Vulkan 1.0 and 1.1 that's not going to end up being an extension or feature bit? And, if we don't know of one yet, do we need to add this extra API switch yet?
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/776#note_67563
should we just set the SPIRV version and Vulkan environment based on that?
Do you mean, make VULKAN_1_1 be a kind of proxy for wave ops support, i.e. the client checks `VkPhysicalDeviceSubgroupProperties` for wave op support, and uses VULKAN_1_1 only if they are available? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/776#note_67570
On Thu Apr 11 00:03:11 2024 +0000, Conor McCarthy wrote:
should we just set the SPIRV version and Vulkan environment based on that? Do you mean, make VULKAN_1_1 be a kind of proxy for wave ops support, i.e. the client checks `VkPhysicalDeviceSubgroupProperties` for wave op support, and uses VULKAN_1_1 only if they are available? I think the other way around; we keep VKD3D_SHADER_COMPILE_OPTION_FEATURE_WAVE_OPS but remove VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_1, and wherever we check the environment in the code we check for wave ops instead.
Not a big deal though and I can see just keep it around for some measure of consistency. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/776#note_67571
Sure, but we already have the GL vs Vulkan environment. Is there actually a meaningful difference between Vulkan 1.0 and 1.1 that's not going to end up being an extension or feature bit? And, if we don't know of one yet, do we need to add this extra API switch yet?
I don't understand, are we adding an API switch? It seems that we're only adding a case to an API that already exists (and is the one that enables the SPIR-V writer to distinguish between OpenGL and Vulkan 1.0; now we also have Vulkan 1.1). My feeling is that since SPIR-V already has a concept of "environment" we should stick to that one instead of inventing our own, so that anybody already knowing SPIR-V finds it a familiar concept instead of having to learn something similar-but-not-completely-the-same which might be puzzling. Also, coming up with a different concept might mean that at some point the SPIR-V concept and ours diverge in some sense that requires us to change our API to something more unwieldy. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/776#note_67581
Should there be a rule mandating than when you're compiling with this flag you should also specify `VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_1` as the target environment?
I don't think we need to explicitly enforce this in libvkd3d-shader, but it's probably worth pointing out in the documentation. I don't think the MR does a great job explaining the issue it addresses, so I spent a bunch of time read through the various specs, but for reference: - Vulkan 1.0 supports SPIR-V 1.0. - Vulkan 1.1 supports SPIR-V 1.0, 1.1, 1.2, and 1.3. - GL_ARB_gl_spirv supports SPIR-V 1.0. - SPIR-V 1.3 doesn't incorporate any of the existing vkd3d_shader_spirv_extension extensions. - There exist extensions like SPV_KHR_shader_ballot that bring some subgroup/wave operations to SPIR-V 1.0, but (AFAIK) not for all of the operations required for D3D shader model 6 wave operations. That requires SPIR-V 1.3 along with the corresponding VkPhysicalDeviceSubgroupProperties features, and by implication Vulkan 1.1. The implication is that in theory you could set VKD3D_SHADER_COMPILE_OPTION_FEATURE_WAVE_OPS with VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_0, but that wouldn't allow translation of sm6 waveops because we wouldn't be able to use SPIR-V 1.3. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/776#note_67603
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/776
There exist extensions like SPV_KHR_shader_ballot that bring some subgroup/wave operations to SPIR-V 1.0, but (AFAIK) not for all of the operations required for D3D shader model 6 wave operations.
That was my conclusion. The extensions don't supply equivalents for all of the DX intrinsics needed for 6.0 support. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/776#note_67618
participants (5)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)
-
Zebediah Figura (＠zfigura)