[Mesa-dev] [PATCH 1/2] radv: don't overwrite results in VkGetQueryPoolResults() when queries are not available

[Samuel Pitoiset]

On 3/22/19 1:03 PM, Samuel Iglesias Gonsálvez wrote:
> If the query is not available and VK_QUERY_RESULT_WAIT_BIT and
> VK_QUERY_RESULT_PARTIAL_BIT are both not set, the spec doesn't
> allow to modify its result.
>
>  From Vulkan spec:
>
> "If VK_QUERY_RESULT_WAIT_BIT and VK_QUERY_RESULT_PARTIAL_BIT are
> both not set then no result values are written to pData for queries
> that are in the unavailable state at the time of the call, and
> vkGetQueryPoolResults returns VK_NOT_READY. However, availability state
> is still written to pData for those queries
> if VK_QUERY_RESULT_WITH_AVAILABILITY_BIT is set."
>
> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> ---
>   src/amd/vulkan/radv_query.c | 52 +++++++++++++++++++++++--------------
>   1 file changed, 33 insertions(+), 19 deletions(-)
>
> diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c
> index e808aa65170..8578680f09d 100644
> --- a/src/amd/vulkan/radv_query.c
> +++ b/src/amd/vulkan/radv_query.c
> @@ -1148,10 +1148,12 @@ VkResult radv_GetQueryPoolResults(
>   			}
>   
>   			if (flags & VK_QUERY_RESULT_64_BIT) {
> -				*(uint64_t*)dest = *(uint64_t*)src;
> +				if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT))
> +					*(uint64_t*)dest = *(uint64_t*)src;
>   				dest += 8;
>   			} else {
> -				*(uint32_t*)dest = *(uint32_t*)src;
> +				if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT))
> +					*(uint32_t*)dest = *(uint32_t*)src;
>   				dest += 4;
>   			}
>   			break;
> @@ -1183,10 +1185,12 @@ VkResult radv_GetQueryPoolResults(
>   			}
>   
>   			if (flags & VK_QUERY_RESULT_64_BIT) {
> -				*(uint64_t*)dest = sample_count;
> +				if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT))
> +					*(uint64_t*)dest = sample_count;
>   				dest += 8;
>   			} else {
> -				*(uint32_t*)dest = sample_count;
> +				if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT))
> +					*(uint32_t*)dest = sample_count;
>   				dest += 4;
>   			}
>   			break;
> @@ -1203,18 +1207,26 @@ VkResult radv_GetQueryPoolResults(
>   			if (flags & VK_QUERY_RESULT_64_BIT) {
>   				uint64_t *dst = (uint64_t*)dest;
>   				dest += util_bitcount(pool->pipeline_stats_mask) * 8;
> -				for(int i = 0; i < 11; ++i)
> -					if(pool->pipeline_stats_mask & (1u << i))
> -						*dst++ = stop[pipeline_statistics_indices[i]] -
> -						         start[pipeline_statistics_indices[i]];
> +				for(int i = 0; i < 11; ++i) {
> +					if(pool->pipeline_stats_mask & (1u << i)) {
> +						if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT))
> +							*dst = stop[pipeline_statistics_indices[i]] -
> +							       start[pipeline_statistics_indices[i]];
> +						dst++;
> +					}
> +				}
>   
>   			} else {
>   				uint32_t *dst = (uint32_t*)dest;
>   				dest += util_bitcount(pool->pipeline_stats_mask) * 4;
> -				for(int i = 0; i < 11; ++i)
> -					if(pool->pipeline_stats_mask & (1u << i))
> -						*dst++ = stop[pipeline_statistics_indices[i]] -
> -						         start[pipeline_statistics_indices[i]];
> +				for(int i = 0; i < 11; ++i) {
> +					if(pool->pipeline_stats_mask & (1u << i)) {
> +						if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT))
> +							*dst = stop[pipeline_statistics_indices[i]] -
> +							       start[pipeline_statistics_indices[i]];
> +						dst++;
> +					}
> +				}
>   			}
>   			break;
>   		}
> @@ -1235,23 +1247,25 @@ VkResult radv_GetQueryPoolResults(
>   					available = 0;
>   			}
>   
> -			if (!available && !(flags & VK_QUERY_RESULT_PARTIAL_BIT)) {
> +			if (!available && !(flags & VK_QUERY_RESULT_PARTIAL_BIT))
>   				result = VK_NOT_READY;
> -				break;
> -			}

Removing that break is useless?

>   
>   			num_primitives_written = src64[3] - src64[1];
>   			primitive_storage_needed = src64[2] - src64[0];
>   
>   			if (flags & VK_QUERY_RESULT_64_BIT) {
> -				*(uint64_t *)dest = num_primitives_written;
> +				if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT))
> +					*(uint64_t *)dest = num_primitives_written;
>   				dest += 8;
> -				*(uint64_t *)dest = primitive_storage_needed;
> +				if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT))
> +					*(uint64_t *)dest = primitive_storage_needed;
>   				dest += 8;
>   			} else {
> -				*(uint32_t *)dest = num_primitives_written;
> +				if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT))
> +					*(uint32_t *)dest = num_primitives_written;
>   				dest += 4;
> -				*(uint32_t *)dest = primitive_storage_needed;
> +				if (available || (flags & VK_QUERY_RESULT_PARTIAL_BIT))
> +					*(uint32_t *)dest = primitive_storage_needed;
>   				dest += 4;
>   			}
>   			break;