[Mesa-dev] [FLAG-DAY-PREP 0/9] nir: accessors for deref vars

[Rob Clark]

In ir3, and some of the lowering passes, pretty much 100% of the spots
we reach into deref chain is just to find the nir_variable.. in amd,
it is less (nir->llvm does a lot more with deref chains).. i965 was
kinda in the middle.  And from what I could tell, vc4/vc5 didn't
really care (tbf, most of the places ir3 needed to find the
nir_variable were compute related things which vc4/vc5 hasn't
implemented yet)

tbh, I'm in favour of some helpers like this to abstract things a bit
better.. it doesn't make transition from deref chains to deref
instructions completely transparent for all the drivers (although it
nearly would for ir3 at least), but it seems like it would cut down
the churn.  It seems like too much open-coding makes refactoring
harder than it needs to be.

BR,
-R

On Fri, Mar 16, 2018 at 8:26 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> I'm honestly not sure how helpful this is.  At least in all the places where
> this series affects our drivers, more serious changes are needed.
>
>
> On Fri, Mar 16, 2018 at 3:45 PM, Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
> wrote:
>>
>> FWIW
>>
>> Reviewed-by: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
>>
>> for the series.
>>
>> On Thu, Mar 15, 2018 at 3:27 PM, Rob Clark <robdclark at gmail.com> wrote:
>> > Since a fair bit of access to nir_deref_var's is just to chase down the
>> > nir_variable for an intrinsic (or texture/sampler vars for tex
>> > instructions), we can make the flag day slightly less painful with some
>> > accessors that could be re-implemented for deref instruction chains.
>> >
>> > It will still be a flag day, there are still a lot of access to the
>> > nir_deref_var's in intrinsic instructions (and to a slightly lesser
>> > extent, tex instructions).
>> >
>> > Looks like amd's nir->llvm does a lot w/ deref chains.. vc4/vc5 and ir3
>> > do not so much, other than tex related lowering pass, but I think that
>> > should be a trivial conversion to the brave new world of deref
>> > instructions (ie. just need a replacement for nir_deref_var_clone())
>> >
>> > This series has light testing w/ freedreno, but it is mostly mechanical
>> > changes.  If someone wants to feed it to automated testing, I have a
>> > branch here:
>> >
>> >   https://github.com/freedreno/mesa/commits/flag-day-prep
>> >
>> > Rob Clark (9):
>> >   nir: start to abstract deref chains/instructions
>> >   freedreno/ir3: use nir_intrinsic_var() accessor
>> >   amd: use nir_intrinsic_var() accessor
>> >   mesa/st: use nir_intrinsic_var() accessor
>> >   intel: use nir_intrinsic_var() accessor
>> >   nir: use nir_intrinsic_var() accessor
>> >   nir: use nir_tex_*_var() accessors
>> >   intel: use nir_tex_*_var() accessors
>> >   amd: use nir_tex_*_var() accessors
>> >
>> >  src/amd/common/ac_nir_to_llvm.c                    | 76
>> > ++++++++++++----------
>> >  src/amd/vulkan/radv_shader_info.c                  | 22 ++++---
>> >  src/compiler/nir/nir.h                             | 34 ++++++++++
>> >  src/compiler/nir/nir_gather_info.c                 |  2 +-
>> >  src/compiler/nir/nir_inline_functions.c            |  6 +-
>> >  src/compiler/nir/nir_instr_set.c                   |  2 +-
>> >  src/compiler/nir/nir_linking_helpers.c             | 10 ++-
>> >  src/compiler/nir/nir_lower_alpha_test.c            |  2 +-
>> >  src/compiler/nir/nir_lower_atomics.c               | 11 ++--
>> >  src/compiler/nir/nir_lower_clamp_color_outputs.c   |  2 +-
>> >  src/compiler/nir/nir_lower_global_vars_to_local.c  |  2 +-
>> >  src/compiler/nir/nir_lower_indirect_derefs.c       |  4 +-
>> >  src/compiler/nir/nir_lower_io.c                    | 10 +--
>> >  src/compiler/nir/nir_lower_io_arrays_to_elements.c |  4 +-
>> >  src/compiler/nir/nir_lower_io_to_scalar.c          |  2 +-
>> >  src/compiler/nir/nir_lower_io_types.c              |  2 +-
>> >  src/compiler/nir/nir_lower_locals_to_regs.c        |  4 +-
>> >  src/compiler/nir/nir_lower_phis_to_scalar.c        |  9 +--
>> >  src/compiler/nir/nir_lower_samplers.c              |  6 +-
>> >  src/compiler/nir/nir_lower_system_values.c         |  2 +-
>> >  src/compiler/nir/nir_lower_wpos_ytransform.c       |  4 +-
>> >  src/compiler/nir/nir_opt_peephole_select.c         |  2 +-
>> >  src/compiler/nir/nir_propagate_invariant.c         |  8 +--
>> >  src/compiler/nir/nir_remove_dead_variables.c       | 25 ++++---
>> >  src/compiler/nir/nir_validate.c                    | 22 ++++---
>> >  .../drivers/freedreno/ir3/ir3_compiler_nir.c       |  8 +--
>> >  src/gallium/drivers/freedreno/ir3/ir3_nir.c        |  2 +-
>> >  src/gallium/drivers/radeonsi/si_shader_nir.c       |  4 +-
>> >  src/intel/compiler/brw_fs_nir.cpp                  |  4 +-
>> >  src/intel/vulkan/anv_nir_apply_pipeline_layout.c   | 20 +++---
>> >  src/intel/vulkan/anv_nir_lower_input_attachments.c |  2 +-
>> >  src/intel/vulkan/anv_nir_lower_ycbcr_textures.c    |  2 +-
>> >  src/mesa/state_tracker/st_nir_lower_builtin.c      |  2 +-
>> >  33 files changed, 184 insertions(+), 133 deletions(-)
>> >
>> > --
>> > 2.14.3
>> >
>
>