[Mesa-dev] [PATCH 4/8] nir: add some helpers for doing linking
Kenneth Graunke
kenneth at whitecape.org
Mon Sep 25 21:51:23 UTC 2017
On Tuesday, September 12, 2017 4:37:31 PM PDT Timothy Arceri wrote:
> The initial helpers as support for removing unused varyings between
> stages.
> ---
> src/compiler/Makefile.sources | 1 +
> src/compiler/nir/nir.h | 6 ++
> src/compiler/nir/nir_linking_helpers.c | 136 +++++++++++++++++++++++++++++++++
> 3 files changed, 143 insertions(+)
> create mode 100644 src/compiler/nir/nir_linking_helpers.c
>
> diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources
> index 0153df2d812..9c7f057eecf 100644
> --- a/src/compiler/Makefile.sources
> +++ b/src/compiler/Makefile.sources
> @@ -203,6 +203,7 @@ NIR_FILES = \
> nir/nir_instr_set.h \
> nir/nir_intrinsics.c \
> nir/nir_intrinsics.h \
> + nir/nir_linking_helpers.c \
> nir/nir_liveness.c \
> nir/nir_loop_analyze.c \
> nir/nir_loop_analyze.h \
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index e52a1006896..1e89c74d14c 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -2448,6 +2448,12 @@ void nir_shader_gather_info(nir_shader *shader, nir_function_impl *entrypoint);
> void nir_assign_var_locations(struct exec_list *var_list, unsigned *size,
> int (*type_size)(const struct glsl_type *));
>
> +/* Some helpers to do very simple linking */
> +bool nir_remove_unwritten_outputs(nir_shader *shader);
> +bool nir_remove_unread_outputs(nir_shader *shader, uint64_t outputs_read);
> +bool nir_remove_unused_varyings(nir_shader *producer, nir_shader *consumer);
> +bool nir_compact_varyings(nir_shader *producer, nir_shader *consumer);
> +
> typedef enum {
> /* If set, this forces all non-flat fragment shader inputs to be
> * interpolated as if with the "sample" qualifier. This requires
> diff --git a/src/compiler/nir/nir_linking_helpers.c b/src/compiler/nir/nir_linking_helpers.c
> new file mode 100644
> index 00000000000..d567aa713ad
> --- /dev/null
> +++ b/src/compiler/nir/nir_linking_helpers.c
> @@ -0,0 +1,136 @@
> +/*
> + * Copyright © 2015 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 "util/set.h"
> +#include "util/hash_table.h"
> +
> +/* This file contains various little helpers for doing simple linking in
> + * NIR. Eventually, we'll probably want a full-blown varying packing
> + * implementation in here. Right now, it just deletes unused things.
> + */
> +
> +static void
> +find_live_tcs_outputs(nir_shader *shader, struct set *live)
Name's a bit misleading, since it only considers which outputs are read,
maybe find_read_tcs_outputs()?
It's a bit odd that the whole pass works in terms of bitfields, but here
we have a set of vars. It definitely works, but it might be cleaner to
do something like:
uint64_t
tcs_output_read_bitmask(nir_shader *shader)
{
uint64_t read = 0;
nir_foreach_function(function, shader) {
if (function->impl) {
nir_foreach_block(block, function->impl) {
nir_foreach_instr(instr, block) {
if (instr->type == nir_instr_type_intrinsic) {
nir_intrinsic_instr *intrin_instr =
nir_instr_as_intrinsic(instr);
if (intrin_instr->intrinsic == nir_intrinsic_load_var &&
intrin_instr->variables[0]->var->data.mode ==
nir_var_shader_out) {
read |= nir_variable_get_io_mask(var, shader->stage);
}
}
}
}
}
}
return read;
}
and below in nir_remove_unused_varyings,
if (producer->stage == MESA_SHADER_TESS_CTRL)
read |= tcs_output_read_bitmask(producer);
then you wouldn't need the special case in remove_unused_io_vars.
> +{
> + nir_foreach_function(function, shader) {
> + if (function->impl) {
> + nir_foreach_block(block, function->impl) {
> + nir_foreach_instr(instr, block) {
> + if (instr->type == nir_instr_type_intrinsic) {
> + nir_intrinsic_instr *intrin_instr =
> + nir_instr_as_intrinsic(instr);
> + if (intrin_instr->intrinsic == nir_intrinsic_load_var &&
> + intrin_instr->variables[0]->var->data.mode ==
> + nir_var_shader_out) {
> + _mesa_set_add(live, intrin_instr->variables[0]->var);
> + }
> + }
> + }
> + }
> + }
> + }
> +}
> +
> +static bool
> +remove_unused_io_vars(nir_shader *shader, struct exec_list *var_list,
> + uint64_t used_by_other_stage,
> + struct set *live_tcs_outputs)
> +{
> + bool progress = false;
> +
> + nir_foreach_variable_safe(var, var_list) {
> + /* TODO: add patch support */
> + if (var->data.patch)
> + continue;
Patch variable handling should be easy, you just need a second bitfield,
one for regular variables and the other for patch variables, then select
which bitfield to look at here.
You'd need the IO mask generator to handle it, which it would if you
stole the one from nir_gather_info.
> +
> + if (var->data.location < VARYING_SLOT_VAR0 && var->data.location >= 0)
> + continue;
and would have to adjust this I guess.
Other than the TCS handling, this patch looks good to me. Nice and
simple :)
> +
> + if (var->data.always_active_io)
> + continue;
> +
> + if (!(used_by_other_stage &
> + nir_variable_get_io_mask(var, shader->stage))) {
> + /* Each TCS invocation can read data written by other TCS invocations,
> + * so even if the outputs are not used by the TES we must also make
> + * sure they are not read by the TCS before demoting them to globals.
> + */
> + if (live_tcs_outputs) {
> + assert(shader->stage == MESA_SHADER_TESS_CTRL &&
> + var->data.mode == nir_var_shader_out);
> +
> + struct set_entry *entry = _mesa_set_search(live_tcs_outputs, var);
> + if (entry) {
> + continue;
> + }
> + }
> +
> + /* This one is invalid, make it a global variable instead */
> + var->data.location = 0;
> + var->data.mode = nir_var_global;
> +
> + exec_node_remove(&var->node);
> + exec_list_push_tail(&shader->globals, &var->node);
> +
> + progress = true;
> + }
> + }
> +
> + return progress;
> +}
> +
> +bool
> +nir_remove_unused_varyings(nir_shader *producer, nir_shader *consumer)
> +{
> + assert(producer->stage != MESA_SHADER_FRAGMENT);
> + assert(consumer->stage != MESA_SHADER_VERTEX);
> +
> + uint64_t read = 0, written = 0;
> +
> + nir_foreach_variable(var, &producer->outputs)
> + written |= nir_variable_get_io_mask(var, producer->stage);
> +
> + nir_foreach_variable(var, &consumer->inputs)
> + read |= nir_variable_get_io_mask(var, consumer->stage);
> +
> + struct set *live_tcs_outputs;
> + if (producer->stage == MESA_SHADER_TESS_CTRL) {
> + live_tcs_outputs =
> + _mesa_set_create(NULL, _mesa_hash_pointer, _mesa_key_pointer_equal);
> + find_live_tcs_outputs(producer, live_tcs_outputs);
> + } else {
> + live_tcs_outputs = NULL;
> + }
> +
> + bool progress = false;
> + progress = remove_unused_io_vars(producer, &producer->outputs, read,
> + live_tcs_outputs);
> +
> + progress = remove_unused_io_vars(consumer, &consumer->inputs, written,
> + NULL) || progress;
> +
> + _mesa_set_destroy(live_tcs_outputs, NULL);
> +
> + return progress;
> +}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170925/699d9337/attachment.sig>
More information about the mesa-dev
mailing list