<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Wed, Aug 15, 2018 at 4:57 PM Caio Marcelo de Oliveira Filho <<a href="mailto:caio.oliveira@intel.com" target="_blank">caio.oliveira@intel.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Instead of doing this as part of the existing (local) copy prop vars pass. This is an intermediate step before changing both the dead write and the copy prop vars to act on the whole program instead of on local blocks. The nature of data we store and the way we iterate is different enough that would be awkward keeping those together. --- src/compiler/Makefile.sources | 1 + src/compiler/nir/meson.build | 1 + src/compiler/nir/nir.h | 2 + src/compiler/nir/nir_opt_dead_write_vars.c | 243 +++++++++++++++++++++ 4 files changed, 247 insertions(+) create mode 100644 src/compiler/nir/nir_opt_dead_write_vars.c diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources index 27a54e0be09..fa93ad08a16 100644 --- a/src/compiler/Makefile.sources +++ b/src/compiler/Makefile.sources @@ -274,6 +274,7 @@ NIR_FILES = \ nir/nir_opt_cse.c \ nir/nir_opt_dce.c \ nir/nir_opt_dead_cf.c \ + nir/nir_opt_dead_write_vars.c \ nir/nir_opt_gcm.c \ nir/nir_opt_global_to_local.c \ nir/nir_opt_if.c \ diff --git a/src/compiler/nir/meson.build b/src/compiler/nir/meson.build index 8708f9b069c..1c164a548a7 100644 --- a/src/compiler/nir/meson.build +++ b/src/compiler/nir/meson.build @@ -158,6 +158,7 @@ files_libnir = files( 'nir_opt_cse.c', 'nir_opt_dce.c', 'nir_opt_dead_cf.c', + 'nir_opt_dead_write_vars.c', 'nir_opt_gcm.c', 'nir_opt_global_to_local.c', 'nir_opt_if.c', diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index d0fa693884b..becf6e351c3 100644 --- a/src/compiler/nir/nir.h +++ b/src/compiler/nir/nir.h @@ -2968,6 +2968,8 @@ bool nir_opt_dce(nir_shader *shader); bool nir_opt_dead_cf(nir_shader *shader); +bool nir_opt_dead_write_vars(nir_shader *shader); + bool nir_opt_gcm(nir_shader *shader, bool value_number); bool nir_opt_if(nir_shader *shader); diff --git a/src/compiler/nir/nir_opt_dead_write_vars.c b/src/compiler/nir/nir_opt_dead_write_vars.c new file mode 100644 index 00000000000..822bfa5595d --- /dev/null +++ b/src/compiler/nir/nir_opt_dead_write_vars.c @@ -0,0 +1,243 @@ +/* + * 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" +#include "nir_deref.h" + +#include "util/u_dynarray.h" + +struct state { + void *mem_ctx; + + /* Maps nir_deref_instr to a corresponding nir_deref_path. Avoids + * rebuilding the paths for the same deref. */ + struct hash_table *paths; + void *path_lin_ctx; +}; + +static nir_deref_path * +get_path(struct state *state, nir_deref_instr *deref) +{ + struct hash_entry *entry = _mesa_hash_table_search(state->paths, deref); + if (!entry) { + nir_deref_path *path = linear_zalloc_child(state->path_lin_ctx, sizeof(nir_deref_path)); + nir_deref_path_init(path, deref, state->mem_ctx); + _mesa_hash_table_insert(state->paths, deref, path); + return path; + } else { + return entry->data; + } +} </blockquote><div> </div><div>Do you have any proof that this actually helps? The deref_path stuff was designed to be put on the stack and absolutely as efficient as possible. In the common case of a deref chain with only a couple of elements, I would expect to actually be more work to look it up in a hash table. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + +/* Entry for unused_writes arrays. */ +struct write_entry { + /* If NULL indicates the entry is free to be reused. */ + nir_intrinsic_instr *intrin; + uintptr_t mask; + nir_deref_path *path; +}; + +static void +clear_unused_for_modes(struct util_dynarray *unused_writes, nir_variable_mode modes) +{ + util_dynarray_foreach(unused_writes, struct write_entry, entry) { + if (!entry->intrin) + continue; + nir_variable *var = entry->path->path[0]->var; + if (var->data.mode & modes) + entry->intrin = NULL; + } +} + +static void +clear_unused_for_src(struct util_dynarray *unused_writes, nir_deref_path *src_path) +{ + util_dynarray_foreach(unused_writes, struct write_entry, entry) { + if (!entry->intrin) + continue; + if (nir_compare_deref_paths(src_path, entry->path) & nir_derefs_may_alias_bit) + entry->intrin = NULL; + } +} + +static bool +update_unused_writes_with_dst(struct util_dynarray *unused_writes, + nir_intrinsic_instr *intrin, + nir_deref_path *dst_path, uintptr_t mask) +{ + /* If we see an empty entry, keep track of it for reuse later. */ + struct write_entry *empty_entry = NULL; + bool progress = false; + + /* Find writes that are unused and can be removed. */ + util_dynarray_foreach(unused_writes, struct write_entry, entry) { + if (!entry->intrin) { + empty_entry = entry; + continue; + } + nir_deref_compare_result comp = nir_compare_deref_paths(dst_path, entry->path); + if (comp & nir_derefs_a_contains_b_bit) { + entry->mask &= ~mask; + if (entry->mask == 0) { + nir_instr_remove(&entry->intrin->instr); + entry->intrin = NULL; + empty_entry = entry; + progress = true; + } + } + } + + /* Add the new write to the unused array. */ + struct write_entry new_entry = { + .intrin = intrin, + .mask = mask, + .path = dst_path, + }; + + if (empty_entry) + *empty_entry = new_entry; + else + util_dynarray_append(unused_writes, struct write_entry, new_entry); + + return progress; +} + +static bool +remove_dead_write_vars_local(struct state *state, nir_block *block) +{ + bool progress = false; + + struct util_dynarray unused_writes; + util_dynarray_init(&unused_writes, state->mem_ctx); + + nir_foreach_instr_safe(instr, block) { </blockquote><div> </div><div>It wouldn't hurt to add a case for call instructions which does a barrier on everything I mentioned below as well as globals and locals.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + if (instr->type != nir_instr_type_intrinsic) + continue; + + nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr); + switch (intrin->intrinsic) { + case nir_intrinsic_barrier: + case nir_intrinsic_memory_barrier: { + nir_variable_mode modes = ~(nir_var_local | nir_var_global | + nir_var_shader_in | nir_var_uniform); </blockquote><div> </div><div>The only thing a barrier like this affects is shared, storage, and output. Locals and globals can't cross between shader channels so there's no reason to do anything with them on a barrier. For inputs and uniforms, they're never written anyway so there's no point in doing anything with them on a barrier. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + clear_unused_for_modes(&unused_writes, modes); </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + break; + } + + case nir_intrinsic_emit_vertex: + case nir_intrinsic_emit_vertex_with_counter: { + nir_variable_mode modes = nir_var_shader_out; + clear_unused_for_modes(&unused_writes, modes); </blockquote><div> </div><div>Just pass nir_var_shader_out in here; no reason to make a temporary variable. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + break; + } + + case nir_intrinsic_load_deref: { + nir_deref_instr *src = nir_src_as_deref(intrin->src[0]); + nir_deref_path *src_path = get_path(state, src); + + clear_unused_for_src(&unused_writes, src_path); + break; + } + + case nir_intrinsic_store_deref: { + nir_deref_instr *dst = nir_src_as_deref(intrin->src[0]); + nir_deref_path *dst_path = get_path(state, dst); + + uintptr_t mask = nir_intrinsic_write_mask(intrin); + progress |= update_unused_writes_with_dst(&unused_writes, intrin, dst_path, mask); + break; + } + + case nir_intrinsic_copy_deref: { + nir_deref_instr *src = nir_src_as_deref(intrin->src[1]); + nir_deref_path *src_path = get_path(state, src); + + nir_deref_instr *dst = nir_src_as_deref(intrin->src[0]); + nir_deref_path *dst_path = get_path(state, dst); + + /* Self-copy is removed. */ + if (src == dst || (nir_compare_deref_paths(src_path, dst_path) & nir_derefs_equal_bit)) { </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + nir_instr_remove(instr); + progress = true; + break; + } + + uintptr_t mask = ~(1 << NIR_MAX_VEC_COMPONENTS); + clear_unused_for_src(&unused_writes, src_path); + progress |= update_unused_writes_with_dst(&unused_writes, intrin, dst_path, mask); + break; + } + + default: + break; + } + } + + /* All unused writes at the end of the block are kept, since we can't be + * sure they'll be overwritten or not with local analysis only. + */ + + return progress; +} + +static bool +remove_dead_write_vars_impl(void *mem_ctx, nir_function_impl *func) +{ + bool progress = false; + + struct state state = { + .mem_ctx = mem_ctx, + + .path_lin_ctx = linear_alloc_parent(mem_ctx, 0), + .paths = _mesa_hash_table_create(mem_ctx, _mesa_hash_pointer, _mesa_key_pointer_equal), + }; + + nir_foreach_block(block, func) + progress |= remove_dead_write_vars_local(&state, block); + + return progress; +} + +bool +nir_opt_dead_write_vars(nir_shader *shader) +{ + void *mem_ctx = ralloc_context(NULL); + bool progress = false; + + nir_foreach_function(function, shader) { + if (!function->impl) + continue; + + nir_metadata_require(function->impl, nir_metadata_block_index); + + if (remove_dead_write_vars_impl(mem_ctx, function->impl)) { + nir_metadata_preserve(function->impl, nir_metadata_block_index | + nir_metadata_dominance); </blockquote><div> </div><div>I'd put the metadata stuff in the _impl version in case we ever want to expose it for use directly. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + progress = true; + } + } + + ralloc_free(mem_ctx); + return progress; +} -- 2.18.0 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div></div>