[Mesa-dev] [PATCH 3/4] anv/pipeline: Put actual pointers in anv_shader_bin

Pohjolainen, Topi topi.pohjolainen at gmail.com
Wed Nov 2 08:16:08 UTC 2016


On Tue, Nov 01, 2016 at 08:09:12PM -0700, Jason Ekstrand wrote:
> While we can simply calculate offsets to get to things such as the
> prog_data and the key, it's much more user-friendly if there are just
> pointers.  Also, it's a bit more fool-proof.
> 
> While we're at it, we rework the pipeline cache API to use the
> brw_stage_prog_data type directly.

Clearer:

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>

> 
> Signed-off-by: Jason Ekstrand <jason at jlekstrand.net>
> ---
>  src/intel/vulkan/anv_blorp.c          |  6 ++---
>  src/intel/vulkan/anv_cmd_buffer.c     |  2 +-
>  src/intel/vulkan/anv_pipeline.c       | 17 +++++++++-----
>  src/intel/vulkan/anv_pipeline_cache.c | 42 +++++++++++++----------------------
>  src/intel/vulkan/anv_private.h        | 28 ++++++++++++-----------
>  5 files changed, 45 insertions(+), 50 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_blorp.c b/src/intel/vulkan/anv_blorp.c
> index 79153a3..87f242c 100644
> --- a/src/intel/vulkan/anv_blorp.c
> +++ b/src/intel/vulkan/anv_blorp.c
> @@ -44,8 +44,7 @@ lookup_blorp_shader(struct blorp_context *blorp,
>     anv_shader_bin_unref(device, bin);
>  
>     *kernel_out = bin->kernel.offset;
> -   *(const struct brw_stage_prog_data **)prog_data_out =
> -      anv_shader_bin_get_prog_data(bin);
> +   *(const struct brw_stage_prog_data **)prog_data_out = bin->prog_data;
>  
>     return true;
>  }
> @@ -79,8 +78,7 @@ upload_blorp_shader(struct blorp_context *blorp,
>     anv_shader_bin_unref(device, bin);
>  
>     *kernel_out = bin->kernel.offset;
> -   *(const struct brw_stage_prog_data **)prog_data_out =
> -      anv_shader_bin_get_prog_data(bin);
> +   *(const struct brw_stage_prog_data **)prog_data_out = bin->prog_data;
>  }
>  
>  void
> diff --git a/src/intel/vulkan/anv_cmd_buffer.c b/src/intel/vulkan/anv_cmd_buffer.c
> index a652f9a..7ff7dba 100644
> --- a/src/intel/vulkan/anv_cmd_buffer.c
> +++ b/src/intel/vulkan/anv_cmd_buffer.c
> @@ -658,7 +658,7 @@ anv_cmd_buffer_push_constants(struct anv_cmd_buffer *cmd_buffer,
>     struct anv_push_constants *data =
>        cmd_buffer->state.push_constants[stage];
>     const struct brw_stage_prog_data *prog_data =
> -      anv_shader_bin_get_prog_data(cmd_buffer->state.pipeline->shaders[stage]);
> +      cmd_buffer->state.pipeline->shaders[stage]->prog_data;
>  
>     /* If we don't actually have any push constants, bail. */
>     if (data == NULL || prog_data == NULL || prog_data->nr_params == 0)
> diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c
> index 0aac711..d0bffc1 100644
> --- a/src/intel/vulkan/anv_pipeline.c
> +++ b/src/intel/vulkan/anv_pipeline.c
> @@ -388,7 +388,8 @@ anv_pipeline_upload_kernel(struct anv_pipeline *pipeline,
>                             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 struct anv_pipeline_bind_map *bind_map)
>  {
>     if (cache) {
> @@ -476,7 +477,8 @@ anv_pipeline_compile_vs(struct anv_pipeline *pipeline,
>  
>        bin = anv_pipeline_upload_kernel(pipeline, cache, sha1, 20,
>                                         shader_code, code_size,
> -                                       &prog_data, sizeof(prog_data), &map);
> +                                       &prog_data.base.base, sizeof(prog_data),
> +                                       &map);
>        if (!bin) {
>           ralloc_free(mem_ctx);
>           return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> @@ -486,7 +488,7 @@ anv_pipeline_compile_vs(struct anv_pipeline *pipeline,
>     }
>  
>     const struct brw_vs_prog_data *vs_prog_data =
> -      (const struct brw_vs_prog_data *)anv_shader_bin_get_prog_data(bin);
> +      (const struct brw_vs_prog_data *)bin->prog_data;
>  
>     if (vs_prog_data->base.dispatch_mode == DISPATCH_MODE_SIMD8) {
>        pipeline->vs_simd8 = bin->kernel.offset;
> @@ -563,7 +565,8 @@ anv_pipeline_compile_gs(struct anv_pipeline *pipeline,
>        /* TODO: SIMD8 GS */
>        bin = anv_pipeline_upload_kernel(pipeline, cache, sha1, 20,
>                                         shader_code, code_size,
> -                                       &prog_data, sizeof(prog_data), &map);
> +                                       &prog_data.base.base, sizeof(prog_data),
> +                                       &map);
>        if (!bin) {
>           ralloc_free(mem_ctx);
>           return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> @@ -686,7 +689,8 @@ anv_pipeline_compile_fs(struct anv_pipeline *pipeline,
>  
>        bin = anv_pipeline_upload_kernel(pipeline, cache, sha1, 20,
>                                         shader_code, code_size,
> -                                       &prog_data, sizeof(prog_data), &map);
> +                                       &prog_data.base, sizeof(prog_data),
> +                                       &map);
>        if (!bin) {
>           ralloc_free(mem_ctx);
>           return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> @@ -758,7 +762,8 @@ anv_pipeline_compile_cs(struct anv_pipeline *pipeline,
>  
>        bin = anv_pipeline_upload_kernel(pipeline, cache, sha1, 20,
>                                         shader_code, code_size,
> -                                       &prog_data, sizeof(prog_data), &map);
> +                                       &prog_data.base, sizeof(prog_data),
> +                                       &map);
>        if (!bin) {
>           ralloc_free(mem_ctx);
>           return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY);
> diff --git a/src/intel/vulkan/anv_pipeline_cache.c b/src/intel/vulkan/anv_pipeline_cache.c
> index 79df315..9fd12a1 100644
> --- a/src/intel/vulkan/anv_pipeline_cache.c
> +++ b/src/intel/vulkan/anv_pipeline_cache.c
> @@ -26,11 +26,6 @@
>  #include "util/debug.h"
>  #include "anv_private.h"
>  
> -struct shader_bin_key {
> -   uint32_t size;
> -   uint8_t data[0];
> -};
> -
>  static size_t
>  anv_shader_bin_size(uint32_t prog_data_size, uint32_t key_size,
>                      uint32_t surface_count, uint32_t sampler_count)
> @@ -44,20 +39,12 @@ anv_shader_bin_size(uint32_t prog_data_size, uint32_t key_size,
>            align_u32(binding_data_size, 8);
>  }
>  
> -static inline const struct shader_bin_key *
> -anv_shader_bin_get_key(const struct anv_shader_bin *shader)
> -{
> -   const void *data = shader;
> -   data += align_u32(sizeof(struct anv_shader_bin), 8);
> -   data += align_u32(shader->prog_data_size, 8);
> -   return data;
> -}
> -
>  struct anv_shader_bin *
>  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 void *prog_data, uint32_t prog_data_size,
> +                      const struct brw_stage_prog_data *prog_data,
> +                      uint32_t prog_data_size,
>                        const struct anv_pipeline_bind_map *bind_map)
>  {
>     const size_t size =
> @@ -82,10 +69,12 @@ anv_shader_bin_create(struct anv_device *device,
>     void *data = shader;
>     data += align_u32(sizeof(struct anv_shader_bin), 8);
>  
> +   shader->prog_data = data;
>     memcpy(data, prog_data, prog_data_size);
>     data += align_u32(prog_data_size, 8);
>  
> -   struct shader_bin_key *key = data;
> +   shader->key = data;
> +   struct anv_shader_bin_key *key = data;
>     key->size = key_size;
>     memcpy(key->data, key_data, key_size);
>     data += align_u32(sizeof(*key) + key_size, 8);
> @@ -115,7 +104,7 @@ static size_t
>  anv_shader_bin_data_size(const struct anv_shader_bin *shader)
>  {
>     return anv_shader_bin_size(shader->prog_data_size,
> -                              anv_shader_bin_get_key(shader)->size,
> +                              shader->key->size,
>                                shader->bind_map.surface_count,
>                                shader->bind_map.sampler_count) +
>            align_u32(shader->kernel_size, 8);
> @@ -126,7 +115,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,
> -                          anv_shader_bin_get_key(shader)->size,
> +                          shader->key->size,
>                            shader->bind_map.surface_count,
>                            shader->bind_map.sampler_count);
>  
> @@ -151,14 +140,14 @@ anv_shader_bin_write_data(const struct anv_shader_bin *shader, void *data)
>  static uint32_t
>  shader_bin_key_hash_func(const void *void_key)
>  {
> -   const struct shader_bin_key *key = void_key;
> +   const struct anv_shader_bin_key *key = void_key;
>     return _mesa_hash_data(key->data, key->size);
>  }
>  
>  static bool
>  shader_bin_key_compare_func(const void *void_a, const void *void_b)
>  {
> -   const struct shader_bin_key *a = void_a, *b = void_b;
> +   const struct anv_shader_bin_key *a = void_a, *b = void_b;
>     if (a->size != b->size)
>        return false;
>  
> @@ -230,7 +219,7 @@ anv_pipeline_cache_search_locked(struct anv_pipeline_cache *cache,
>                                   const void *key_data, uint32_t key_size)
>  {
>     uint32_t vla[1 + DIV_ROUND_UP(key_size, sizeof(uint32_t))];
> -   struct shader_bin_key *key = (void *)vla;
> +   struct anv_shader_bin_key *key = (void *)vla;
>     key->size = key_size;
>     memcpy(key->data, key_data, key_size);
>  
> @@ -281,7 +270,7 @@ anv_pipeline_cache_add_shader(struct anv_pipeline_cache *cache,
>     if (!bin)
>        return NULL;
>  
> -   _mesa_hash_table_insert(cache->cache, anv_shader_bin_get_key(bin), bin);
> +   _mesa_hash_table_insert(cache->cache, bin->key, bin);
>  
>     return bin;
>  }
> @@ -290,7 +279,8 @@ struct anv_shader_bin *
>  anv_pipeline_cache_upload_kernel(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 struct anv_pipeline_bind_map *bind_map)
>  {
>     if (cache->cache) {
> @@ -369,7 +359,7 @@ anv_pipeline_cache_load(struct anv_pipeline_cache *cache,
>        const void *prog_data = p;
>        p += align_u32(bin.prog_data_size, 8);
>  
> -      struct shader_bin_key key;
> +      struct anv_shader_bin_key key;
>        if (p + sizeof(key) > end)
>           break;
>        memcpy(&key, p, sizeof(key));
> @@ -532,11 +522,11 @@ VkResult anv_MergePipelineCaches(
>        struct hash_entry *entry;
>        hash_table_foreach(src->cache, entry) {
>           struct anv_shader_bin *bin = entry->data;
> -         if (_mesa_hash_table_search(dst->cache, anv_shader_bin_get_key(bin)))
> +         if (_mesa_hash_table_search(dst->cache, bin->key))
>              continue;
>  
>           anv_shader_bin_ref(bin);
> -         _mesa_hash_table_insert(dst->cache, anv_shader_bin_get_key(bin), bin);
> +         _mesa_hash_table_insert(dst->cache, bin->key, bin);
>        }
>     }
>  
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> index 3fe9d7d..79f7c14 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -516,7 +516,8 @@ struct anv_shader_bin *
>  anv_pipeline_cache_upload_kernel(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 struct anv_pipeline_bind_map *bind_map);
>  
>  struct anv_device {
> @@ -1297,16 +1298,24 @@ struct anv_pipeline_bind_map {
>     struct anv_pipeline_binding *                sampler_to_descriptor;
>  };
>  
> +struct anv_shader_bin_key {
> +   uint32_t size;
> +   uint8_t data[0];
> +};
> +
>  struct anv_shader_bin {
>     uint32_t ref_cnt;
>  
> +   const struct anv_shader_bin_key *key;
> +
>     struct anv_state kernel;
>     uint32_t kernel_size;
>  
> -   struct anv_pipeline_bind_map bind_map;
> -
> +   const struct brw_stage_prog_data *prog_data;
>     uint32_t prog_data_size;
>  
> +   struct anv_pipeline_bind_map bind_map;
> +
>     /* Prog data follows, then the key, both aligned to 8-bytes */
>  };
>  
> @@ -1314,7 +1323,8 @@ struct anv_shader_bin *
>  anv_shader_bin_create(struct anv_device *device,
>                        const void *key, uint32_t key_size,
>                        const void *kernel, 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 struct anv_pipeline_bind_map *bind_map);
>  
>  void
> @@ -1335,14 +1345,6 @@ anv_shader_bin_unref(struct anv_device *device, struct anv_shader_bin *shader)
>        anv_shader_bin_destroy(device, shader);
>  }
>  
> -static inline const struct brw_stage_prog_data *
> -anv_shader_bin_get_prog_data(const struct anv_shader_bin *shader)
> -{
> -   const void *data = shader;
> -   data += align_u32(sizeof(struct anv_shader_bin), 8);
> -   return data;
> -}
> -
>  struct anv_pipeline {
>     struct anv_device *                          device;
>     struct anv_batch                             batch;
> @@ -1409,7 +1411,7 @@ get_##prefix##_prog_data(struct anv_pipeline *pipeline)              \
>  {                                                                    \
>     if (anv_pipeline_has_stage(pipeline, stage)) {                    \
>        return (const struct brw_##prefix##_prog_data *)               \
> -             anv_shader_bin_get_prog_data(pipeline->shaders[stage]); \
> +             pipeline->shaders[stage]->prog_data;                    \
>     } else {                                                          \
>        return NULL;                                                   \
>     }                                                                 \
> -- 
> 2.5.0.400.gff86faf
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list