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

[Juan A. Suarez Romero]

On Tue, 2017-08-29 at 09:48 +0300, Tapani Pälli wrote:
> 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().
> 

Yes. The v2 patch already covers this case (using UINT32_MAX instead).
It's the last part of the patch

> 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?
> 
> 

AFAIK, that number is to define arrays of 256 anv_pipeline_bind_map. It
is not related with the fact of the datatype for the internal field of
the map.


	J.A.

> 
> > ---
> >   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;
> >      }
> >   
> > 
> 
>