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

[Jason Ekstrand]

On March 16, 2018 18:04:26 Rob Clark <robdclark at gmail.com> wrote:

> 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)

In that case, it may make more sense than I initially thought.  I'm sure 
I'll understand better once I get into some other drivers.  I've already 
come across a case or two where I need the variable and it's kind of a pain 
with the new derefs.

--Jason

> 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
>>> >
>>
>>