[Mesa-dev] [PATCH 00/61] nir: Move to using instructions for derefs

Rob Clark robdclark at gmail.com
Thu Mar 29 00:43:30 UTC 2018


On Wed, Mar 28, 2018 at 8:16 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On March 28, 2018 16:54:33 Rob Clark <robdclark at gmail.com> wrote:
>
> I had noticed the code to remove dead deref's in a few of the passes
> (at least on your wip branch), and had wondered a bit about not just
> requiring all the deref related lowering to happen in ssa and possibly
> require dce after, although admittedly hadn't thought about it *too*
> much yet..
>
> Yeah. Like I said below, it should be ready enough to just have a tiny
> clean-up pass instead of having to run full-on dce.  Maybe just running dce
> is the right choice; I'm not sure.
>
>
> I kinda expected to use the dce clean things up once we are in a
> deref-instruction world.. re: validation passes, could we not just
> allow dead deref instructions to be ok.  That seems like kind of a
> natural thing..
>
> Making validation ignore them is easy. The trickier bit is that they can
> cause problems for any pass which works on all deref instructions as opposed
> to working on texture instructions or intrinsics and tracing the deref chain
> back.  The later are ok because they'll never look at dead derefs.  There
> former (which are likely to be more efficient if we've CSEd derefs) can run
> into trouble as it's not always obvious when a deref is dead.
>
> I'm sure I'll get a better feel for this whole mess as I continue to
> progress.

Defn not trying to second guess you since you are deeper into it that
I am..  But requiring dce (or a some sort of mini deref-dce) pass in
various places seems reasonable.. I guess it would be nice (given the
growing list of nir passes) to have some more formal way to require
that some pass(es) is run prior to a whatever random pass driver wants
to run would be nice.  (Not sure if llvm's PassManager provides this..
if it doesn't, it should.)

BR,
-R

> --Jason
>
>
>
> BR,
> -R
>
>
> On Wed, Mar 28, 2018 at 7:43 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> One interesting and unexpected side effect of this series has been that dead
> code elimination is now required to clean up unused deref instructions.
> This can be a problem for passes which alter and/or delete the variable
> because they may leave invalid deref instructions lying around.  This is a
> bit troublesome because it causes validation issues and can confused later
> passes if there are invalid deref instructions even if they are unused.  I
> have added a helper that allows you to easily check if a deref instruction
> is in use and remove it and its ancestors if it is not. This seems to help a
> bit but means that you have to manually clean up derefs whenever you alter
> or remove a variable.  Another option would be to write a simple dead deref
> elimination pass that other optimization passes can run.
>
> I'm not 100% sure what I think of that.  On the balance, though, I think the
> amount that removing deref chains simplifies the IR still makes it worth it.
>
>
> On March 23, 2018 14:43:16 Jason Ekstrand <jason at jlekstrand.net> wrote:
>
> This is something that Connor and I have been talking about for some time
> now.  The basic idea is to replace the current singly linked nir_deref
> list
> with deref instructions.  This is similar to what LLVM does and it offers
> quite a bit more freedom when we start getting more realistic pointers
> from
> compute applications.
>
> This series implements an almost complete conversion for both i965 and
> anv.
> The two remaining gaps are nir_lower_locals_to_regs and
> nir_lower_samplers.
> The former will have to wait for ir3 to be converted and the later will
> have to wait for radeonsi.  I've got patches for nir_lower_samplers but
> not
> nir_lower_samplers_as_deref which is required by at least radeonsi.  Once
> those are in place, we should be able to drop the lowering pass from the
> Intel back-end completely.
>
> The next step (which I will start on next week) will be removing legacy
> derefs from core NIR.  This will also involve significant reworks in some
> passes such as vars_to_ssa which still uses legacy derefs internally even
> for things which use deref instructions.
>
> Clearly, we can't remove anything until all of the other drivers are
> converted.  However, this series should be a good basis for anyone wanting
> to work on converting another driver since almost all of the core NIR
> passes now work with both types of derefs so you can convert in whatever
> way makes sense.
>
> This series can be found as a branch on gitlab:
>
>
> https://gitlab.freedesktop.org/jekstrand/mesa/commits/review/nir-deref-instrs-v1
>
> Cc: Rob Clark <robdclark at gmail.com>
> Cc: Timothy Arceri <tarceri at itsqueeze.com>
> Cc: Eric Anholt <eric at anholt.net>
> Cc: Connor Abbott <cwabbott0 at gmail.com>
> Cc: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
> Cc: Karol Herbst <kherbst at redhat.com>
>
> Jason Ekstrand (61):
> nir: Add src/dest num_components helpers
> nir: Return a cursor from nir_instr_remove
> nir/vars_to_ssa: Remove copies from the correct set
> nir/lower_indirect_derefs: Support interp_var_at intrinsics
> intel/vec4: Set channel_sizes for MOV_INDIRECT sources
> nir/validator: Validate that all used variables exist
> nir: Add a deref instruction type
> nir/builder: Add deref building helpers
> nir: Add _deref versions of all of the _var intrinsics
> nir: Add deref sources to texture instructions
> nir: Add helpers for working with deref instructions
> anv,i965,radv,st,ir3: Call nir_lower_deref_instrs
> glsl/nir: Only claim to handle intrinsic functions
> glsl/nir: Use deref instructions instead of dref chains
> prog/nir: Simplify some load/store operations
> prog/nir: Use deref instructions for params
> nir/lower_atomics: Rework the main walker loop a bit
> nir: Support deref instructions in remove_dead_variables
> nir: Add a pass for fixing deref modes
> nir: Support deref instructions in lower_global_vars_to_local
> nir: Support deref instructions in lower_io_to_temporaries
> nir: Add a deref path helper struct
> nir: Support deref instructions in lower_var_copies
> nir: Support deref instructions in split_var_copies
> nir: Support deref instructions in lower_vars_to_ssa
> nir: Support deref instructions in lower_indirect_derefs
> nir/deref: Add a deref cleanup function
> nir: Support deref instructions in lower_system_values
> nir: Support deref instructions in lower_clip_cull
> nir: Support deref instructions in propagate_invariant
> nir: Support deref instructions in gather_info
> nir: Support deref instructions in lower_io
> nir: Support deref instructions in lower_atomics
> nir: Support deref instructions in lower_wpos_ytransform
> nir: Support deref instructions in lower_pos_center
> nir: Support deref instructions in remove_unused_varyings
> intel,ir3: Disable nir_opt_copy_prop_vars
> intel/nir: Fixup deref modes after lowering patch vertices
> i965: Move nir_lower_deref_instrs to right before locals_to_regs
> st/nir: Move lower_deref_instrs later
> spirv: Use deref instructions for most variables
> nir: Add a concept of per-member structs and a lowering pass
> nir/lower_system_values: Support SYSTEM_VALUE_LOCAL_GROUP_SIZE
> spirv: Use the LOCAL_GROUP_SIZE system value
> nir/spirv: Pass nir_variable_data into apply_var_decoration
> anv/pipeline: Lower more constant initializers earlier
> spirv: Use NIR per-member splitting
> spirv: Make push constants an offset-based pointer
> spirv: Clean up vtn_pointer_to_offset
> spirv: Allow pointers to have a deref at the base
> spirv: Update vtn_pointer_to/from_ssa to handle deref pointers
> spirv: Record the type of functions
> spirv/cfg: Make the builder fully capable for both walks
> nir,spirv: Rework function calls
> anv/pipeline: Do less deref instruction lowering
> anv/pipeline: Convert lower_input_attachments to deref instructions
> anv/pipeline: Convert YCbCr lowering to deref instructiosn
> anv/apply_pipeline_layout: Simplify extract_tex_src_plane
> anv/pipeline: Convert apply_pipeline_layout to deref instructions
> intel/fs: Use image_deref intrinsics instead of image_var
> intel/nir: Only lower load/store derefs
>
> src/amd/vulkan/radv_shader.c                       |  10 +
> src/compiler/Makefile.sources                      |   3 +
> src/compiler/glsl/glsl_to_nir.cpp                  | 265 ++++------
> src/compiler/nir/meson.build                       |   3 +
> src/compiler/nir/nir.c                             | 107 ++--
> src/compiler/nir/nir.h                             | 175 +++++-
> src/compiler/nir/nir_builder.h                     | 236 +++++++++
> src/compiler/nir/nir_clone.c                       |  65 ++-
> src/compiler/nir/nir_deref.c                       | 393 ++++++++++++++
> src/compiler/nir/nir_deref.h                       |  55 ++
> src/compiler/nir/nir_gather_info.c                 |  26 +-
> src/compiler/nir/nir_inline_functions.c            | 193 +------
> src/compiler/nir/nir_instr_set.c                   |  78 +++
> src/compiler/nir/nir_intrinsics.h                  |  88 ++++
> src/compiler/nir/nir_linking_helpers.c             |  50 +-
> src/compiler/nir/nir_lower_atomics.c               | 137 ++++-
> .../nir/nir_lower_clip_cull_distance_arrays.c      |  69 ++-
> src/compiler/nir/nir_lower_global_vars_to_local.c  |  62 ++-
> src/compiler/nir/nir_lower_indirect_derefs.c       | 169 +++++-
> src/compiler/nir/nir_lower_io.c                    |  70 ++-
> src/compiler/nir/nir_lower_io_to_temporaries.c     |   2 +
> src/compiler/nir/nir_lower_system_values.c         |  23 +-
> src/compiler/nir/nir_lower_var_copies.c            |  90 +++-
> src/compiler/nir/nir_lower_vars_to_ssa.c           |  77 ++-
> src/compiler/nir/nir_lower_wpos_center.c           |  13 +-
> src/compiler/nir/nir_lower_wpos_ytransform.c       |  51 +-
> src/compiler/nir/nir_opt_copy_prop_vars.c          |  19 +-
> src/compiler/nir/nir_opt_copy_propagate.c          |  62 ++-
> src/compiler/nir/nir_opt_dce.c                     |   7 +
> src/compiler/nir/nir_print.c                       | 119 +++--
> src/compiler/nir/nir_propagate_invariant.c         |  23 +-
> src/compiler/nir/nir_remove_dead_variables.c       | 117 +++-
> src/compiler/nir/nir_serialize.c                   | 137 +++--
> src/compiler/nir/nir_split_per_member_structs.c    | 289 ++++++++++
> src/compiler/nir/nir_split_var_copies.c            |  42 ++
> src/compiler/nir/nir_sweep.c                       |   4 -
> src/compiler/nir/nir_validate.c                    | 143 +++--
> src/compiler/spirv/spirv_to_nir.c                  | 180 ++++---
> src/compiler/spirv/vtn_cfg.c                       | 231 ++++----
> src/compiler/spirv/vtn_glsl450.c                   |  19 +-
> src/compiler/spirv/vtn_private.h                   |  28 +-
> src/compiler/spirv/vtn_variables.c                 | 586
> +++++++--------------
> src/gallium/drivers/freedreno/ir3/ir3_cmdline.c    |   3 +
> src/gallium/drivers/freedreno/ir3/ir3_nir.c        |   2 +-
> src/intel/compiler/brw_fs.h                        |   2 +-
> src/intel/compiler/brw_fs_nir.cpp                  | 157 +++---
> src/intel/compiler/brw_nir.c                       |   4 +-
> src/intel/compiler/brw_vec4.cpp                    |   5 +-
> src/intel/vulkan/anv_nir_apply_pipeline_layout.c   | 187 +++----
> src/intel/vulkan/anv_nir_lower_input_attachments.c |  31 +-
> src/intel/vulkan/anv_nir_lower_ycbcr_textures.c    |  34 +-
> src/intel/vulkan/anv_pipeline.c                    |  19 +-
> src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp     |   2 +
> src/mesa/drivers/dri/i965/brw_program.c            |   1 +
> src/mesa/program/prog_to_nir.c                     |  65 +--
> src/mesa/state_tracker/st_glsl_to_nir.cpp          |   2 +
> 56 files changed, 3451 insertions(+), 1579 deletions(-)
> create mode 100644 src/compiler/nir/nir_deref.c
> create mode 100644 src/compiler/nir/nir_deref.h
> create mode 100644 src/compiler/nir/nir_split_per_member_structs.c
>
> --
> 2.5.0.400.gff86faf
>
>
>


More information about the mesa-dev mailing list