On Fri, Jun 1, 2018 at 1:01 AM, 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. > > Right now, we still have a fully "logical" pointer model where pointer > chains eventually terminate at variable dereferences. In future, we'd like > to be able to use nir_deref_instrs for things like UBOs and SSBOs where tha > tmay not be the case. There are still a couple open questions around how > we want to handle "raw" pointers in SPIR-V for OpenCL drivers particularly > around casting. However, the hard part is getting the deref instructions > and getting everything switched over to them. Now that we've done that, > some of those other details can be sorted out later. > > This series is, as far as Rob, Bas, and I can tell, a complete and correct > transition for all NIR-based drivers. The final patch deletes the data > structures and helpers for the older deref chains. > > A massive thank you goes out to Rob for putting the final patch set > together and trying to get things in an order that will hopefully not > regress anyone. Thanks also to Bas for fixing up the radeon and radv > bits. > > This series can be found as a branch on gitlab: > > https://gitlab.freedesktop.org/jekstrand/mesa/commits/review/nir-deref-instr-v4 > > Ideally, I'd like the series to get some amount of real review before it > lands. Honestly, it's been baking long enough and tested by enough people > on enough drivers that we can probably throw a bunch of Acked-by and > Tested-bys on it and call it a day but I'd rather not. I plan to review > all of the patches I didn't write but that will have to wait until > tomorrow. > > At the very least, I'd like some sort of an ACK from a variety of the > people that use NIR on the core concept and the sort of general shape of > things at the end of the series. A lot of work has gone into this but it's > also a big change and the more positive feedback it gets, the more > comfortable I'll be pulling the trigger.
fwiw, with the issues that Bas hit, strong a-b from me.. I've been working with the patchset for a while, (and a big stack of compute related patches on top that I'd like to be able to start sending to list) and the handful of comments made on irc in the early stages have been addressed. This makes for a much cleaner base to start adding "real" pointer support for compute, so I'm totally happy, it works out much cleaner than earlier attempts based on working around deref chains :-) not a traditional patch-by-patch review, so not really sure r-b is appropriate, but I'm pretty happy with the result (and the amount of churn involved does make tranditional patch-by-patch review difficult) BR, -R > > Thanks, > > --Jason Ekstrand > > > > 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> > Cc: Matt Turner <matts...@gmail.com> > Cc: Kenneth Graunke <kenn...@whitecape.org> > Cc: Ian Romanick <ian.d.roman...@intel.com> > > Bas Nieuwenhuizen (16): > ac/nir: Implement the deref instr for shared memory. > ac/nir: Support deref instructions in get_sampler_desc. > ac/nir: Support deref instructions in tex instructions. > ac/nir: Implement derefs for integer gather4 lowering. > ac/nir: Add deref support to image intrinsics. > radv: Add shader info support for image deref instructions. > ac/nir: Add deref based var loads/stores. > radv: Gather info for deref instr based load/store. > ac/nir: Add shared atomic deref instr support. > ac/nir: Add deref interp support. > radv: Use deref instructions for tex derefs in meta shaders. > radv: Remove image_var stores. > radeonsi: Add deref support to the nir scan pass. > ac/nir: Remove deref chain support. > radv: Remove deref chain support in radv shader info pass. > radeonsi: Remove deref chain support in nir scan pass. > > Eric Anholt (1): > broadcom/vc4: Remove deref chain support from nir_lower_txf_ms. > > Jason Ekstrand (98): > nir/validate: Rework intrinsic type validation > 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 > nir/deref: Add some deref cleanup functions > 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: Use deref instructions for params > 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: 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 > nir: Support deref instructions in loop_analyze > nir: Support deref instructions in lower_alpha_test > nir: Support deref instructions in lower_clamp_color_outputs > nir: Support deref instructions in lower_drawpixels > nir: Consider deref instructions in lower_phis_to_scalar > nir: Consider deref instructions in opt_peephole_select > nir: Support deref instructions in opt_undef > intel,ir3: Disable nir_opt_copy_prop_vars > intel/nir: Fixup deref modes after lowering patch vertices > nir/lower_tex: Always copy deref and offset sources > 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/spirv: Pass nir_variable_data into apply_var_decoration > 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/pipeline: Convert lower_multiview to deref instructions > 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 > intel/blorp: Stop setting tex->texture/sampler > nir/lower_samplers: Clean up function arguments > nir: Use derefs in nir_lower_samplers > nir/builder: Use deref instructions for load/store/copy_var > nir: Use deref instructions in lower_constant_initializers > nir/vars_to_ssa: Add an is_direct field to deref_node > nir/vars_to_ssa: Rework to entirely use deref instructions > nir: Rework gather_info to entirely use deref instructions > nir: Remove deref chain support from lower_indirect_derefs > nir: Remove deref chain support from lower_clip_cull_distance_arrays > nir: Remove deref chain support from lower_atomics > nir: Remove deref chain support from lower_alpha_test > nir: Remove deref chain support from lower_clamp_color_outputs > nir: Remove deref chain support from lower_global_vars_to_local > nir/lower_io: Convert atomic lowering to deref instructions > nir: Convert lower_io to deref instructions > nir: Remove deref chain support from lower_phis_to_scalar > nir: Delete lower_io_types > nir: Remove deref chain support from remove_unused_varyings > nir: Remove deref chain support from lower_system_values > nir: Remove deref chain support from lower_wpos_center > nir: Remove deref chain support from lower_wpos_ytransform > nir: Remove deref chain support from lower_tex > nir: Remove deref chain support from opt_peephole_select > nir: Remove deref chain support from lower_drawpixels > nir: Remove deref chain support from lower_var_copies > nir: Remove deref chain support from propagate_invariant > nir: Remove deref chain support from dead_variables > nir: Remove deref chain support from split_var_copies > nir: Remove deref chain support from opt_undef > nir: Remove deref chain support from split_per_member_structs > nir/copy_prop_vars: Re-order some logic in compare_derefs > nir: Rework opt_copy_prop_vars to use deref instructions > intel,ir3: Re-enable nir_opt_copy_prop_vars > nir: Rework lower_locals_to_regs to use deref instructions > nir: Remove deref chain support from analyze_loops > nir: Remove old-school deref chain support > nir/lower_system_values: Assert/assume direct var derefs > > Rob Clark (14): > nir: add deref lowering sanity checking > ttn: convert to deref instructions > nir/lower_samplers: split out _legacy version for deref chains > move lower_deref_instrs > nir: convert lower_io_to_scalar to deref instructions > mesa/st: temporarily disable lower_io_to_elements() > mesa/st/nir: convert lower_builtins to deref instructions > nir: convert lower_io_arrays_to_elements to deref instructions > mesa/st: re-enable lower_io_to_elements() > nir: convert lower_samplers_as_deref to deref instructions > nir/lower_samplers: remove legacy version > st,ir3,radeonsi: push lower_deref_instrs back into driver > nir: promote intrinsic_get_var() to helper > freedreno/ir3: convert to deref instructions > > src/amd/common/ac_nir_to_llvm.c | 547 ++++++++++---------- > src/amd/vulkan/radv_meta.c | 20 +- > src/amd/vulkan/radv_meta_blit.c | 30 +- > src/amd/vulkan/radv_meta_blit2d.c | 21 +- > src/amd/vulkan/radv_meta_bufimage.c | 62 +-- > src/amd/vulkan/radv_meta_fast_clear.c | 17 +- > src/amd/vulkan/radv_meta_resolve_cs.c | 10 +- > src/amd/vulkan/radv_shader.c | 8 + > src/amd/vulkan/radv_shader_info.c | 139 ++--- > src/compiler/Makefile.sources | 4 +- > src/compiler/glsl/gl_nir_lower_atomics.c | 137 ++--- > src/compiler/glsl/gl_nir_lower_samplers.c | 162 +++--- > src/compiler/glsl/gl_nir_lower_samplers_as_deref.c | 180 ++++--- > src/compiler/glsl/glsl_to_nir.cpp | 267 ++++------ > src/compiler/nir/meson.build | 4 +- > src/compiler/nir/nir.c | 423 +++------------- > src/compiler/nir/nir.h | 207 ++++---- > src/compiler/nir/nir_builder.h | 254 ++++++++-- > src/compiler/nir/nir_clone.c | 143 ++---- > src/compiler/nir/nir_deref.c | 165 ++++++ > src/compiler/nir/nir_deref.h | 55 ++ > src/compiler/nir/nir_gather_info.c | 51 +- > src/compiler/nir/nir_inline_functions.c | 193 ++----- > src/compiler/nir/nir_instr_set.c | 101 +++- > src/compiler/nir/nir_intrinsics.py | 99 ++-- > src/compiler/nir/nir_intrinsics_c.py | 1 - > src/compiler/nir/nir_linking_helpers.c | 47 +- > src/compiler/nir/nir_loop_analyze.c | 70 ++- > src/compiler/nir/nir_lower_alpha_test.c | 7 +- > src/compiler/nir/nir_lower_clamp_color_outputs.c | 9 +- > .../nir/nir_lower_clip_cull_distance_arrays.c | 85 ++-- > src/compiler/nir/nir_lower_constant_initializers.c | 57 ++- > src/compiler/nir/nir_lower_drawpixels.c | 10 +- > src/compiler/nir/nir_lower_global_vars_to_local.c | 45 +- > src/compiler/nir/nir_lower_indirect_derefs.c | 174 +++---- > src/compiler/nir/nir_lower_io.c | 185 ++++--- > src/compiler/nir/nir_lower_io_arrays_to_elements.c | 154 +++--- > src/compiler/nir/nir_lower_io_to_scalar.c | 90 ++-- > src/compiler/nir/nir_lower_io_to_temporaries.c | 2 + > src/compiler/nir/nir_lower_io_types.c | 176 ------- > src/compiler/nir/nir_lower_locals_to_regs.c | 186 ++++--- > src/compiler/nir/nir_lower_phis_to_scalar.c | 14 +- > src/compiler/nir/nir_lower_system_values.c | 31 +- > src/compiler/nir/nir_lower_tex.c | 47 +- > src/compiler/nir/nir_lower_var_copies.c | 165 +++--- > src/compiler/nir/nir_lower_vars_to_ssa.c | 330 ++++++------ > src/compiler/nir/nir_lower_wpos_center.c | 8 +- > src/compiler/nir/nir_lower_wpos_ytransform.c | 33 +- > src/compiler/nir/nir_opt_constant_folding.c | 53 -- > src/compiler/nir/nir_opt_copy_prop_vars.c | 316 ++++++------ > src/compiler/nir/nir_opt_copy_propagate.c | 95 ++-- > src/compiler/nir/nir_opt_dce.c | 7 + > src/compiler/nir/nir_opt_peephole_select.c | 4 +- > src/compiler/nir/nir_opt_undef.c | 2 +- > src/compiler/nir/nir_print.c | 252 +++++---- > src/compiler/nir/nir_propagate_invariant.c | 17 +- > src/compiler/nir/nir_remove_dead_variables.c | 152 +++--- > src/compiler/nir/nir_serialize.c | 234 ++++----- > src/compiler/nir/nir_split_per_member_structs.c | 208 ++++++++ > src/compiler/nir/nir_split_var_copies.c | 234 ++------- > src/compiler/nir/nir_sweep.c | 4 - > src/compiler/nir/nir_validate.c | 247 ++++----- > src/compiler/spirv/spirv_to_nir.c | 184 ++++--- > src/compiler/spirv/vtn_cfg.c | 231 ++++----- > src/compiler/spirv/vtn_glsl450.c | 19 +- > src/compiler/spirv/vtn_private.h | 26 +- > src/compiler/spirv/vtn_variables.c | 563 > +++++++-------------- > src/gallium/auxiliary/nir/tgsi_to_nir.c | 52 +- > src/gallium/drivers/freedreno/ir3/ir3_cmdline.c | 1 - > .../drivers/freedreno/ir3/ir3_compiler_nir.c | 49 +- > src/gallium/drivers/freedreno/ir3/ir3_nir.c | 4 +- > .../freedreno/ir3/ir3_nir_lower_tg4_to_tex.c | 4 +- > src/gallium/drivers/radeonsi/si_shader_nir.c | 72 ++- > src/gallium/drivers/vc4/vc4_nir_lower_txf_ms.c | 1 - > src/intel/blorp/blorp_blit.c | 2 - > src/intel/compiler/brw_fs.h | 2 +- > src/intel/compiler/brw_fs_nir.cpp | 157 +++--- > 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_multiview.c | 17 +- > src/intel/vulkan/anv_nir_lower_ycbcr_textures.c | 34 +- > src/intel/vulkan/anv_pipeline.c | 7 + > src/mesa/drivers/dri/i965/brw_nir_uniforms.cpp | 2 + > src/mesa/drivers/dri/i965/brw_program.c | 5 +- > src/mesa/program/prog_to_nir.c | 41 +- > src/mesa/state_tracker/st_nir_lower_builtin.c | 62 ++- > 86 files changed, 4183 insertions(+), 4620 deletions(-) > create mode 100644 src/compiler/nir/nir_deref.c > create mode 100644 src/compiler/nir/nir_deref.h > delete mode 100644 src/compiler/nir/nir_lower_io_types.c > 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