[Mesa-dev] [PATCH 6/6] anv: add dispatch macro to find right function for given generation

Kenneth Graunke kenneth at whitecape.org
Mon Oct 17 15:51:09 UTC 2016


On Wednesday, October 12, 2016 11:28:04 PM PDT Lionel Landwerlin wrote:
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> ---
>  src/intel/vulkan/anv_cmd_buffer.c | 33 ++++++---------------------------
>  src/intel/vulkan/anv_device.c     | 19 +------------------
>  src/intel/vulkan/anv_pipeline.c   | 30 ++++--------------------------
>  src/intel/vulkan/anv_private.h    | 23 +++++++++++++++++++++++
>  4 files changed, 34 insertions(+), 71 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_cmd_buffer.c b/src/intel/vulkan/anv_cmd_buffer.c
> index 5bcd5e0..b051489 100644
> --- a/src/intel/vulkan/anv_cmd_buffer.c
> +++ b/src/intel/vulkan/anv_cmd_buffer.c
> @@ -356,19 +356,9 @@ VkResult anv_ResetCommandBuffer(
>  void
>  anv_cmd_buffer_emit_state_base_address(struct anv_cmd_buffer *cmd_buffer)
>  {
> -   switch (cmd_buffer->device->info.gen) {
> -   case 7:
> -      if (cmd_buffer->device->info.is_haswell)
> -         return gen75_cmd_buffer_emit_state_base_address(cmd_buffer);
> -      else
> -         return gen7_cmd_buffer_emit_state_base_address(cmd_buffer);
> -   case 8:
> -      return gen8_cmd_buffer_emit_state_base_address(cmd_buffer);
> -   case 9:
> -      return gen9_cmd_buffer_emit_state_base_address(cmd_buffer);
> -   default:
> -      unreachable("unsupported gen\n");
> -   }
> +   ANV_GEN_DISPATCH(cmd_buffer->device,
> +                    cmd_buffer_emit_state_base_address,
> +                    cmd_buffer);
>  }
>  
>  VkResult anv_BeginCommandBuffer(
> @@ -714,20 +704,9 @@ static struct anv_state
>  anv_cmd_buffer_alloc_null_surface_state(struct anv_cmd_buffer *cmd_buffer,
>                                          struct anv_framebuffer *fb)
>  {
> -   switch (cmd_buffer->device->info.gen) {
> -   case 7:
> -      if (cmd_buffer->device->info.is_haswell) {
> -         return gen75_cmd_buffer_alloc_null_surface_state(cmd_buffer, fb);
> -      } else {
> -         return gen7_cmd_buffer_alloc_null_surface_state(cmd_buffer, fb);
> -      }
> -   case 8:
> -      return gen8_cmd_buffer_alloc_null_surface_state(cmd_buffer, fb);
> -   case 9:
> -      return gen9_cmd_buffer_alloc_null_surface_state(cmd_buffer, fb);
> -   default:
> -      unreachable("Invalid hardware generation");
> -   }
> +   return ANV_GEN_DISPATCH(cmd_buffer->device,
> +                           cmd_buffer_alloc_null_surface_state,
> +                           cmd_buffer, fb);
>  }
>  
>  VkResult
> diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c
> index 24f7227..6dfdfdb 100644
> --- a/src/intel/vulkan/anv_device.c
> +++ b/src/intel/vulkan/anv_device.c
> @@ -921,24 +921,7 @@ VkResult anv_CreateDevice(
>  
>     anv_queue_init(device, &device->queue);
>  
> -   switch (device->info.gen) {
> -   case 7:
> -      if (!device->info.is_haswell)
> -         result = gen7_init_device_state(device);
> -      else
> -         result = gen75_init_device_state(device);
> -      break;
> -   case 8:
> -      result = gen8_init_device_state(device);
> -      break;
> -   case 9:
> -      result = gen9_init_device_state(device);
> -      break;
> -   default:
> -      /* Shouldn't get here as we don't create physical devices for any other
> -       * gens. */
> -      unreachable("unhandled gen");
> -   }
> +   result = ANV_GEN_DISPATCH(device, init_device_state, device);
>     if (result != VK_SUCCESS)
>        goto fail_fd;
>  
> diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c
> index 6b393a6..f9f1cbf 100644
> --- a/src/intel/vulkan/anv_pipeline.c
> +++ b/src/intel/vulkan/anv_pipeline.c
> @@ -1182,19 +1182,8 @@ anv_graphics_pipeline_create(
>     ANV_FROM_HANDLE(anv_device, device, _device);
>     ANV_FROM_HANDLE(anv_pipeline_cache, cache, _cache);
>  
> -   switch (device->info.gen) {
> -   case 7:
> -      if (device->info.is_haswell)
> -         return gen75_graphics_pipeline_create(_device, cache, pCreateInfo, extra, pAllocator, pPipeline);
> -      else
> -         return gen7_graphics_pipeline_create(_device, cache, pCreateInfo, extra, pAllocator, pPipeline);
> -   case 8:
> -      return gen8_graphics_pipeline_create(_device, cache, pCreateInfo, extra, pAllocator, pPipeline);
> -   case 9:
> -      return gen9_graphics_pipeline_create(_device, cache, pCreateInfo, extra, pAllocator, pPipeline);
> -   default:
> -      unreachable("unsupported gen\n");
> -   }
> +   return ANV_GEN_DISPATCH(device, graphics_pipeline_create,
> +                           _device, cache, pCreateInfo, extra, pAllocator, pPipeline);
>  }
>  
>  VkResult anv_CreateGraphicsPipelines(
> @@ -1235,19 +1224,8 @@ static VkResult anv_compute_pipeline_create(
>     ANV_FROM_HANDLE(anv_device, device, _device);
>     ANV_FROM_HANDLE(anv_pipeline_cache, cache, _cache);
>  
> -   switch (device->info.gen) {
> -   case 7:
> -      if (device->info.is_haswell)
> -         return gen75_compute_pipeline_create(_device, cache, pCreateInfo, pAllocator, pPipeline);
> -      else
> -         return gen7_compute_pipeline_create(_device, cache, pCreateInfo, pAllocator, pPipeline);
> -   case 8:
> -      return gen8_compute_pipeline_create(_device, cache, pCreateInfo, pAllocator, pPipeline);
> -   case 9:
> -      return gen9_compute_pipeline_create(_device, cache, pCreateInfo, pAllocator, pPipeline);
> -   default:
> -      unreachable("unsupported gen\n");
> -   }
> +   return ANV_GEN_DISPATCH(device, compute_pipeline_create,
> +                           _device, cache, pCreateInfo, pAllocator, pPipeline);
>  }
>  
>  VkResult anv_CreateComputePipelines(
> diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> index 0705263..27ae5e88 100644
> --- a/src/intel/vulkan/anv_private.h
> +++ b/src/intel/vulkan/anv_private.h
> @@ -1994,6 +1994,29 @@ ANV_DEFINE_STRUCT_CASTS(anv_common, VkMemoryBarrier)
>  ANV_DEFINE_STRUCT_CASTS(anv_common, VkBufferMemoryBarrier)
>  ANV_DEFINE_STRUCT_CASTS(anv_common, VkImageMemoryBarrier)
>  
> +#define ANV_GEN_DISPATCH(device, name, ...)      \
> +   ({                                            \
> +      __typeof(gen7_ ## name)* __func = NULL;    \
> +      switch ((device)->info.gen) {              \
> +      case 7:                                    \
> +         if ((device)->info.is_haswell) {        \
> +            __func = gen75_ ## name;             \
> +         } else {                                \
> +            __func = gen7_ ## name;              \
> +         }                                       \
> +         break;                                  \
> +      case 8:                                    \
> +         __func = gen8_ ## name;                 \
> +         break;                                  \
> +      case 9:                                    \
> +         __func = gen9_ ## name;                 \
> +         break;                                  \
> +      default:                                   \
> +         unreachable("unhandled gen");           \
> +      };                                         \
> +      __func( __VA_ARGS__);                      \
> +   })
> +
>  /* Gen-specific function declarations */
>  #ifdef genX
>  #  include "anv_genX.h"
> 

How does the generated assembly look now compared to before this patch?
I don't have a good sense of whether the compiler can eliminate the
function pointer or not.

Assuming the compiler is able to generate sensible code for this,
it looks great.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161017/30b012c3/attachment-0001.sig>


More information about the mesa-dev mailing list