On Wed, Mar 28, 2018 at 8:16 PM, Jason Ekstrand <ja...@jlekstrand.net> wrote: > On March 28, 2018 16:54:33 Rob Clark <robdcl...@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 <ja...@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 <ja...@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 <robdcl...@gmail.com> > Cc: Timothy Arceri <tarc...@itsqueeze.com> > Cc: Eric Anholt <e...@anholt.net> > Cc: Connor Abbott <cwabbo...@gmail.com> > Cc: Bas Nieuwenhuizen <b...@basnieuwenhuizen.nl> > Cc: Karol Herbst <kher...@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 > > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
