<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jun 1, 2018 at 6:03 AM, Bas Nieuwenhuizen <<a href="mailto:bas@basnieuwenhuizen.nl" target="_blank">bas@basnieuwenhuizen.nl</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Fri, Jun 1, 2018 at 7:01 AM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > This commit adds a pass for lowering deref instructions to deref chains > as well as some smaller helpers to ease the transition. > > Reviewed-by: Caio Marcelo de Oliveira Filho <<a href="mailto:caio.oliveira@intel.com">caio.oliveira@intel.com</a>> > --- > src/compiler/Makefile.sources | 1 + > src/compiler/nir/meson.build | 1 + > src/compiler/nir/nir.h | 33 +++++ > src/compiler/nir/nir_builder.h | 23 ++++ > src/compiler/nir/nir_deref.c | 301 +++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 359 insertions(+) > create mode 100644 src/compiler/nir/nir_deref.c > > diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources > index 3daa2c5..ee30046 100644 > --- a/src/compiler/Makefile.sources > +++ b/src/compiler/Makefile.sources > @@ -201,6 +201,7 @@ NIR_FILES = \ > nir/nir_control_flow.c \ > nir/nir_control_flow.h \ > nir/nir_control_flow_private.h \ > + nir/nir_deref.c \ > nir/nir_dominance.c \ > nir/nir_format_convert.h \ > nir/nir_from_ssa.c \ > diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build > index 3fec363..6c80c36 100644 > --- a/src/compiler/nir/meson.build > +++ b/src/compiler/nir/meson.build > @@ -92,6 +92,7 @@ files_libnir = files( > 'nir_control_flow.c', > 'nir_control_flow.h', > 'nir_control_flow_private.h', > + 'nir_deref.c', > 'nir_dominance.c', > 'nir_format_convert.h', > 'nir_from_ssa.c', > diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h > index 4f359f1..8b826d8 100644 > --- a/src/compiler/nir/nir.h > +++ b/src/compiler/nir/nir.h > @@ -1003,6 +1003,27 @@ nir_src_as_deref(nir_src src) > return nir_instr_as_deref(src.ssa->parent_instr); > } > > +static inline nir_deref_instr * > +nir_deref_instr_parent(const nir_deref_instr *instr) > +{ > + if (instr->deref_type == nir_deref_type_var) > + return NULL; > + else > + return nir_src_as_deref(instr->parent); > +} > + > +static inline nir_variable * > +nir_deref_instr_get_variable(const nir_deref_instr *instr) > +{ > + while (instr->deref_type != nir_deref_type_var) > + instr = nir_deref_instr_parent(instr); </div></div>I think we need to handle casts here, for which the type can be != nir_deref_type_var, but the next iteration can have instr = NULL. Can be fixed with static inline nir_variable * nir_deref_instr_get_variable(const nir_deref_instr *instr) { - while (instr->deref_type != nir_deref_type_var) + while (instr && instr->deref_type != nir_deref_type_var) instr = nir_deref_instr_parent(instr); - return instr->var; + return instr ? instr->var : NULL; } (Unless we want it to not look past casts, then we need to add && instr->deref_type != nir_deref_type_cast to the while condition) <div><div class="h5"></div></div></blockquote><div> </div><div>I think we want to not handle casts here. Anything that's calling this and doesn't know about casts will almost certainly be wrong if we give it a variable. I'm going to send out a few FIXUP patches in a minute. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> > + > + return instr->var; > +} > + > +nir_deref_var * > +nir_deref_instr_to_deref(nir_deref_instr *instr, void *mem_ctx); > + > typedef struct { > nir_instr instr; > > @@ -2598,6 +2619,18 @@ bool nir_inline_functions(nir_shader *shader); > > bool nir_propagate_invariant(nir_shader *shader); > > +enum nir_lower_deref_flags { > + nir_lower_load_store_derefs = (1 << 0), > + nir_lower_texture_derefs = (1 << 1), > + nir_lower_interp_derefs = (1 << 2), > + nir_lower_atomic_counter_derefs = (1 << 3), > + nir_lower_atomic_derefs = (1 << 4), > + nir_lower_image_derefs = (1 << 5), > +}; > + > +bool nir_lower_deref_instrs(nir_shader *shader, > + enum nir_lower_deref_flags flags); > + > void nir_lower_var_copy_instr(nir_intrinsic_instr *copy, nir_shader *shader); > bool nir_lower_var_copies(nir_shader *shader); > > diff --git a/src/compiler/nir/nir_builder.h b/src/compiler/nir/nir_builder.h > index a667372..42fe285 100644 > --- a/src/compiler/nir/nir_builder.h > +++ b/src/compiler/nir/nir_builder.h > @@ -644,6 +644,29 @@ nir_build_deref_cast(nir_builder *build, nir_ssa_def *parent, > return deref; > } > > +static inline nir_deref_instr * > +nir_build_deref_for_chain(nir_builder *b, nir_deref_var *deref_var) > +{ > + nir_deref_instr *tail = nir_build_deref_var(b, deref_var->var); > + for (nir_deref *d = deref_var->deref.child; d; d = d->child) { > + if (d->deref_type == nir_deref_type_array) { > + nir_deref_array *a = nir_deref_as_array(d); > + assert(a->deref_array_type != nir_deref_array_type_wildcard); > + > + nir_ssa_def *index = nir_imm_int(b, a->base_offset); > + if (a->deref_array_type == nir_deref_array_type_indirect) > + index = nir_iadd(b, index, nir_ssa_for_src(b, a->indirect, 1)); > + > + tail = nir_build_deref_array(b, tail, index); > + } else { > + nir_deref_struct *s = nir_deref_as_struct(d); > + tail = nir_build_deref_struct(b, tail, s->index); > + } > + } > + > + return tail; > +} > + > static inline nir_ssa_def * > nir_load_reg(nir_builder *build, nir_register *reg) > { > diff --git a/src/compiler/nir/nir_deref.c b/src/compiler/nir/nir_deref.c > new file mode 100644 > index 0000000..87a8192 > --- /dev/null > +++ b/src/compiler/nir/nir_deref.c > @@ -0,0 +1,301 @@ > +/* > + * Copyright © 2018 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS > + * IN THE SOFTWARE. > + */ > + > +#include "nir.h" > +#include "nir_builder.h" > + > +nir_deref_var * > +nir_deref_instr_to_deref(nir_deref_instr *instr, void *mem_ctx) > +{ > + nir_deref *deref = NULL; > + > + while (instr->deref_type != nir_deref_type_var) { > + nir_deref *nderef; > + switch (instr->deref_type) { > + case nir_deref_type_array: > + case nir_deref_type_array_wildcard: { > + nir_deref_array *deref_arr = nir_deref_array_create(mem_ctx); > + if (instr->deref_type == nir_deref_type_array) { > + nir_const_value *const_index = > + nir_src_as_const_value(instr->arr.index); > + if (const_index) { > + deref_arr->deref_array_type = nir_deref_array_type_direct; > + deref_arr->base_offset = const_index->u32[0]; > + } else { > + deref_arr->deref_array_type = nir_deref_array_type_indirect; > + deref_arr->base_offset = 0; > + nir_src_copy(&deref_arr->indirect, &instr->arr.index, mem_ctx); > + } > + } else { > + deref_arr->deref_array_type = nir_deref_array_type_wildcard; > + } > + nderef = &deref_arr->deref; > + break; > + } > + > + case nir_deref_type_struct: > + nderef = &nir_deref_struct_create(mem_ctx, instr->strct.index)->deref; > + break; > + > + default: > + unreachable("Invalid deref instruction type"); > + } > + > + nderef->child = deref; > + ralloc_steal(nderef, deref); > + nderef->type = instr->type; > + > + deref = nderef; > + assert(instr->parent.is_ssa); > + instr = nir_src_as_deref(instr->parent); > + } > + > + assert(instr->deref_type == nir_deref_type_var); > + nir_deref_var *deref_var = nir_deref_var_create(mem_ctx, instr->var); > + deref_var->deref.child = deref; > + ralloc_steal(deref_var, deref); > + > + return deref_var; > +} > + > +static nir_deref_var * > +nir_deref_src_to_deref(nir_src src, void *mem_ctx) > +{ > + return nir_deref_instr_to_deref(nir_src_as_deref(src), mem_ctx); > +} > + > +static bool > +nir_lower_deref_instrs_tex(nir_tex_instr *tex) > +{ > + bool progress = false; > + > + /* Remove the instruction before we modify it. This way we won't mess up > + * use-def chains when we move sources around. > + */ > + nir_cursor cursor = nir_instr_remove(&tex->instr); > + > + unsigned new_num_srcs = 0; > + for (unsigned i = 0; i < tex->num_srcs; i++) { > + if (tex->src[i].src_type == nir_tex_src_texture_deref) { > + tex->texture = nir_deref_src_to_deref(tex->src[i].src, tex); > + progress = true; > + continue; > + } else if (tex->src[i].src_type == nir_tex_src_sampler_deref) { > + tex->sampler = nir_deref_src_to_deref(tex->src[i].src, tex); > + progress = true; > + continue; > + } > + > + /* Compact the sources down to remove the deref sources */ > + assert(new_num_srcs <= i); > + tex->src[new_num_srcs++] = tex->src[i]; > + } > + tex->num_srcs = new_num_srcs; > + > + nir_instr_insert(cursor, &tex->instr); > + > + return progress; > +} > + > +static bool > +nir_lower_deref_instrs_intrin(nir_intrinsic_instr *intrin, > + enum nir_lower_deref_flags flags) > +{ > + nir_intrinsic_op deref_op = intrin->intrinsic; > + nir_intrinsic_op var_op; > + > + switch (deref_op) { > +#define CASE(a) \ > + case nir_intrinsic_##a##_deref: \ > + if (!(flags & nir_lower_load_store_derefs)) \ > + return false; \ > + var_op = nir_intrinsic_##a##_var; \ > + break; > + CASE(load) > + CASE(store) > + CASE(copy) > +#undef CASE > + > +#define CASE(a) \ > + case nir_intrinsic_interp_deref_##a: \ > + if (!(flags & nir_lower_interp_derefs)) \ > + return false; \ > + var_op = nir_intrinsic_interp_var_##a; \ > + break; > + CASE(at_centroid) > + CASE(at_sample) > + CASE(at_offset) > +#undef CASE > + > +#define CASE(a) \ > + case nir_intrinsic_atomic_counter_##a##_deref: \ > + if (!(flags & nir_lower_atomic_counter_derefs)) \ > + return false; \ > + var_op = nir_intrinsic_atomic_counter_##a##_var; \ > + break; > + CASE(inc) > + CASE(dec) > + CASE(read) > + CASE(add) > + CASE(min) > + CASE(max) > + CASE(and) > + CASE(or) > + CASE(xor) > + CASE(exchange) > + CASE(comp_swap) > +#undef CASE > + > +#define CASE(a) \ > + case nir_intrinsic_deref_atomic_##a: \ > + if (!(flags & nir_lower_atomic_derefs)) \ > + return false; \ > + var_op = nir_intrinsic_var_atomic_##a; \ > + break; > + CASE(add) > + CASE(imin) > + CASE(umin) > + CASE(imax) > + CASE(umax) > + CASE(and) > + CASE(or) > + CASE(xor) > + CASE(exchange) > + CASE(comp_swap) > +#undef CASE > + > +#define CASE(a) \ > + case nir_intrinsic_image_deref_##a: \ > + if (!(flags & nir_lower_image_derefs)) \ > + return false; \ > + var_op = nir_intrinsic_image_var_##a; \ > + break; > + CASE(load) > + CASE(store) > + CASE(atomic_add) > + CASE(atomic_min) > + CASE(atomic_max) > + CASE(atomic_and) > + CASE(atomic_or) > + CASE(atomic_xor) > + CASE(atomic_exchange) > + CASE(atomic_comp_swap) > + CASE(size) > + CASE(samples) > +#undef CASE > + > + default: > + return false; > + } > + > + /* Remove the instruction before we modify it. This way we won't mess up > + * use-def chains when we move sources around. > + */ > + nir_cursor cursor = nir_instr_remove(&intrin->instr); > + > + unsigned num_derefs = nir_intrinsic_infos[var_op].num_variables; > + assert(nir_intrinsic_infos[var_op].num_srcs + num_derefs == > + nir_intrinsic_infos[deref_op].num_srcs); > + > + /* Move deref sources to variables */ > + for (unsigned i = 0; i < num_derefs; i++) > + intrin->variables[i] = nir_deref_src_to_deref(intrin->src[i], intrin); > + > + /* Shift all the other sources down */ > + for (unsigned i = 0; i < nir_intrinsic_infos[var_op].num_srcs; i++) > + nir_src_copy(&intrin->src[i], &intrin->src[i + num_derefs], intrin); > + > + /* Rewrite the extra sources to NIR_SRC_INIT just in case */ > + for (unsigned i = 0; i < num_derefs; i++) > + intrin->src[nir_intrinsic_infos[var_op].num_srcs + i] = NIR_SRC_INIT; > + > + /* It's safe to just stomp the intrinsic to var intrinsic since every > + * intrinsic has room for some variables and the number of sources only > + * shrinks. > + */ > + intrin->intrinsic = var_op; > + > + nir_instr_insert(cursor, &intrin->instr); > + > + return true; > +} > + > +static bool > +nir_lower_deref_instrs_impl(nir_function_impl *impl, > + enum nir_lower_deref_flags flags) > +{ > + bool progress = false; > + > + /* Walk the instructions in reverse order so that we can safely clean up > + * the deref instructions after we clean up their uses. > + */ > + nir_foreach_block_reverse(block, impl) { > + nir_foreach_instr_reverse_safe(instr, block) { > + switch (instr->type) { > + case nir_instr_type_deref: > + if (list_empty(&nir_instr_as_deref(instr)->dest.ssa.uses)) { > + nir_instr_remove(instr); > + progress = true; > + } > + break; > + > + case nir_instr_type_tex: > + if (flags & nir_lower_texture_derefs) > + progress |= nir_lower_deref_instrs_tex(nir_instr_as_tex(instr)); > + break; > + > + case nir_instr_type_intrinsic: > + progress |= > + nir_lower_deref_instrs_intrin(nir_instr_as_intrinsic(instr), > + flags); > + break; > + > + default: > + break; /* Nothing to do */ > + } > + } > + } > + > + if (progress) { > + nir_metadata_preserve(impl, nir_metadata_block_index | > + nir_metadata_dominance); > + } > + > + return progress; > +} > + > +bool > +nir_lower_deref_instrs(nir_shader *shader, > + enum nir_lower_deref_flags flags) > +{ > + bool progress = false; > + > + nir_foreach_function(function, shader) { > + if (!function->impl) > + continue; > + > + progress |= nir_lower_deref_instrs_impl(function->impl, flags); > + } > + > + return progress; > +} > -- > 2.5.0.400.gff86faf > </div></div>> _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>