[Mesa-dev] [PATCH 4/4] anv/pipeline: Properly cache prog_data::param

Timothy Arceri timothy.arceri at collabora.com
Wed Nov 2 04:16:59 UTC 2016


On Tue, 2016-11-01 at 20:09 -0700, Jason Ekstrand wrote:
> Before we were caching the prog data but we weren't doing anything
> with
> brw_stage_prog_data::param so anything with push constants wasn't
> getting
> cached properly.  This commit fixes that.
> 
> Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> ---
>  src/intel/vulkan/anv_pipeline.c       |  3 ++-
>  src/intel/vulkan/anv_pipeline_cache.c | 43
> ++++++++++++++++++++++++++---------
>  src/intel/vulkan/anv_private.h        |  4 ++--
>  3 files changed, 36 insertions(+), 14 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_pipeline.c
> b/src/intel/vulkan/anv_pipeline.c
> index d0bffc1..bdc2f01 100644
> --- a/src/intel/vulkan/anv_pipeline.c
> +++ b/src/intel/vulkan/anv_pipeline.c
> @@ -400,7 +400,8 @@ anv_pipeline_upload_kernel(struct anv_pipeline
> *pipeline,
>     } else {
>        return anv_shader_bin_create(pipeline->device, key_data,
> key_size,
>                                     kernel_data, kernel_size,
> -                                   prog_data, prog_data_size,
> bind_map);
> +                                   prog_data, prog_data_size,
> +                                   prog_data->param, bind_map);
>     }
>  }
>  
> diff --git a/src/intel/vulkan/anv_pipeline_cache.c
> b/src/intel/vulkan/anv_pipeline_cache.c
> index 9fd12a1..ff6e651 100644
> --- a/src/intel/vulkan/anv_pipeline_cache.c
> +++ b/src/intel/vulkan/anv_pipeline_cache.c
> @@ -27,7 +27,8 @@
>  #include "anv_private.h"
>  
>  static size_t
> -anv_shader_bin_size(uint32_t prog_data_size, uint32_t key_size,
> +anv_shader_bin_size(uint32_t prog_data_size, uint32_t nr_params,
> +                    uint32_t key_size,
>                      uint32_t surface_count, uint32_t sampler_count)
>  {
>     const uint32_t binding_data_size =
> @@ -35,6 +36,7 @@ anv_shader_bin_size(uint32_t prog_data_size,
> uint32_t key_size,
>  
>     return align_u32(sizeof(struct anv_shader_bin), 8) +
>            align_u32(prog_data_size, 8) +
> +          align_u32(nr_params * sizeof(void *), 8) +
>            align_u32(sizeof(uint32_t) + key_size, 8) +
>            align_u32(binding_data_size, 8);
>  }
> @@ -44,11 +46,11 @@ anv_shader_bin_create(struct anv_device *device,
>                        const void *key_data, uint32_t key_size,
>                        const void *kernel_data, uint32_t kernel_size,
>                        const struct brw_stage_prog_data *prog_data,
> -                      uint32_t prog_data_size,
> +                      uint32_t prog_data_size, const void
> *prog_data_param,
>                        const struct anv_pipeline_bind_map *bind_map)
>  {
>     const size_t size =
> -      anv_shader_bin_size(prog_data_size, key_size,
> +      anv_shader_bin_size(prog_data_size, prog_data->nr_params,
> key_size,
>                            bind_map->surface_count, bind_map-
> >sampler_count);
>  
>     struct anv_shader_bin *shader =
> @@ -70,9 +72,17 @@ anv_shader_bin_create(struct anv_device *device,
>     data += align_u32(sizeof(struct anv_shader_bin), 8);
>  
>     shader->prog_data = data;
> +   struct brw_stage_prog_data *new_prog_data = data;
>     memcpy(data, prog_data, prog_data_size);
>     data += align_u32(prog_data_size, 8);
>  
> +   assert(prog_data->nr_pull_params == 0);
> +   assert(prog_data->nr_image_params == 0);
> +   new_prog_data->param = data;

I think it would be easier to follow if this were just:

   shader->prog_data->param = data;

Otherwise this series is:

Reviewed-by: Timothy Arceri <timothy.arceri at collabora.com>

> +   uint32_t param_size = prog_data->nr_params * sizeof(void *);
> +   memcpy(data, prog_data_param, param_size);
> +   data += align_u32(param_size, 8);
> +
>     shader->key = data;
>     struct anv_shader_bin_key *key = data;
>     key->size = key_size;
> @@ -104,7 +114,7 @@ static size_t
>  anv_shader_bin_data_size(const struct anv_shader_bin *shader)
>  {
>     return anv_shader_bin_size(shader->prog_data_size,
> -                              shader->key->size,
> +                              shader->prog_data->nr_params, shader-
> >key->size,
>                                shader->bind_map.surface_count,
>                                shader->bind_map.sampler_count) +
>            align_u32(shader->kernel_size, 8);
> @@ -115,7 +125,7 @@ anv_shader_bin_write_data(const struct
> anv_shader_bin *shader, void *data)
>  {
>     size_t struct_size =
>        anv_shader_bin_size(shader->prog_data_size,
> -                          shader->key->size,
> +                          shader->prog_data->nr_params, shader->key-
> >size,
>                            shader->bind_map.surface_count,
>                            shader->bind_map.sampler_count);
>  
> @@ -255,7 +265,9 @@ static struct anv_shader_bin *
>  anv_pipeline_cache_add_shader(struct anv_pipeline_cache *cache,
>                                const void *key_data, uint32_t
> key_size,
>                                const void *kernel_data, uint32_t
> kernel_size,
> -                              const void *prog_data, uint32_t
> prog_data_size,
> +                              const struct brw_stage_prog_data
> *prog_data,
> +                              uint32_t prog_data_size,
> +                              const void *prog_data_param,
>                                const struct anv_pipeline_bind_map
> *bind_map)
>  {
>     struct anv_shader_bin *shader =
> @@ -266,7 +278,8 @@ anv_pipeline_cache_add_shader(struct
> anv_pipeline_cache *cache,
>     struct anv_shader_bin *bin =
>        anv_shader_bin_create(cache->device, key_data, key_size,
>                              kernel_data, kernel_size,
> -                            prog_data, prog_data_size, bind_map);
> +                            prog_data, prog_data_size,
> prog_data_param,
> +                            bind_map);
>     if (!bin)
>        return NULL;
>  
> @@ -289,7 +302,8 @@ anv_pipeline_cache_upload_kernel(struct
> anv_pipeline_cache *cache,
>        struct anv_shader_bin *bin =
>           anv_pipeline_cache_add_shader(cache, key_data, key_size,
>                                         kernel_data, kernel_size,
> -                                       prog_data, prog_data_size,
> bind_map);
> +                                       prog_data, prog_data_size,
> +                                       prog_data->param, bind_map);
>  
>        pthread_mutex_unlock(&cache->mutex);
>  
> @@ -301,7 +315,8 @@ anv_pipeline_cache_upload_kernel(struct
> anv_pipeline_cache *cache,
>        /* In this case, we're not caching it so the caller owns it
> entirely */
>        return anv_shader_bin_create(cache->device, key_data,
> key_size,
>                                     kernel_data, kernel_size,
> -                                   prog_data, prog_data_size,
> bind_map);
> +                                   prog_data, prog_data_size,
> +                                   prog_data->param, bind_map);
>     }
>  }
>  
> @@ -356,8 +371,14 @@ anv_pipeline_cache_load(struct
> anv_pipeline_cache *cache,
>        memcpy(&bin, p, sizeof(bin));
>        p += align_u32(sizeof(struct anv_shader_bin), 8);
>  
> -      const void *prog_data = p;
> +      const struct brw_stage_prog_data *prog_data = p;
>        p += align_u32(bin.prog_data_size, 8);
> +      if (p > end)
> +         break;
> +
> +      uint32_t param_size = prog_data->nr_params * sizeof(void *);
> +      const void *prog_data_param = p;
> +      p += align_u32(param_size, 8);
>  
>        struct anv_shader_bin_key key;
>        if (p + sizeof(key) > end)
> @@ -382,7 +403,7 @@ anv_pipeline_cache_load(struct anv_pipeline_cache
> *cache,
>        anv_pipeline_cache_add_shader(cache, key_data, key.size,
>                                      kernel_data, bin.kernel_size,
>                                      prog_data, bin.prog_data_size,
> -                                    &bin.bind_map);
> +                                    prog_data_param, &bin.bind_map);
>     }
>  }
>  
> diff --git a/src/intel/vulkan/anv_private.h
> b/src/intel/vulkan/anv_private.h
> index 79f7c14..8f5a95b 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -1316,7 +1316,7 @@ struct anv_shader_bin {
>  
>     struct anv_pipeline_bind_map bind_map;
>  
> -   /* Prog data follows, then the key, both aligned to 8-bytes */
> +   /* Prog data follows, then params, then the key, all aligned to
> 8-bytes */
>  };
>  
>  struct anv_shader_bin *
> @@ -1324,7 +1324,7 @@ anv_shader_bin_create(struct anv_device
> *device,
>                        const void *key, uint32_t key_size,
>                        const void *kernel, uint32_t kernel_size,
>                        const struct brw_stage_prog_data *prog_data,
> -                      uint32_t prog_data_size,
> +                      uint32_t prog_data_size, const void
> *prog_data_param,
>                        const struct anv_pipeline_bind_map *bind_map);
>  
>  void


More information about the mesa-dev mailing list