From 1e0a69afa72c61e5f5841db3e5e7f6bb846a0fab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Pi=C3=B1eiro?= Date: Mon, 29 Mar 2021 12:10:46 +0200 Subject: [PATCH] vulkan: track number of bindings instead of max binding for CreateDescriptorSetLayout As that handles better, and more clear, the case of bindingCount being zero. For the case of Anvil and Turnip, this avoids allocating a non-needed binding when bindingCount is zero. Inspired on radv, that was what it was doing so far. Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/4526 Reviewed-by: Mike Blumenkrantz Reviewed-by: Iago Toral Quiroga Reviewed-by: Lionel Landwerlin Reviewed-by: Hyunjun Ko Part-of: --- src/broadcom/vulkan/v3dv_descriptor_set.c | 12 ++++++------ src/freedreno/vulkan/tu_descriptor_set.c | 12 ++++++------ src/gallium/frontends/lavapipe/lvp_descriptor_set.c | 10 +++++----- src/intel/vulkan/anv_descriptor_set.c | 12 ++++++------ 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/broadcom/vulkan/v3dv_descriptor_set.c b/src/broadcom/vulkan/v3dv_descriptor_set.c index 60a5a609e38..3487d701a04 100644 --- a/src/broadcom/vulkan/v3dv_descriptor_set.c +++ b/src/broadcom/vulkan/v3dv_descriptor_set.c @@ -569,10 +569,10 @@ v3dv_CreateDescriptorSetLayout(VkDevice _device, assert(pCreateInfo->sType == VK_STRUCTURE_TYPE_DESCRIPTOR_SET_LAYOUT_CREATE_INFO); - int32_t max_binding = pCreateInfo->bindingCount > 0 ? 0 : -1; + uint32_t num_bindings = 0; uint32_t immutable_sampler_count = 0; for (uint32_t j = 0; j < pCreateInfo->bindingCount; j++) { - max_binding = MAX2(max_binding, pCreateInfo->pBindings[j].binding); + num_bindings = MAX2(num_bindings, pCreateInfo->pBindings[j].binding + 1); /* From the Vulkan 1.1.97 spec for VkDescriptorSetLayoutBinding: * @@ -594,7 +594,7 @@ v3dv_CreateDescriptorSetLayout(VkDevice _device, } uint32_t samplers_offset = sizeof(struct v3dv_descriptor_set_layout) + - (max_binding + 1) * sizeof(set_layout->binding[0]); + num_bindings * sizeof(set_layout->binding[0]); uint32_t size = samplers_offset + immutable_sampler_count * sizeof(struct v3dv_sampler); @@ -605,9 +605,9 @@ v3dv_CreateDescriptorSetLayout(VkDevice _device, return vk_error(device->instance, VK_ERROR_OUT_OF_HOST_MEMORY); /* We just allocate all the immutable samplers at the end of the struct */ - struct v3dv_sampler *samplers = (void*) &set_layout->binding[max_binding + 1]; + struct v3dv_sampler *samplers = (void*) &set_layout->binding[num_bindings]; - assert(pCreateInfo->bindingCount == 0 || max_binding >= 0); + assert(pCreateInfo->bindingCount == 0 || num_bindings > 0); VkDescriptorSetLayoutBinding *bindings = NULL; VkResult result = vk_create_sorted_bindings(pCreateInfo->pBindings, @@ -620,7 +620,7 @@ v3dv_CreateDescriptorSetLayout(VkDevice _device, memset(set_layout->binding, 0, size - sizeof(struct v3dv_descriptor_set_layout)); - set_layout->binding_count = max_binding + 1; + set_layout->binding_count = num_bindings; set_layout->flags = pCreateInfo->flags; set_layout->shader_stages = 0; set_layout->bo_size = 0; diff --git a/src/freedreno/vulkan/tu_descriptor_set.c b/src/freedreno/vulkan/tu_descriptor_set.c index e906cb2c95e..46f8cc24b34 100644 --- a/src/freedreno/vulkan/tu_descriptor_set.c +++ b/src/freedreno/vulkan/tu_descriptor_set.c @@ -90,11 +90,11 @@ tu_CreateDescriptorSetLayout( pCreateInfo->pNext, DESCRIPTOR_SET_LAYOUT_BINDING_FLAGS_CREATE_INFO_EXT); - uint32_t max_binding = 0; + uint32_t num_bindings = 0; uint32_t immutable_sampler_count = 0; uint32_t ycbcr_sampler_count = 0; for (uint32_t j = 0; j < pCreateInfo->bindingCount; j++) { - max_binding = MAX2(max_binding, pCreateInfo->pBindings[j].binding); + num_bindings = MAX2(num_bindings, pCreateInfo->pBindings[j].binding + 1); if ((pCreateInfo->pBindings[j].descriptorType == VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER || pCreateInfo->pBindings[j].descriptorType == VK_DESCRIPTOR_TYPE_SAMPLER) && pCreateInfo->pBindings[j].pImmutableSamplers) { @@ -112,7 +112,7 @@ tu_CreateDescriptorSetLayout( } uint32_t samplers_offset = - offsetof(struct tu_descriptor_set_layout, binding[max_binding + 1]); + offsetof(struct tu_descriptor_set_layout, binding[num_bindings]); /* note: only need to store TEX_SAMP_DWORDS for immutable samples, * but using struct tu_sampler makes things simpler */ @@ -128,7 +128,7 @@ tu_CreateDescriptorSetLayout( set_layout->flags = pCreateInfo->flags; /* We just allocate all the immutable samplers at the end of the struct */ - struct tu_sampler *samplers = (void*) &set_layout->binding[max_binding + 1]; + struct tu_sampler *samplers = (void*) &set_layout->binding[num_bindings]; struct tu_sampler_ycbcr_conversion *ycbcr_samplers = (void*) &samplers[immutable_sampler_count]; @@ -140,7 +140,7 @@ tu_CreateDescriptorSetLayout( return vk_error(device->instance, result); } - set_layout->binding_count = max_binding + 1; + set_layout->binding_count = num_bindings; set_layout->shader_stages = 0; set_layout->has_immutable_samplers = false; set_layout->size = 0; @@ -164,7 +164,7 @@ tu_CreateDescriptorSetLayout( VK_DESCRIPTOR_BINDING_VARIABLE_DESCRIPTOR_COUNT_BIT_EXT)) { assert(!binding->pImmutableSamplers); /* Terribly ill defined how many samplers are valid */ - assert(binding->binding == max_binding); + assert(binding->binding == num_bindings - 1); set_layout->has_variable_descriptors = true; } diff --git a/src/gallium/frontends/lavapipe/lvp_descriptor_set.c b/src/gallium/frontends/lavapipe/lvp_descriptor_set.c index 57b1fea5fb6..d8844031b68 100644 --- a/src/gallium/frontends/lavapipe/lvp_descriptor_set.c +++ b/src/gallium/frontends/lavapipe/lvp_descriptor_set.c @@ -36,10 +36,10 @@ VKAPI_ATTR VkResult VKAPI_CALL lvp_CreateDescriptorSetLayout( struct lvp_descriptor_set_layout *set_layout; assert(pCreateInfo->sType == VK_STRUCTURE_TYPE_DESCRIPTOR_SET_LAYOUT_CREATE_INFO); - uint32_t max_binding = 0; + uint32_t num_bindings = 0; uint32_t immutable_sampler_count = 0; for (uint32_t j = 0; j < pCreateInfo->bindingCount; j++) { - max_binding = MAX2(max_binding, pCreateInfo->pBindings[j].binding); + num_bindings = MAX2(num_bindings, pCreateInfo->pBindings[j].binding + 1); /* From the Vulkan 1.1.97 spec for VkDescriptorSetLayoutBinding: * * "If descriptorType specifies a VK_DESCRIPTOR_TYPE_SAMPLER or @@ -59,7 +59,7 @@ VKAPI_ATTR VkResult VKAPI_CALL lvp_CreateDescriptorSetLayout( } size_t size = sizeof(struct lvp_descriptor_set_layout) + - (max_binding + 1) * sizeof(set_layout->binding[0]) + + num_bindings * sizeof(set_layout->binding[0]) + immutable_sampler_count * sizeof(struct lvp_sampler *); set_layout = vk_zalloc2(&device->vk.alloc, pAllocator, size, 8, @@ -72,10 +72,10 @@ VKAPI_ATTR VkResult VKAPI_CALL lvp_CreateDescriptorSetLayout( set_layout->ref_cnt = 1; /* We just allocate all the samplers at the end of the struct */ struct lvp_sampler **samplers = - (struct lvp_sampler **)&set_layout->binding[max_binding + 1]; + (struct lvp_sampler **)&set_layout->binding[num_bindings]; set_layout->alloc = pAllocator; - set_layout->binding_count = max_binding + 1; + set_layout->binding_count = num_bindings; set_layout->shader_stages = 0; set_layout->size = 0; diff --git a/src/intel/vulkan/anv_descriptor_set.c b/src/intel/vulkan/anv_descriptor_set.c index f59608cd921..e8995a6c312 100644 --- a/src/intel/vulkan/anv_descriptor_set.c +++ b/src/intel/vulkan/anv_descriptor_set.c @@ -351,10 +351,10 @@ VkResult anv_CreateDescriptorSetLayout( assert(pCreateInfo->sType == VK_STRUCTURE_TYPE_DESCRIPTOR_SET_LAYOUT_CREATE_INFO); - uint32_t max_binding = 0; + uint32_t num_bindings = 0; uint32_t immutable_sampler_count = 0; for (uint32_t j = 0; j < pCreateInfo->bindingCount; j++) { - max_binding = MAX2(max_binding, pCreateInfo->pBindings[j].binding); + num_bindings = MAX2(num_bindings, pCreateInfo->pBindings[j].binding + 1); /* From the Vulkan 1.1.97 spec for VkDescriptorSetLayoutBinding: * @@ -381,7 +381,7 @@ VkResult anv_CreateDescriptorSetLayout( VK_MULTIALLOC(ma); VK_MULTIALLOC_DECL(&ma, struct anv_descriptor_set_layout, set_layout, 1); VK_MULTIALLOC_DECL(&ma, struct anv_descriptor_set_binding_layout, - bindings, max_binding + 1); + bindings, num_bindings); VK_MULTIALLOC_DECL(&ma, struct anv_sampler *, samplers, immutable_sampler_count); @@ -393,9 +393,9 @@ VkResult anv_CreateDescriptorSetLayout( vk_object_base_init(&device->vk, &set_layout->base, VK_OBJECT_TYPE_DESCRIPTOR_SET_LAYOUT); set_layout->ref_cnt = 1; - set_layout->binding_count = max_binding + 1; + set_layout->binding_count = num_bindings; - for (uint32_t b = 0; b <= max_binding; b++) { + for (uint32_t b = 0; b < num_bindings; b++) { /* Initialize all binding_layout entries to -1 */ memset(&set_layout->binding[b], -1, sizeof(set_layout->binding[b])); @@ -427,7 +427,7 @@ VkResult anv_CreateDescriptorSetLayout( vk_find_struct_const(pCreateInfo->pNext, DESCRIPTOR_SET_LAYOUT_BINDING_FLAGS_CREATE_INFO_EXT); - for (uint32_t b = 0; b <= max_binding; b++) { + for (uint32_t b = 0; b < num_bindings; b++) { /* We stashed the pCreateInfo->pBindings[] index (plus one) in the * immutable_samplers pointer. Check for NULL (empty binding) and then * reset it and compute the index.