[Mesa-dev] [PATCH 00/21] intel/compiler: Rework the world of push/pull params
Jordan Justen
jordan.l.justen at intel.com
Fri Oct 6 17:27:02 UTC 2017
Series Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>
Although, I think you said you might rewrite patch 13 (the
thread_local_id_index patch). If you just add the small stage check I
mentioned then you can add my r-b for it.
-Jordan
On 2017-09-29 14:25:00, Jason Ekstrand wrote:
> 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
>
> _______________________________________________
> 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