[Mesa-dev] [PATCH 6/6] anv: add dispatch macro to find right function for given generation
Jason Ekstrand
jason at jlekstrand.net
Mon Oct 17 16:06:18 UTC 2016
As a side-note, I pushed code on Friday that gets rid of two of these (the
create_pipeline calls). I think the best way to solve
emit_state_base_address is by moving a bit of code into genX_cmd_buffer.c.
null surface states probably need to be moved into ISL.
In other words, while it's a nice cleanup, I think the correct answer for
almost all cases is to just stop doing the switches.
On Mon, Oct 17, 2016 at 8:51 AM, Kenneth Graunke <kenneth at whitecape.org>
wrote:
> 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.
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161017/b7177b42/attachment-0001.html>
More information about the mesa-dev
mailing list