[Mesa-dev] [PATCH 00/21] intel/compiler: Rework the world of push/pull params

Jason Ekstrand jason at jlekstrand.net
Fri Sep 29 21:25:00 UTC 2017


This little series reworks basically the entire world of push/pull params.
This refactor has been on my ToDo list ever since I hooked up push
constants in Vulkan over two years ago.  Recently, thanks to some
up-and-coming Vulkan features, the pain of the old system increased enough
to finally push me over the edge to make the changes.

Historically, we have had two arrays in brw_stage_prog_data: param and
pull_param which are arrays of pointers to a gl_constant_value union.  In
state setup, we would walk the arrays, dereference each pointer, and copy
the value into our push or pull constant storage.  These pointers may point
to uniform storage, ARB program parameter storage, or random bits of API
state that we've gathered up.  This model has never really matched what we
want in Vulkan because we don't have a dedicated CPU memory location where
the push constant values are stored that we can know at compile time.
Instead, in Vulkan, we just store offsets into a struct anv_push_constants
in the pointers and trust that the back-end compiler will re-arrange the
array without ever dereferencing any of the pointers.

This primary purpose of this series is to move us over to a model where the
param and pull_param arrays are just arrays of uint32_t handles which have
a meaning which is defined by the driver.  This makes the contract between
compiler and driver a bit more clear and we stop passing random pointers to
API state into the back-end compiler.

The secondary purpose of this series is to add support for "builtin"
params.  These params have a well-defined meaning as per their builtin enum
name and will get filled out by the driver during state upload.  Having a
well-defined meaning means that they can be added by either the front-end
or back-end compilers without having to do a lot of juggling in order to
make them work in two drivers.  One example of this is the last patch which
moves the thread local id from being this weird special-cased thing to
being a normal builtin param.  We still have to do a bit of special-case
work because it needs to be part of the per-thread push constants but other
than that, it now works more-or-less like any other param.

Thirdly, I did a bunch of refactoring to try and get us to the point where
the param and pull_param arrays are, as far as practical, out parameters of
the compilation process rather than in-out parameters.  In particular, we
stop trying to estimate in the driver how much space we need to allocate
for params and just allocate what we need on-demand in the various places
that set them up.

This series can be found here:

https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=wip/i965-push-enums

Jason Ekstrand (21):
  i965: Move brw_upload_pull_constants to gen6_constant_state.c
  i965: Add a helper for populating constant buffers
  i965: Get rid of gen7_cs_state.c
  intel: Rewrite the world of push/pull params
  i965: Use prog->info.num_images for needs_dc computation
  i965: Store image_param in brw_context instead of prog_data
  anv/pipeline: Add a mem_ctx parameter to anv_pipeline_compile
  anv/pipeline: Ralloc prog_data::param of the compile mem_ctx
  intel/compiler: Add a flag for pull constant support
  i965: Only add the wpos state reference if we lowered something
  intel/compiler: Stop adding params for texture sizes
  intel/compiler: Add a helper for growing the prog_data::param array
  intel/cs: Grow prog_data::param on-demand for thread_local_id_index
  intel/vs: Grow the param array for clip planes
  anv/pipeline: Whack nir->num_uniforms to MAX_PUSH_CONSTANT_SIZE
  anv/pipeline: Grow the param array for images
  anv/pipeline: Refactor setup of the prog_data::param array
  ralloc: Allow reparenting to a NULL context
  intel: Allocate prog_data::[pull_]param deeper inside the compiler
  intel/compiler: Allocate pull_param in assign_constant_locations
  intel/cs: Make thread_local_id a regular builtin param

 src/intel/blorp/blorp.c                          |   2 +-
 src/intel/compiler/brw_compiler.h                |  98 ++++++++--
 src/intel/compiler/brw_fs.cpp                    |  78 ++++----
 src/intel/compiler/brw_fs.h                      |   6 +-
 src/intel/compiler/brw_fs_visitor.cpp            |  15 +-
 src/intel/compiler/brw_nir.h                     |   5 +-
 src/intel/compiler/brw_nir_intrinsics.c          |  18 +-
 src/intel/compiler/brw_vec4.cpp                  |  18 +-
 src/intel/compiler/brw_vec4_visitor.cpp          |   7 +-
 src/intel/compiler/brw_vec4_vs.h                 |   3 -
 src/intel/compiler/brw_vec4_vs_visitor.cpp       |  11 +-
 src/intel/vulkan/anv_cmd_buffer.c                |  41 +++--
 src/intel/vulkan/anv_device.c                    |   1 +
 src/intel/vulkan/anv_nir_apply_pipeline_layout.c |  28 ++-
 src/intel/vulkan/anv_pipeline.c                  | 116 ++++++------
 src/intel/vulkan/anv_pipeline_cache.c            |   6 +-
 src/intel/vulkan/anv_private.h                   |   3 +
 src/mesa/drivers/dri/i965/Makefile.sources       |   1 -
 src/mesa/drivers/dri/i965/brw_context.h          |   2 +
 src/mesa/drivers/dri/i965/brw_cs.c               |  29 +--
 src/mesa/drivers/dri/i965/brw_curbe.c            |  12 +-
 src/mesa/drivers/dri/i965/brw_gs.c               |  28 +--
 src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp   |  83 +++++----
 src/mesa/drivers/dri/i965/brw_program.c          |  10 +-
 src/mesa/drivers/dri/i965/brw_program.h          |  23 +++
 src/mesa/drivers/dri/i965/brw_state.h            |  14 +-
 src/mesa/drivers/dri/i965/brw_tcs.c              |  50 ++----
 src/mesa/drivers/dri/i965/brw_tes.c              |  29 +--
 src/mesa/drivers/dri/i965/brw_vs.c               |  30 +---
 src/mesa/drivers/dri/i965/brw_vs_surface_state.c |  63 -------
 src/mesa/drivers/dri/i965/brw_wm.c               |  25 +--
 src/mesa/drivers/dri/i965/brw_wm_surface_state.c |   2 +-
 src/mesa/drivers/dri/i965/gen6_constant_state.c  | 218 ++++++++++++++++++++++-
 src/mesa/drivers/dri/i965/gen7_cs_state.c        | 173 ------------------
 src/mesa/drivers/dri/i965/gen7_l3_state.c        |   5 +-
 src/mesa/drivers/dri/i965/genX_state_upload.c    |  68 ++++++-
 src/mesa/drivers/dri/i965/intel_screen.c         |   1 +
 src/util/ralloc.c                                |   2 +-
 38 files changed, 702 insertions(+), 622 deletions(-)
 delete mode 100644 src/mesa/drivers/dri/i965/gen7_cs_state.c

-- 
2.5.0.400.gff86faf



More information about the mesa-dev mailing list