On 11/03/2015 09:31 AM, Kenneth Graunke wrote: > We can't preserve dominance or live variable information. > > This also begs the question: what about globals? Metadata only exists > at the nir_function_impl level, so it would seem there is no metadata > about global variables for us to invalidate. >
Shouldn't globals be always lowered to locals prior to invoking this pass? That's what i965 does. Are there other drivers using nir, that deal with global vars? > Signed-off-by: Kenneth Graunke <[email protected]> > --- > src/glsl/nir/nir_remove_dead_variables.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/src/glsl/nir/nir_remove_dead_variables.c > b/src/glsl/nir/nir_remove_dead_variables.c > index d6783e7..77b6c13 100644 > --- a/src/glsl/nir/nir_remove_dead_variables.c > +++ b/src/glsl/nir/nir_remove_dead_variables.c > @@ -126,8 +126,13 @@ nir_remove_dead_variables(nir_shader *shader) > progress = remove_dead_vars(&shader->globals, live) || progress; At least in i965, where nir_lower_global_vars_to_local() runs prior to this pass, only dead global variables can reach this point. So metadata seems irrelevant here. I tested this quickly with the following patch, and saw no new crashes running several tests: diff --git a/src/glsl/nir/nir_remove_dead_variables.c b/src/glsl/nir/nir_remove_dead_variables.c index d6783e7..7db48eb 100644 --- a/src/glsl/nir/nir_remove_dead_variables.c +++ b/src/glsl/nir/nir_remove_dead_variables.c @@ -124,6 +124,7 @@ nir_remove_dead_variables(nir_shader *shader) add_var_use_shader(shader, live); progress = remove_dead_vars(&shader->globals, live) || progress; + assert(exec_list_length(&shader->globals) == 0); nir_foreach_overload(shader, overload) { if (overload->impl) Eduardo > > nir_foreach_overload(shader, overload) { > - if (overload->impl) > - progress = remove_dead_vars(&overload->impl->locals, live) || > progress; > + if (overload->impl) { > + if (remove_dead_vars(&overload->impl->locals, live)) { > + nir_metadata_preserve(overload->impl, nir_metadata_block_index | > + nir_metadata_dominance); > + progress = true; > + } > + } > } > > _mesa_set_destroy(live, NULL); > _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
