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