[Mesa-dev] [PATCH v4 006/129] nir: Add helpers for working with deref instructions
Jason Ekstrand
jason at jlekstrand.net
Fri Jun 1 16:53:10 UTC 2018
On Fri, Jun 1, 2018 at 6:03 AM, Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
wrote:
> On Fri, Jun 1, 2018 at 7:01 AM, Jason Ekstrand <jason at 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.oliveira at 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 at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180601/94e2c2ff/attachment-0001.html>
More information about the mesa-dev
mailing list