From cc59503b161424f4d1e3f7214967233be9e1f093 Mon Sep 17 00:00:00 2001 From: Jason Ekstrand Date: Mon, 4 Mar 2019 12:12:48 -0600 Subject: [PATCH] nir/vars_to_ssa: Properly ignore variables with complex derefs Because the core principle of the vars_to_ssa pass is that it globally (within a function) looks at all of the uses of a never-indirected path and does a full into-SSA on that path, it can't handle a path which has any chance of having aliasing. If a function_temp variable has a cast or anything else which may cause aliasing, we have to assume that all paths to that variable may alias and ignore the entire variable. Reviewed-by: Dave Airlie Reviewed-by: Caio Marcelo de Oliveira Filho --- src/compiler/nir/nir_lower_vars_to_ssa.c | 78 +++++++++++++++++++----- 1 file changed, 64 insertions(+), 14 deletions(-) diff --git a/src/compiler/nir/nir_lower_vars_to_ssa.c b/src/compiler/nir/nir_lower_vars_to_ssa.c index b1b26c1fbf2..d04d84f9e20 100644 --- a/src/compiler/nir/nir_lower_vars_to_ssa.c +++ b/src/compiler/nir/nir_lower_vars_to_ssa.c @@ -56,6 +56,12 @@ struct deref_node { */ bool is_direct; + /* Set on a root node for a variable to indicate that variable is used by a + * cast or passed through some other sequence of instructions that are not + * derefs. + */ + bool has_complex_use; + struct deref_node *wildcard; struct deref_node *indirect; struct deref_node *children[0]; @@ -147,8 +153,13 @@ get_deref_node_recur(nir_deref_instr *deref, if (deref->deref_type == nir_deref_type_var) return get_deref_node_for_var(deref->var, state); + if (deref->deref_type == nir_deref_type_cast) + return NULL; + struct deref_node *parent = get_deref_node_recur(nir_deref_instr_parent(deref), state); + if (parent == NULL) + return NULL; switch (deref->deref_type) { case nir_deref_type_struct: @@ -363,9 +374,28 @@ path_may_be_aliased(nir_deref_path *path, { assert(path->path[0]->deref_type == nir_deref_type_var); nir_variable *var = path->path[0]->var; + struct deref_node *var_node = get_deref_node_for_var(var, state); + + /* First see if this variable is ever used by anything other than a + * load/store. If there's even so much as a cast in the way, we have to + * assume aliasing and bail. + */ + if (var_node->has_complex_use) + return true; + + return path_may_be_aliased_node(var_node, &path->path[1], state); +} + +static void +register_complex_use(nir_deref_instr *deref, + struct lower_variables_state *state) +{ + assert(deref->deref_type == nir_deref_type_var); + struct deref_node *node = get_deref_node_for_var(deref->var, state); + if (node == NULL) + return; - return path_may_be_aliased_node(get_deref_node_for_var(var, state), - &path->path[1], state); + node->has_complex_use = true; } static void @@ -421,26 +451,41 @@ register_variable_uses(nir_function_impl *impl, { nir_foreach_block(block, impl) { nir_foreach_instr_safe(instr, block) { - if (instr->type != nir_instr_type_intrinsic) - continue; + switch (instr->type) { + case nir_instr_type_deref: { + nir_deref_instr *deref = nir_instr_as_deref(instr); - nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr); + if (deref->deref_type == nir_deref_type_var && + nir_deref_instr_has_complex_use(deref)) + register_complex_use(deref, state); - switch (intrin->intrinsic) { - case nir_intrinsic_load_deref: - register_load_instr(intrin, state); break; + } - case nir_intrinsic_store_deref: - register_store_instr(intrin, state); - break; + case nir_instr_type_intrinsic: { + nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr); + + switch (intrin->intrinsic) { + case nir_intrinsic_load_deref: + register_load_instr(intrin, state); + break; + + case nir_intrinsic_store_deref: + register_store_instr(intrin, state); + break; + + case nir_intrinsic_copy_deref: + register_copy_instr(intrin, state); + break; - case nir_intrinsic_copy_deref: - register_copy_instr(intrin, state); + default: + continue; + } break; + } default: - continue; + break; } } } @@ -510,6 +555,9 @@ rename_variables(struct lower_variables_state *state) continue; struct deref_node *node = get_deref_node(deref, state); + if (node == NULL) + continue; + if (node == UNDEF_NODE) { /* If we hit this path then we are referencing an invalid * value. Most likely, we unrolled something and are @@ -560,6 +608,8 @@ rename_variables(struct lower_variables_state *state) continue; struct deref_node *node = get_deref_node(deref, state); + if (node == NULL) + continue; assert(intrin->src[1].is_ssa); nir_ssa_def *value = intrin->src[1].ssa; -- 2.30.2