[Mesa-dev] [PATCH v2] anv: set right datatypes in anv_pipeline_binding

[Tapani Pälli]

Hi;

On 08/25/2017 06:32 PM, Juan A. Suarez Romero wrote:
> This structure contains two fields, binding and index, that store the
> binding in the descriptor set and the index inside the binding.
> 
> These structures are defined as uint8_t, but the types in Vulkan
> specification are uint32_t, so big values are clamp.
> 
> This fixes dEQP-VK.binding_model.shader_access.*.multiple_arbitrary_descriptors.*
> 
> v2: use UINT32_MAX for index when having no render targets (Tapani)

Sorry, I think I did not catch everything here. There's UINT8_MAX in use 
also when checking index in has_color_buffer_write_enabled().

Then .. in many places where anv_pipeline_bind_map is used, there is 
magic number 256 in use with surface to descriptor and sampler to 
descriptor mappings, are these related or where does 256 come from?



> ---
>   src/intel/vulkan/anv_pipeline.c  | 2 +-
>   src/intel/vulkan/anv_private.h   | 4 ++--
>   src/intel/vulkan/genX_pipeline.c | 2 +-
>   3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c
> index 279d76561a..13ee5f47e2 100644
> --- a/src/intel/vulkan/anv_pipeline.c
> +++ b/src/intel/vulkan/anv_pipeline.c
> @@ -907,7 +907,7 @@ anv_pipeline_compile_fs(struct anv_pipeline *pipeline,
>            rt_bindings[0] = (struct anv_pipeline_binding) {
>               .set = ANV_DESCRIPTOR_SET_COLOR_ATTACHMENTS,
>               .binding = 0,
> -            .index = UINT8_MAX,
> +            .index = UINT32_MAX,
>            };
>            num_rts = 1;
>         }
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> index 6b2414429f..d5657c5e09 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -1236,10 +1236,10 @@ struct anv_pipeline_binding {
>      uint8_t set;
>   
>      /* Binding in the descriptor set */
> -   uint8_t binding;
> +   uint32_t binding;
>   
>      /* Index in the binding */
> -   uint8_t index;
> +   uint32_t index;
>   
>      /* Input attachment index (relative to the subpass) */
>      uint8_t input_attachment_index;
> diff --git a/src/intel/vulkan/genX_pipeline.c b/src/intel/vulkan/genX_pipeline.c
> index 55db5339d6..acf3ee37d3 100644
> --- a/src/intel/vulkan/genX_pipeline.c
> +++ b/src/intel/vulkan/genX_pipeline.c
> @@ -1346,7 +1346,7 @@ has_color_buffer_write_enabled(const struct anv_pipeline *pipeline)
>         if (bind_map->surface_to_descriptor[i].set !=
>             ANV_DESCRIPTOR_SET_COLOR_ATTACHMENTS)
>            continue;
> -      if (bind_map->surface_to_descriptor[i].index != UINT8_MAX)
> +      if (bind_map->surface_to_descriptor[i].index != UINT32_MAX)
>            return true;
>      }
>   
>