[Mesa-dev] [PATCH v3 2/3] nir: add nir_opt_vectorize_io()
Timothy Arceri
tarceri at itsqueeze.com
Fri Feb 15 03:54:41 UTC 2019
On 15/2/19 1:43 pm, Jason Ekstrand wrote:
> On Thu, Feb 14, 2019 at 7:25 PM Timothy Arceri <tarceri at itsqueeze.com
> <mailto:tarceri at itsqueeze.com>> wrote:
>
> On 15/2/19 7:51 am, Jason Ekstrand wrote:
> > ping?
>
> You seem to be asking me to rewrite the entire meat of the pass without
> any suggestion of how to achieve it, I didn't know how to reply.
>
>
> Right... Sorry, that wasn't incredibly helpful. Perhaps I should have
> taken a bit more time to explain my thoughts more fully. Yes,
> write-combining is a hard problem in general except that we have one big
> advantage. Because we always run nir_lower_io_to_temporaries on outputs
> in all stages except TCS, we should be guaranteed two things:
>
> 1. That variables with the nir_var_shader_out mode are only ever
> written and not read (except in TCS)
hmm my intention was to support TCS. For one to avoid some register use
regressions from splitting patches [1] in radv. But also the tess
shaders were part of the regressions in bug 107510 [2]
[1]
https://gitlab.freedesktop.org/tarceri/mesa/commit/d57683d944d608781605b5e6b8e7c8c69930f39c
[2] https://bugs.freedesktop.org/show_bug.cgi?id=107510
> 2. That all writes to variables with the nir_var_shader_out mode
> happen in the last block of the shader.
>
> This should make handling writes substantially easier. We can just walk
> the last block forwards and take the last write we see of each
> component. Because they all happen in one block, we don't have to worry
> about dataflow or anything like that. It's also possible that I'm
> wrong, we don't have those guarantees, it's terribly complicated, and I
> need to sit down and try to think through all this stuff. :-/
>
> For reads, as I said below, we can just emit them and hope CSE cleans
> them up.
>
> Does that make more sense? Sorry, I've been a bit curt with the e-mails
> and other correspondence lately and I need to try to slow down and
> explain myself better. I didn't mean to be impatient with the ping I
> just was wondering why I hadn't heard anything.
>
> --Jason
>
> Recombining stores is tricky, this pass is based on Conners series for
> vectorizing alu instructions[1].
>
> Since we need to do stores this way we just do loads properly while we
> are at it.
>
> [1] https://gitlab.freedesktop.org/mesa/mesa/merge_requests/94/commits
>
> >
> > On Mon, Feb 4, 2019 at 3:37 PM Jason Ekstrand
> <jason at jlekstrand.net <mailto:jason at jlekstrand.net>
> > <mailto:jason at jlekstrand.net <mailto:jason at jlekstrand.net>>> wrote:
> >
> > On Mon, Nov 5, 2018 at 8:58 PM Timothy Arceri
> <tarceri at itsqueeze.com <mailto:tarceri at itsqueeze.com>
> > <mailto:tarceri at itsqueeze.com
> <mailto:tarceri at itsqueeze.com>>> wrote:
> >
> > Once linking opts are done this pass recombines varying
> components.
> >
> > This patch is loosely based on Connor's vectorize alu pass.
> >
> > V2: skip fragment shaders
> >
> > V3:
> > - dont accidentally vectorise local vars
> > - pass correct component to create_new_store()
> > ---
> > src/compiler/Makefile.sources | 1 +
> > src/compiler/nir/meson.build | 1 +
> > src/compiler/nir/nir.h | 2 +
> > src/compiler/nir/nir_opt_vectorize_io.c | 527
> > ++++++++++++++++++++++++
> > 4 files changed, 531 insertions(+)
> > create mode 100644 src/compiler/nir/nir_opt_vectorize_io.c
> >
> > diff --git a/src/compiler/Makefile.sources
> > b/src/compiler/Makefile.sources
> > index ae170f02e82..5991df5a61c 100644
> > --- a/src/compiler/Makefile.sources
> > +++ b/src/compiler/Makefile.sources
> > @@ -290,6 +290,7 @@ NIR_FILES = \
> > nir/nir_opt_shrink_load.c \
> > nir/nir_opt_trivial_continues.c \
> > nir/nir_opt_undef.c \
> > + nir/nir_opt_vectorize_io.c \
> > nir/nir_phi_builder.c \
> > nir/nir_phi_builder.h \
> > nir/nir_print.c \
> > diff --git a/src/compiler/nir/meson.build
> > b/src/compiler/nir/meson.build
> > index b0c3a7feb31..9555cc40e21 100644
> > --- a/src/compiler/nir/meson.build
> > +++ b/src/compiler/nir/meson.build
> > @@ -174,6 +174,7 @@ files_libnir = files(
> > 'nir_opt_shrink_load.c',
> > 'nir_opt_trivial_continues.c',
> > 'nir_opt_undef.c',
> > + 'nir_opt_vectorize_io.c',
> > 'nir_phi_builder.c',
> > 'nir_phi_builder.h',
> > 'nir_print.c',
> > diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> > index a0ae9a4430e..79bbdedaf00 100644
> > --- a/src/compiler/nir/nir.h
> > +++ b/src/compiler/nir/nir.h
> > @@ -3105,6 +3105,8 @@ bool
> nir_opt_trivial_continues(nir_shader
> > *shader);
> >
> > bool nir_opt_undef(nir_shader *shader);
> >
> > +bool nir_opt_vectorize_io(nir_shader *shader, bool
> skip_fs_inputs);
> > +
> > bool nir_opt_conditional_discard(nir_shader *shader);
> >
> > void nir_sweep(nir_shader *shader);
> > diff --git a/src/compiler/nir/nir_opt_vectorize_io.c
> > b/src/compiler/nir/nir_opt_vectorize_io.c
> > new file mode 100644
> > index 00000000000..c2ab30d307b
> > --- /dev/null
> > +++ b/src/compiler/nir/nir_opt_vectorize_io.c
> > @@ -0,0 +1,527 @@
> > +/*
> > + * Copyright © 2018 Timothy Arceri
> > + *
> > + * 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"
> > +#include "nir_deref.h"
> > +#include "util/u_dynarray.h"
> > +#include "util/u_math.h"
> > +
> > +/** @file nir_opt_vectorize_io.c
> > + *
> > + * Replaces scalar nir_load_input/nir_store_output
> operations with
> > + * vectorized instructions.
> > + */
> > +
> > +static bool
> > +is_io_load(nir_intrinsic_instr *intr)
> > +{
> > + switch (intr->intrinsic) {
> > + case nir_intrinsic_interp_deref_at_centroid:
> > + case nir_intrinsic_interp_deref_at_sample:
> > + case nir_intrinsic_interp_deref_at_offset:
> > + case nir_intrinsic_load_deref:
> > + return true;
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > +static nir_deref_instr *
> > +clone_deref_array(nir_builder *b, nir_deref_instr *dst_tail,
> > + const nir_deref_instr *src_head)
> > +{
> > + const nir_deref_instr *parent =
> > nir_deref_instr_parent(src_head);
> > +
> > + if (!parent)
> > + return dst_tail;
> > +
> > + assert(src_head->deref_type == nir_deref_type_array);
> > +
> > + dst_tail = clone_deref_array(b, dst_tail, parent);
> > +
> > + return nir_build_deref_array(b, dst_tail,
> > + nir_ssa_for_src(b,
> > src_head->arr.index, 1));
> > +}
> > +
> > +static bool
> > +instr_can_rewrite(nir_instr *instr)
> > +{
> > + if (instr->type != nir_instr_type_intrinsic)
> > + return false;
> > +
> > + nir_intrinsic_instr *intr =
> nir_instr_as_intrinsic(instr);
> > +
> > + if (intr->num_components != 1)
> > + return false;
> > +
> > + if (!is_io_load(intr) &&
> > + intr->intrinsic != nir_intrinsic_store_deref)
> > + return false;
> > +
> > + nir_variable *var =
> > +
> nir_deref_instr_get_variable(nir_src_as_deref(intr->src[0]));
> > +
> > + if (var->data.mode != nir_var_shader_in &&
> > + var->data.mode != nir_var_shader_out)
> > + return false;
> >
> >
> > Please check deref->mode before calling
> nir_deref_instr_get_variable
> > so the pass doesn't trip up on incomplete deref chains. We know
> > shader_in/out are currently safe but we can't get the variable in
> > general.
> >
> > +
> > + /* TODO: add doubles support ? */
> > + if (glsl_type_is_64bit(glsl_without_array(var->type)))
> >
> >
> > Maybe
> >
> > if (glsl_get_bit_size(glsl_without_array(var->type)) != 32)
> >
> > instead to be more future-proof?
> >
> > + return false;
> > +
> > + /* Only touch user defined varyings as these are the only
> > ones we split */
> > + if (var->data.location < VARYING_SLOT_VAR0 &&
> > var->data.location >= 0)
> >
> >
> > Why are we allowing negative locations?
> >
> > + return false;
> > +
> > + /* Skip complex types we don't split in the first
> place */
> > + if (glsl_type_is_matrix(glsl_without_array(var->type)) ||
> > + glsl_type_is_struct(glsl_without_array(var->type)))
> > + return false;
> >
> >
> > if
> (!glsl_type_is_vector_or_scalar(glsl_without_array(var->type)))
> >
> > instead? For that matter, glsl_type_is_scalar?
> >
> > +
> > + return true;
> > +}
> > +
> > +static bool
> > +io_access_same_var(const nir_instr *instr1, const nir_instr
> > *instr2)
> > +{
> > + assert(instr1->type == nir_instr_type_intrinsic &&
> > + instr2->type == nir_instr_type_intrinsic);
> > +
> > + nir_intrinsic_instr *intr1 =
> nir_instr_as_intrinsic(instr1);
> > + nir_intrinsic_instr *intr2 =
> nir_instr_as_intrinsic(instr2);
> > +
> > + nir_variable *var1 =
> > +
> >
> nir_deref_instr_get_variable(nir_src_as_deref(intr1->src[0]));
> > + nir_variable *var2 =
> > +
> >
> nir_deref_instr_get_variable(nir_src_as_deref(intr2->src[0]));
> > +
> > + /* We don't handle combining vars of different type e.g.
> > different array
> > + * lengths so just skip if the type doesn't match.
> > + */
> > + if (var1->type != var2->type)
> >
> >
> > Above, we allow vectors (maybe a mistake?) but here, we would
> fail
> > to combine vec2 and float.
> >
> > + return false;
> > +
> > + if (is_io_load(intr1) != is_io_load(intr2))
> > + return false;
> > +
> > + if (var1->data.location != var2->data.location)
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > +static struct util_dynarray *
> > +vec_instr_stack_create(void *mem_ctx)
> > +{
> > + struct util_dynarray *stack = ralloc(mem_ctx, struct
> > util_dynarray);
> > + util_dynarray_init(stack, mem_ctx);
> > + return stack;
> > +}
> > +
> > +static void
> > +vec_instr_stack_push(struct util_dynarray *stack,
> nir_instr *instr)
> > +{
> > + util_dynarray_append(stack, nir_instr *, instr);
> > +}
> > +
> > +static void
> > +create_new_load(nir_builder *b, nir_intrinsic_instr *intr,
> > nir_variable *var,
> > + unsigned comp, unsigned num_comps)
> > +{
> > + b->cursor = nir_before_instr(&intr->instr);
> > +
> > + assert(intr->dest.is_ssa);
> > +
> > + nir_intrinsic_instr *new_intr =
> > + nir_intrinsic_instr_create(b->shader,
> intr->intrinsic);
> > + nir_ssa_dest_init(&new_intr->instr, &new_intr->dest,
> num_comps,
> > + intr->dest.ssa.bit_size, NULL);
> > + new_intr->num_components = num_comps;
> > +
> > + nir_deref_instr *deref = nir_build_deref_var(b, var);
> > + deref = clone_deref_array(b, deref,
> > nir_src_as_deref(intr->src[0]));
> > +
> > + new_intr->src[0] = nir_src_for_ssa(&deref->dest.ssa);
> > +
> > + if (intr->intrinsic ==
> nir_intrinsic_interp_deref_at_offset ||
> > + intr->intrinsic ==
> nir_intrinsic_interp_deref_at_sample)
> > + nir_src_copy(&new_intr->src[1], &intr->src[1],
> > &new_intr->instr);
> > +
> > + nir_builder_instr_insert(b, &new_intr->instr);
> > +
> > + unsigned channel = comp - var->data.location_frac;
> > + nir_ssa_def *load = nir_channel(b, &new_intr->dest.ssa,
> > channel);
> > + nir_ssa_def_rewrite_uses(&intr->dest.ssa,
> > nir_src_for_ssa(load));
> > +
> > + /* Remove the old load intrinsic */
> > + nir_instr_remove(&intr->instr);
> > +}
> > +
> > +static void
> > +create_new_store(nir_builder *b, nir_intrinsic_instr *intr,
> > nir_variable *var,
> > + nir_ssa_def **srcs, unsigned first_comp,
> > unsigned num_comps)
> > +{
> > + b->cursor = nir_before_instr(&intr->instr);
> > +
> > + nir_intrinsic_instr *new_intr =
> > + nir_intrinsic_instr_create(b->shader,
> intr->intrinsic);
> > + new_intr->num_components = num_comps;
> > +
> > + nir_intrinsic_set_write_mask(new_intr, (1 <<
> num_comps) - 1);
> > +
> > + nir_deref_instr *deref = nir_build_deref_var(b, var);
> > + deref = clone_deref_array(b, deref,
> > nir_src_as_deref(intr->src[0]));
> > +
> > + new_intr->src[0] = nir_src_for_ssa(&deref->dest.ssa);
> > +
> > + nir_ssa_def *stores[4];
> > + for (unsigned i = 0; i < num_comps; i++) {
> > + stores[i] = srcs[first_comp + i];
> > + }
> > +
> > + new_intr->src[1] = nir_src_for_ssa(nir_vec(b, stores,
> > num_comps));
> > +
> > + nir_builder_instr_insert(b, &new_intr->instr);
> > +
> > + /* Remove the old store intrinsic */
> > + nir_instr_remove(&intr->instr);
> > +}
> > +
> > +static bool
> > +vec_instr_stack_pop(nir_builder *b, struct util_dynarray
> *stack,
> > + nir_instr *instr,
> > + nir_variable
> > *input_vars[MAX_VARYINGS_INCL_PATCH][4],
> > + nir_variable
> > *output_vars[MAX_VARYINGS_INCL_PATCH][4])
> > +{
> > + nir_instr *last = util_dynarray_pop(stack, nir_instr *);
> > +
> > + assert(last == instr);
> > + assert(last->type == nir_instr_type_intrinsic);
> > +
> > + nir_intrinsic_instr *intr = nir_instr_as_intrinsic(last);
> > + nir_variable *var =
> > +
> nir_deref_instr_get_variable(nir_src_as_deref(intr->src[0]));
> > + unsigned loc = var->data.location - VARYING_SLOT_VAR0;
> > +
> > + nir_variable *new_var;
> > + if (var->data.mode == nir_var_shader_in)
> > + new_var = input_vars[loc][var->data.location_frac];
> > + else
> > + new_var = output_vars[loc][var->data.location_frac];
> > +
> > + unsigned num_comps =
> > +
> glsl_get_vector_elements(glsl_without_array(new_var->type));
> > +
> > + /* Don't bother walking the stack if this component
> can't be
> > vectorised. */
> > + if (num_comps == 1)
> > + return false;
> > +
> > + if (new_var == var)
> > + return false;
> > +
> > + if (is_io_load(intr)) {
> > + create_new_load(b, intr, new_var,
> > var->data.location_frac, num_comps);
> > + return true;
> > + }
> > +
> > + b->cursor = nir_before_instr(last);
> > + nir_ssa_undef_instr *instr_undef =
> > + nir_ssa_undef_instr_create(b->shader, 1, 32);
> > + nir_builder_instr_insert(b, &instr_undef->instr);
> > +
> > + nir_ssa_def *srcs[4];
> > + for (int i = 0; i < 4; i++) {
> > + srcs[i] = &instr_undef->def;
> > + }
> > + srcs[var->data.location_frac] = intr->src[1].ssa;
> > +
> > + util_dynarray_foreach_reverse(stack, nir_instr *,
> stack_instr) {
> > + nir_intrinsic_instr *intr2 =
> > nir_instr_as_intrinsic(*stack_instr);
> > + nir_variable *var2 =
> > +
> >
> nir_deref_instr_get_variable(nir_src_as_deref(intr2->src[0]));
> > + unsigned loc2 = var2->data.location -
> VARYING_SLOT_VAR0;
> > +
> > + if (output_vars[loc][var->data.location_frac] !=
> > + output_vars[loc2][var2->data.location_frac])
> > + continue;
> > +
> > +
> >
> assert(glsl_get_vector_elements(glsl_without_array(var2->type))
> > == 1);
> > +
> > + if (srcs[var2->data.location_frac] ==
> &instr_undef->def) {
> > + assert(intr2->src[1].is_ssa);
> > + assert(intr2->src[1].ssa);
> > +
> > + srcs[var2->data.location_frac] = intr2->src[1].ssa;
> > + }
> > + }
> > +
> > + create_new_store(b, intr, new_var, srcs,
> > new_var->data.location_frac,
> > + num_comps);
> > +
> > + return true;
> > +}
> > +
> > +static bool
> > +cmp_func(const void *data1, const void *data2)
> > +{
> > + const struct util_dynarray *arr1 = data1;
> > + const struct util_dynarray *arr2 = data2;
> > +
> > + const nir_instr *instr1 = *(nir_instr
> > **)util_dynarray_begin(arr1);
> > + const nir_instr *instr2 = *(nir_instr
> > **)util_dynarray_begin(arr2);
> > +
> > + return io_access_same_var(instr1, instr2);
> > +}
> > +
> > +#define HASH(hash, data)
> _mesa_fnv32_1a_accumulate((hash), (data))
> > +
> > +static uint32_t
> > +hash_instr(const nir_instr *instr)
> > +{
> > + assert(instr->type == nir_instr_type_intrinsic);
> > +
> > + nir_intrinsic_instr *intr =
> nir_instr_as_intrinsic(instr);
> > + nir_variable *var =
> > +
> nir_deref_instr_get_variable(nir_src_as_deref(intr->src[0]));
> > +
> > + uint32_t hash = _mesa_fnv32_1a_offset_bias;
> > + bool is_load = is_io_load(intr);
> > +
> > + hash = HASH(hash, var->type);
> > + hash = HASH(hash, is_load);
> > + return HASH(hash, var->data.location);
> > +}
> > +
> > +static uint32_t
> > +hash_stack(const void *data)
> > +{
> > + const struct util_dynarray *stack = data;
> > + const nir_instr *first = *(nir_instr
> > **)util_dynarray_begin(stack);
> > + return hash_instr(first);
> > +}
> > +
> > +static struct set *
> > +vec_instr_set_create(void)
> > +{
> > + return _mesa_set_create(NULL, hash_stack, cmp_func);
> > +}
> > +
> > +static void
> > +vec_instr_set_destroy(struct set *instr_set)
> > +{
> > + _mesa_set_destroy(instr_set, NULL);
> > +}
> > +
> > +static void
> > +vec_instr_set_add(struct set *instr_set, nir_instr *instr)
> > +{
> > + if (!instr_can_rewrite(instr))
> > + return;
> > +
> > + struct util_dynarray *new_stack =
> > vec_instr_stack_create(instr_set);
> > + vec_instr_stack_push(new_stack, instr);
> > +
> > + struct set_entry *entry = _mesa_set_search(instr_set,
> > new_stack);
> > +
> > + if (entry) {
> > + ralloc_free(new_stack);
> > + struct util_dynarray *stack = (struct util_dynarray *)
> > entry->key;
> > + vec_instr_stack_push(stack, instr);
> > + return;
> > + }
> > +
> > + _mesa_set_add(instr_set, new_stack);
> > + return;
> > +}
> > +
> > +static bool
> > +vec_instr_set_remove(nir_builder *b, struct set *instr_set,
> > nir_instr *instr,
> > + nir_variable
> > *input_vars[MAX_VARYINGS_INCL_PATCH][4],
> > + nir_variable
> > *output_vars[MAX_VARYINGS_INCL_PATCH][4])
> > +{
> > + if (!instr_can_rewrite(instr))
> > + return false;
> > +
> > + /*
> > + * It's pretty unfortunate that we have to do this,
> but it's
> > a side effect
> > + * of the hash set interfaces. The hash set assumes that
> > we're only
> > + * interested in storing one equivalent element at a
> time,
> > and if we try to
> > + * insert a duplicate element it will remove the
> original.
> > We could hack up
> > + * the comparison function to "know" which input is an
> > instruction we
> > + * passed in and which is an array that's part of the
> entry,
> > but that
> > + * wouldn't work because we need to pass an array to
> > _mesa_set_add() in
> > + * vec_instr_add() above, and _mesa_set_add() will
> call our
> > comparison
> > + * function as well.
> > + */
> > + struct util_dynarray *temp =
> vec_instr_stack_create(instr_set);
> > + vec_instr_stack_push(temp, instr);
> > + struct set_entry *entry = _mesa_set_search(instr_set,
> temp);
> > + ralloc_free(temp);
> > +
> > + if (entry) {
> > + struct util_dynarray *stack = (struct util_dynarray *)
> > entry->key;
> > + bool progress = vec_instr_stack_pop(b, stack, instr,
> > input_vars,
> > + output_vars);
> > +
> > + if (!util_dynarray_num_elements(stack, nir_instr *))
> > + _mesa_set_remove(instr_set, entry);
> > +
> > + return progress;
> > + }
> > +
> > + return false;
> > +}
> > +
> > +static bool
> > +vectorize_block(nir_builder *b, nir_block *block, struct set
> > *instr_set,
> > + nir_variable
> > *input_vars[MAX_VARYINGS_INCL_PATCH][4],
> > + nir_variable
> > *output_vars[MAX_VARYINGS_INCL_PATCH][4])
> >
> >
> > No comments on the actal meat of it yet except one: Why is it so
> > complicated??? For inputs, just change loads to load the whole
> > vector and insert a swizzle after it to grab the component you
> > want. CSE will come along and clean up the mess by and
> large. For
> > stores, it's a bit more complicated but it still doesn't seem
> like
> > it needs to be this bad.
> >
> > +{
> > + bool progress = false;
> > +
> > + nir_foreach_instr_safe(instr, block) {
> > + vec_instr_set_add(instr_set, instr);
> > + }
> > +
> > + for (unsigned i = 0; i < block->num_dom_children; i++) {
> > + nir_block *child = block->dom_children[i];
> > + progress |= vectorize_block(b, child, instr_set,
> input_vars,
> > + output_vars);
> > + }
> > +
> > + nir_foreach_instr_reverse_safe(instr, block) {
> > + progress |= vec_instr_set_remove(b, instr_set, instr,
> > input_vars,
> > + output_vars);
> > + }
> > +
> > + return progress;
> > +}
> > +
> > +static void
> > +create_new_io_var(nir_shader *shader,
> > + nir_variable
> *vars[MAX_VARYINGS_INCL_PATCH][4],
> > + unsigned location, unsigned comps)
> > +{
> > + unsigned num_comps = util_bitcount(comps);
> >
> >
> > Do we need to do anything if num_comps == 1? Or can we just
> leave
> > it alone?
> >
> > + unsigned first_comp = u_bit_scan(&comps);
> >
> >
> > Might be worth a quick comment here that we're taking a component
> > off. Using u_bit_scan here is very nice from the perspective
> that
> > it helps the loop below but it's not obviuos.
> >
> > +
> > + nir_variable *var =
> > nir_variable_clone(vars[location][first_comp], shader);
> > + var->data.location_frac = first_comp;
> > + var->type = glsl_replace_vector_type(var->type,
> num_comps);
> > +
> > + nir_shader_add_variable(shader, var);
> > +
> > + vars[location][first_comp] = var;
> > +
> > + while (comps) {
> > + const int comp = u_bit_scan(&comps);
> > + vars[location][comp] = var;
> > + }
> > +}
> > +
> > +static void
> > +create_new_io_vars(nir_shader *shader, struct exec_list
> *io_list,
> > + nir_variable
> *vars[MAX_VARYINGS_INCL_PATCH][4])
> > +{
> > + if (exec_list_is_empty(io_list))
> > + return;
> > +
> > + nir_foreach_variable(var, io_list) {
> > + if (var->data.location >= VARYING_SLOT_VAR0) {
> >
> >
> > Do we want to check that it's a (possibly array of) scalar or
> > anything like that?
> >
> > + unsigned loc = var->data.location -
> VARYING_SLOT_VAR0;
> > + vars[loc][var->data.location_frac] = var;
> > + }
> > + }
> > +
> > + /* We don't handle combining vars of different type e.g.
> > different array
> > + * lengths.
> > + */
> > + for (unsigned i = 0; i < MAX_VARYINGS_INCL_PATCH; i++) {
> > + unsigned comps = 0;
> > + for (unsigned j = 0; j < 3; j++) {
> > + if (vars[i][j] && vars[i][j+1] &&
> vars[i][j]->type ==
> > vars[i][j+1]->type) {
> > + if (j == 2) {
> > + /* last component so create new variable */
> > + comps |= 3 << vars[i][j]->data.location_frac;
> >
> >
> > Why not move setting comps out of the if? Might make things
> clearer.
> >
> > + create_new_io_var(shader, vars, i, comps);
> > + } else {
> > + /* Set comps */
> > + comps |= 3 << vars[i][j]->data.location_frac;
> > + }
> > + } else {
> > + if (comps) {
> > + /* Types didn't match but we have already
> seen
> > matching types
> > + * at this location so create a new
> variable for
> > those
> > + * components.
> > + */
> > + create_new_io_var(shader, vars, i, comps);
> > + comps = 0;
> > + }
> > + }
> > + }
> > + }
> > +}
> > +
> > +static bool
> > +nir_opt_vectorize_io_impl(nir_function_impl *impl)
> > +{
> > + nir_builder b;
> > + nir_builder_init(&b, impl);
> > +
> > + nir_metadata_require(impl, nir_metadata_dominance);
> > +
> > + nir_shader *shader = impl->function->shader;
> > + nir_variable *input_vars[MAX_VARYINGS_INCL_PATCH][4]
> = {0};
> > + nir_variable *output_vars[MAX_VARYINGS_INCL_PATCH][4]
> = {0};
> > +
> > + create_new_io_vars(shader, &shader->inputs, input_vars);
> > + create_new_io_vars(shader, &shader->outputs,
> output_vars);
> > +
> > + struct set *instr_set = vec_instr_set_create();
> > + bool progress = vectorize_block(&b,
> nir_start_block(impl),
> > instr_set,
> > + input_vars, output_vars);
> > +
> > + if (progress)
> > + nir_metadata_preserve(impl, nir_metadata_block_index |
> > + nir_metadata_dominance);
> >
> >
> > Two lines. Please use { }.
> >
> > +
> > + vec_instr_set_destroy(instr_set);
> > + return false;
> > +}
> > +
> > +bool
> > +nir_opt_vectorize_io(nir_shader *shader, bool
> skip_fs_inputs)
> > +{
> > + bool progress = false;
> > +
> > + if (skip_fs_inputs && shader->info.stage ==
> > MESA_SHADER_FRAGMENT)
> > + return false;
> > +
> > + nir_foreach_function(function, shader) {
> > + if (function->impl)
> > + progress |=
> nir_opt_vectorize_io_impl(function->impl);
> > + }
> > +
> > + return progress;
> > +}
> > --
> > 2.19.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> <mailto:mesa-dev at lists.freedesktop.org>
> > <mailto:mesa-dev at lists.freedesktop.org
> <mailto:mesa-dev at lists.freedesktop.org>>
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
>
More information about the mesa-dev
mailing list