On Fri, Jun 1, 2018 at 6:03 AM, Bas Nieuwenhuizen <b...@basnieuwenhuizen.nl> wrote:
> On Fri, Jun 1, 2018 at 7:01 AM, Jason Ekstrand <ja...@jlekstrand.net> > 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 <caio.olive...@intel.com> > > --- > > 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); > > 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) > 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. > > + > > + 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 > > > > _______________________________________________ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev