From 7f6491b76d51f35e76715275124d4a8d2eaf8db1 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Thu, 6 Apr 2023 13:19:31 -0400 Subject: [PATCH] nir: Combine if_uses with instruction uses Every nir_ssa_def is part of a chain of uses, implemented with doubly linked lists. That means each requires 2 * 64-bit = 16 bytes per def, which is memory intensive. Together they require 32 bytes per def. Not cool. To cut that memory use in half, we can combine the two linked lists into a single use list that contains both regular instruction uses and if-uses. To do this, we augment the nir_src with a boolean "is_if", and reimplement the abstract if-uses operations on top of that list. That boolean should fit into the padding already in nir_src so should not actually affect memory use, and in the future we sneak it into the bottom bit of a pointer. However, this creates a new inefficiency: now iterating over regular uses separate from if-uses is (nominally) more expensive. It turns out virtually every caller of nir_foreach_if_use(_safe) also calls nir_foreach_use(_safe) immediately before, so we rewrite most of the callers to instead call a new single `nir_foreach_use_including_if(_safe)` which predicates the logic based on `src->is_if`. This should mitigate the performance difference. There's a bit of churn, but this is largely a mechanical set of changes. Signed-off-by: Alyssa Rosenzweig Reviewed-by: Faith Ekstrand Part-of: --- src/compiler/nir/nir.c | 101 ++++++++---------- src/compiler/nir/nir.h | 54 +++++++--- src/compiler/nir/nir_clone.c | 3 +- src/compiler/nir/nir_control_flow.c | 6 +- src/compiler/nir/nir_deref.c | 15 ++- src/compiler/nir/nir_from_ssa.c | 8 +- src/compiler/nir/nir_loop_analyze.c | 4 +- src/compiler/nir/nir_lower_io.c | 3 +- src/compiler/nir/nir_lower_regs_to_ssa.c | 1 - src/compiler/nir/nir_lower_to_source_mods.c | 10 +- src/compiler/nir/nir_lower_vec_to_movs.c | 8 +- src/compiler/nir/nir_opt_copy_propagate.c | 9 +- src/compiler/nir/nir_opt_dead_cf.c | 22 ++-- src/compiler/nir/nir_opt_if.c | 42 +++----- src/compiler/nir/nir_opt_intrinsics.c | 6 +- src/compiler/nir/nir_opt_peephole_select.c | 9 +- src/compiler/nir/nir_opt_phi_precision.c | 14 +-- src/compiler/nir/nir_opt_ray_queries.c | 7 +- .../nir/nir_opt_rematerialize_compares.c | 72 ++++++------- src/compiler/nir/nir_opt_sink.c | 48 ++++----- src/compiler/nir/nir_opt_uniform_atomics.c | 1 - src/compiler/nir/nir_repair_ssa.c | 65 +++++------ src/compiler/nir/nir_search_helpers.h | 22 +--- src/compiler/nir/nir_serialize.c | 1 - src/compiler/nir/nir_to_lcssa.c | 30 +++--- src/compiler/nir/nir_validate.c | 39 +++---- .../nir/tests/ssa_def_bits_used_tests.cpp | 3 +- src/freedreno/ir3/ir3_nir_opt_preamble.c | 14 ++- src/gallium/auxiliary/nir/nir_to_tgsi.c | 10 +- .../drivers/etnaviv/etnaviv_compiler_nir.c | 6 +- .../drivers/etnaviv/etnaviv_compiler_nir.h | 7 +- src/gallium/drivers/vc4/vc4_program.c | 9 +- src/intel/compiler/brw_fs.cpp | 8 +- .../compiler/brw_nir_opt_peephole_ffma.c | 11 +- src/microsoft/compiler/dxil_nir_tess.c | 18 ++-- src/panfrost/.clang-format | 2 + src/panfrost/midgard/nir_fuse_io_16.c | 7 +- src/panfrost/util/nir_mod_helpers.c | 6 +- 38 files changed, 329 insertions(+), 372 deletions(-) diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c index 279cc571c37..d70fdb60a84 100644 --- a/src/compiler/nir/nir.c +++ b/src/compiler/nir/nir.c @@ -225,7 +225,6 @@ reg_create(void *mem_ctx, struct exec_list *list) list_inithead(®->uses); list_inithead(®->defs); - list_inithead(®->if_uses); reg->num_components = 0; reg->bit_size = 32; @@ -888,7 +887,7 @@ nir_phi_instr_add_src(nir_phi_instr *instr, nir_block *pred, nir_src src) phi_src = gc_zalloc(gc_get_context(instr), nir_phi_src, 1); phi_src->pred = pred; phi_src->src = src; - phi_src->src.parent_instr = &instr->instr; + nir_src_set_parent_instr(&phi_src->src, &instr->instr); exec_list_push_tail(&instr->srcs, &phi_src->node); return phi_src; @@ -1056,7 +1055,7 @@ add_use_cb(nir_src *src, void *state) { nir_instr *instr = state; - src->parent_instr = instr; + nir_src_set_parent_instr(src, instr); list_addtail(&src->use_link, src->is_ssa ? &src->ssa->uses : &src->reg.reg->uses); @@ -1633,19 +1632,16 @@ src_add_all_uses(nir_src *src, nir_instr *parent_instr, nir_if *parent_if) continue; if (parent_instr) { - src->parent_instr = parent_instr; - if (src->is_ssa) - list_addtail(&src->use_link, &src->ssa->uses); - else - list_addtail(&src->use_link, &src->reg.reg->uses); + nir_src_set_parent_instr(src, parent_instr); } else { assert(parent_if); - src->parent_if = parent_if; - if (src->is_ssa) - list_addtail(&src->use_link, &src->ssa->if_uses); - else - list_addtail(&src->use_link, &src->reg.reg->if_uses); + nir_src_set_parent_if(src, parent_if); } + + if (src->is_ssa) + list_addtail(&src->use_link, &src->ssa->uses); + else + list_addtail(&src->use_link, &src->reg.reg->uses); } } @@ -1677,7 +1673,7 @@ nir_if_rewrite_condition(nir_if *if_stmt, nir_src new_src) { nir_shader *shader = ralloc_parent(if_stmt); nir_src *src = &if_stmt->condition; - assert(!src_is_valid(src) || src->parent_if == if_stmt); + assert(!src_is_valid(src) || (src->is_if && src->parent_if == if_stmt)); src_remove_all_uses(src); src_copy(src, &new_src, shader->gctx); @@ -1716,7 +1712,6 @@ nir_ssa_def_init(nir_instr *instr, nir_ssa_def *def, { def->parent_instr = instr; list_inithead(&def->uses); - list_inithead(&def->if_uses); def->num_components = num_components; def->bit_size = bit_size; def->divergent = true; /* This is the safer default */ @@ -1747,11 +1742,12 @@ void nir_ssa_def_rewrite_uses(nir_ssa_def *def, nir_ssa_def *new_ssa) { assert(def != new_ssa); - nir_foreach_use_safe(use_src, def) - nir_instr_rewrite_src_ssa(use_src->parent_instr, use_src, new_ssa); - - nir_foreach_if_use_safe(use_src, def) - nir_if_rewrite_condition_ssa(use_src->parent_if, use_src, new_ssa); + nir_foreach_use_including_if_safe(use_src, def) { + if (use_src->is_if) + nir_if_rewrite_condition_ssa(use_src->parent_if, use_src, new_ssa); + else + nir_instr_rewrite_src_ssa(use_src->parent_instr, use_src, new_ssa); + } } void @@ -1760,11 +1756,12 @@ nir_ssa_def_rewrite_uses_src(nir_ssa_def *def, nir_src new_src) if (new_src.is_ssa) { nir_ssa_def_rewrite_uses(def, new_src.ssa); } else { - nir_foreach_use_safe(use_src, def) - nir_instr_rewrite_src(use_src->parent_instr, use_src, new_src); - - nir_foreach_if_use_safe(use_src, def) - nir_if_rewrite_condition(use_src->parent_if, new_src); + nir_foreach_use_including_if_safe(use_src, def) { + if (use_src->is_if) + nir_if_rewrite_condition(use_src->parent_if, new_src); + else + nir_instr_rewrite_src(use_src->parent_instr, use_src, new_src); + } } } @@ -1805,20 +1802,20 @@ nir_ssa_def_rewrite_uses_after(nir_ssa_def *def, nir_ssa_def *new_ssa, if (def == new_ssa) return; - nir_foreach_use_safe(use_src, def) { - assert(use_src->parent_instr != def->parent_instr); - /* Since def already dominates all of its uses, the only way a use can - * not be dominated by after_me is if it is between def and after_me in - * the instruction list. - */ - if (!is_instr_between(def->parent_instr, after_me, use_src->parent_instr)) - nir_instr_rewrite_src_ssa(use_src->parent_instr, use_src, new_ssa); - } - - nir_foreach_if_use_safe(use_src, def) { - nir_if_rewrite_condition_ssa(use_src->parent_if, - &use_src->parent_if->condition, - new_ssa); + nir_foreach_use_including_if_safe(use_src, def) { + if (use_src->is_if) { + nir_if_rewrite_condition_ssa(use_src->parent_if, + &use_src->parent_if->condition, + new_ssa); + } else { + assert(use_src->parent_instr != def->parent_instr); + /* Since def already dominates all of its uses, the only way a use can + * not be dominated by after_me is if it is between def and after_me in + * the instruction list. + */ + if (!is_instr_between(def->parent_instr, after_me, use_src->parent_instr)) + nir_instr_rewrite_src_ssa(use_src->parent_instr, use_src, new_ssa); + } } } @@ -1862,11 +1859,9 @@ nir_ssa_def_components_read(const nir_ssa_def *def) { nir_component_mask_t read_mask = 0; - if (!list_is_empty(&def->if_uses)) - read_mask |= 1; + nir_foreach_use_including_if(use, def) { + read_mask |= use->is_if ? 1 : nir_src_components_read(use); - nir_foreach_use(use, def) { - read_mask |= nir_src_components_read(use); if (read_mask == (1 << def->num_components) - 1) return read_mask; } @@ -2267,7 +2262,7 @@ nir_function_impl_lower_instructions(nir_function_impl *impl, assert(nir_foreach_dest(instr, dest_is_ssa, NULL)); nir_ssa_def *old_def = nir_instr_ssa_def(instr); - struct list_head old_uses, old_if_uses; + struct list_head old_uses; if (old_def != NULL) { /* We're about to ask the callback to generate a replacement for instr. * Save off the uses from instr's SSA def so we know what uses to @@ -2283,8 +2278,6 @@ nir_function_impl_lower_instructions(nir_function_impl *impl, list_replace(&old_def->uses, &old_uses); list_inithead(&old_def->uses); - list_replace(&old_def->if_uses, &old_if_uses); - list_inithead(&old_def->if_uses); } b.cursor = nir_after_instr(instr); @@ -2296,11 +2289,12 @@ nir_function_impl_lower_instructions(nir_function_impl *impl, preserved = nir_metadata_none; nir_src new_src = nir_src_for_ssa(new_def); - list_for_each_entry_safe(nir_src, use_src, &old_uses, use_link) - nir_instr_rewrite_src(use_src->parent_instr, use_src, new_src); - - list_for_each_entry_safe(nir_src, use_src, &old_if_uses, use_link) - nir_if_rewrite_condition(use_src->parent_if, new_src); + list_for_each_entry_safe(nir_src, use_src, &old_uses, use_link) { + if (use_src->is_if) + nir_if_rewrite_condition(use_src->parent_if, new_src); + else + nir_instr_rewrite_src(use_src->parent_instr, use_src, new_src); + } if (nir_ssa_def_is_unused(old_def)) { iter = nir_instr_free_and_dce(instr); @@ -2310,10 +2304,9 @@ nir_function_impl_lower_instructions(nir_function_impl *impl, progress = true; } else { /* We didn't end up lowering after all. Put the uses back */ - if (old_def) { + if (old_def) list_replace(&old_uses, &old_def->uses); - list_replace(&old_if_uses, &old_def->if_uses); - } + if (new_def == NIR_LOWER_INSTR_PROGRESS_REPLACE) { /* Only instructions without a return value can be removed like this */ assert(!old_def); diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index d0c01756e57..62c72276451 100644 --- a/src/compiler/nir/nir.h +++ b/src/compiler/nir/nir.h @@ -876,9 +876,6 @@ typedef struct nir_register { /** set of nir_dests where this register is defined (written to) */ struct list_head defs; - - /** set of nir_ifs where this register is used as a condition */ - struct list_head if_uses; } nir_register; #define nir_foreach_register(reg, reg_list) \ @@ -952,9 +949,6 @@ typedef struct nir_ssa_def { /** set of nir_instrs where this register is used (read from) */ struct list_head uses; - /** set of nir_ifs where this register is used as a condition */ - struct list_head if_uses; - /** generic SSA definition index. */ unsigned index; @@ -1008,8 +1002,23 @@ typedef struct nir_src { }; bool is_ssa; + bool is_if; } nir_src; +static inline void +nir_src_set_parent_instr(nir_src *src, nir_instr *parent_instr) +{ + src->is_if = false; + src->parent_instr = parent_instr; +} + +static inline void +nir_src_set_parent_if(nir_src *src, struct nir_if *parent_if) +{ + src->is_if = true; + src->parent_if = parent_if; +} + static inline nir_src nir_src_init(void) { @@ -1019,17 +1028,36 @@ nir_src_init(void) #define NIR_SRC_INIT nir_src_init() -#define nir_foreach_use(src, reg_or_ssa_def) \ +#define nir_foreach_use_including_if(src, reg_or_ssa_def) \ list_for_each_entry(nir_src, src, &(reg_or_ssa_def)->uses, use_link) -#define nir_foreach_use_safe(src, reg_or_ssa_def) \ +#define nir_foreach_use_including_if_safe(src, reg_or_ssa_def) \ list_for_each_entry_safe(nir_src, src, &(reg_or_ssa_def)->uses, use_link) +#define nir_foreach_use(src, reg_or_ssa_def) \ + nir_foreach_use_including_if(src, reg_or_ssa_def) \ + if (!src->is_if) + +#define nir_foreach_use_safe(src, reg_or_ssa_def) \ + nir_foreach_use_including_if_safe(src, reg_or_ssa_def) \ + if (!src->is_if) + #define nir_foreach_if_use(src, reg_or_ssa_def) \ - list_for_each_entry(nir_src, src, &(reg_or_ssa_def)->if_uses, use_link) + nir_foreach_use_including_if(src, reg_or_ssa_def) \ + if (src->is_if) #define nir_foreach_if_use_safe(src, reg_or_ssa_def) \ - list_for_each_entry_safe(nir_src, src, &(reg_or_ssa_def)->if_uses, use_link) + nir_foreach_use_including_if_safe(src, reg_or_ssa_def) \ + if (src->is_if) + +static inline bool +nir_ssa_def_used_by_if(const nir_ssa_def *def) +{ + nir_foreach_if_use(_, def) + return true; + + return false; +} typedef struct { union { @@ -4317,6 +4345,7 @@ static inline void nir_instr_rewrite_src_ssa(ASSERTED nir_instr *instr, nir_src *src, nir_ssa_def *new_ssa) { + assert(!src->is_if); assert(src->parent_instr == instr); assert(src->is_ssa && src->ssa); list_del(&src->use_link); @@ -4331,11 +4360,12 @@ static inline void nir_if_rewrite_condition_ssa(ASSERTED nir_if *if_stmt, nir_src *src, nir_ssa_def *new_ssa) { + assert(src->is_if); assert(src->parent_if == if_stmt); assert(src->is_ssa && src->ssa); list_del(&src->use_link); src->ssa = new_ssa; - list_addtail(&src->use_link, &new_ssa->if_uses); + list_addtail(&src->use_link, &new_ssa->uses); } void nir_if_rewrite_condition(nir_if *if_stmt, nir_src new_src); @@ -4367,7 +4397,7 @@ nir_component_mask_t nir_ssa_def_components_read(const nir_ssa_def *def); static inline bool nir_ssa_def_is_unused(nir_ssa_def *ssa) { - return list_is_empty(&ssa->uses) && list_is_empty(&ssa->if_uses); + return list_is_empty(&ssa->uses); } diff --git a/src/compiler/nir/nir_clone.c b/src/compiler/nir/nir_clone.c index 52682a1a185..a18f27feea2 100644 --- a/src/compiler/nir/nir_clone.c +++ b/src/compiler/nir/nir_clone.c @@ -214,10 +214,9 @@ clone_register(clone_state *state, const nir_register *reg) nreg->num_array_elems = reg->num_array_elems; nreg->index = reg->index; - /* reconstructing uses/defs/if_uses handled by nir_instr_insert() */ + /* reconstructing uses/defs handled by nir_instr_insert() */ list_inithead(&nreg->uses); list_inithead(&nreg->defs); - list_inithead(&nreg->if_uses); return nreg; } diff --git a/src/compiler/nir/nir_control_flow.c b/src/compiler/nir/nir_control_flow.c index 8a93435ef30..79f7ea7ea7d 100644 --- a/src/compiler/nir/nir_control_flow.c +++ b/src/compiler/nir/nir_control_flow.c @@ -577,14 +577,14 @@ update_if_uses(nir_cf_node *node) return; nir_if *if_stmt = nir_cf_node_as_if(node); + nir_src_set_parent_if(&if_stmt->condition, if_stmt); - if_stmt->condition.parent_if = if_stmt; if (if_stmt->condition.is_ssa) { list_addtail(&if_stmt->condition.use_link, - &if_stmt->condition.ssa->if_uses); + &if_stmt->condition.ssa->uses); } else { list_addtail(&if_stmt->condition.use_link, - &if_stmt->condition.reg.reg->if_uses); + &if_stmt->condition.reg.reg->uses); } } diff --git a/src/compiler/nir/nir_deref.c b/src/compiler/nir/nir_deref.c index 9687eba60e6..35c594b5599 100644 --- a/src/compiler/nir/nir_deref.c +++ b/src/compiler/nir/nir_deref.c @@ -157,7 +157,10 @@ bool nir_deref_instr_has_complex_use(nir_deref_instr *deref, nir_deref_instr_has_complex_use_options opts) { - nir_foreach_use(use_src, &deref->dest.ssa) { + nir_foreach_use_including_if(use_src, &deref->dest.ssa) { + if (use_src->is_if) + return true; + nir_instr *use_instr = use_src->parent_instr; switch (use_instr->type) { @@ -235,9 +238,6 @@ nir_deref_instr_has_complex_use(nir_deref_instr *deref, } } - nir_foreach_if_use(use, &deref->dest.ssa) - return true; - return false; } @@ -1187,7 +1187,9 @@ opt_deref_cast(nir_builder *b, nir_deref_instr *cast) assert(cast->dest.is_ssa); assert(cast->parent.is_ssa); - nir_foreach_use_safe(use_src, &cast->dest.ssa) { + nir_foreach_use_including_if_safe(use_src, &cast->dest.ssa) { + assert(!use_src->is_if && "there cannot be if-uses"); + /* If this isn't a trivial array cast, we can't propagate into * ptr_as_array derefs. */ @@ -1199,9 +1201,6 @@ opt_deref_cast(nir_builder *b, nir_deref_instr *cast) progress = true; } - /* If uses would be a bit crazy */ - assert(list_is_empty(&cast->dest.ssa.if_uses)); - if (nir_deref_instr_remove_if_unused(cast)) progress = true; diff --git a/src/compiler/nir/nir_from_ssa.c b/src/compiler/nir/nir_from_ssa.c index 766c88a1953..1a29c0e913a 100644 --- a/src/compiler/nir/nir_from_ssa.c +++ b/src/compiler/nir/nir_from_ssa.c @@ -1080,16 +1080,14 @@ static bool ssa_def_is_local_to_block(nir_ssa_def *def, UNUSED void *state) { nir_block *block = def->parent_instr->block; - nir_foreach_use(use_src, def) { - if (use_src->parent_instr->block != block || + nir_foreach_use_including_if(use_src, def) { + if (use_src->is_if || + use_src->parent_instr->block != block || use_src->parent_instr->type == nir_instr_type_phi) { return false; } } - if (!list_is_empty(&def->if_uses)) - return false; - return true; } diff --git a/src/compiler/nir/nir_loop_analyze.c b/src/compiler/nir/nir_loop_analyze.c index 861e7b076f2..0e92a390f84 100644 --- a/src/compiler/nir/nir_loop_analyze.c +++ b/src/compiler/nir/nir_loop_analyze.c @@ -178,8 +178,8 @@ instr_cost(loop_info_state *state, nir_instr *instr, /* Also if the selects condition is only used by the select then * remove that alu instructons cost from the cost total also. */ - if (!list_is_empty(&sel_alu->dest.dest.ssa.if_uses) || - !list_is_singular(&sel_alu->dest.dest.ssa.uses)) + if (!list_is_singular(&sel_alu->dest.dest.ssa.uses) || + nir_ssa_def_used_by_if(&sel_alu->dest.dest.ssa)) return 0; else return -1; diff --git a/src/compiler/nir/nir_lower_io.c b/src/compiler/nir/nir_lower_io.c index a04485a7b2c..fd405ae948c 100644 --- a/src/compiler/nir/nir_lower_io.c +++ b/src/compiler/nir/nir_lower_io.c @@ -2124,8 +2124,7 @@ lower_explicit_io_deref(nir_builder *b, nir_deref_instr *deref, * one deref which could break our list walking since we walk the list * backwards. */ - assert(list_is_empty(&deref->dest.ssa.if_uses)); - if (list_is_empty(&deref->dest.ssa.uses)) { + if (nir_ssa_def_is_unused(&deref->dest.ssa)) { nir_instr_remove(&deref->instr); return; } diff --git a/src/compiler/nir/nir_lower_regs_to_ssa.c b/src/compiler/nir/nir_lower_regs_to_ssa.c index 6fac3d4cbbe..46fc263e42b 100644 --- a/src/compiler/nir/nir_lower_regs_to_ssa.c +++ b/src/compiler/nir/nir_lower_regs_to_ssa.c @@ -284,7 +284,6 @@ nir_lower_regs_to_ssa_impl(nir_function_impl *impl) nir_foreach_register_safe(reg, &impl->registers) { if (state.values[reg->index]) { assert(list_is_empty(®->uses)); - assert(list_is_empty(®->if_uses)); assert(list_is_empty(®->defs)); exec_node_remove(®->node); } diff --git a/src/compiler/nir/nir_lower_to_source_mods.c b/src/compiler/nir/nir_lower_to_source_mods.c index c5c316aedfa..55d7b509c5d 100644 --- a/src/compiler/nir/nir_lower_to_source_mods.c +++ b/src/compiler/nir/nir_lower_to_source_mods.c @@ -152,11 +152,13 @@ nir_lower_to_source_mods_block(nir_block *block, if (!(options & nir_lower_float_source_mods)) continue; - if (!list_is_empty(&alu->dest.dest.ssa.if_uses)) - continue; - bool all_children_are_sat = true; - nir_foreach_use(child_src, &alu->dest.dest.ssa) { + nir_foreach_use_including_if(child_src, &alu->dest.dest.ssa) { + if (child_src->is_if) { + all_children_are_sat = false; + break; + } + assert(child_src->is_ssa); nir_instr *child = child_src->parent_instr; if (child->type != nir_instr_type_alu) { diff --git a/src/compiler/nir/nir_lower_vec_to_movs.c b/src/compiler/nir/nir_lower_vec_to_movs.c index e932ca63b01..94bf2966446 100644 --- a/src/compiler/nir/nir_lower_vec_to_movs.c +++ b/src/compiler/nir/nir_lower_vec_to_movs.c @@ -139,7 +139,10 @@ try_coalesce(nir_alu_instr *vec, unsigned start_idx, void *_data) /* If we are going to do a reswizzle, then the vecN operation must be the * only use of the source value. We also can't have any source modifiers. */ - nir_foreach_use(src, vec->src[start_idx].src.ssa) { + nir_foreach_use_including_if(src, vec->src[start_idx].src.ssa) { + if (src->is_if) + return 0; + if (src->parent_instr != &vec->instr) return 0; @@ -148,9 +151,6 @@ try_coalesce(nir_alu_instr *vec, unsigned start_idx, void *_data) return 0; } - if (!list_is_empty(&vec->src[start_idx].src.ssa->if_uses)) - return 0; - if (vec->src[start_idx].src.ssa->parent_instr->type != nir_instr_type_alu) return 0; diff --git a/src/compiler/nir/nir_opt_copy_propagate.c b/src/compiler/nir/nir_opt_copy_propagate.c index 8b88779956a..70c0733df0f 100644 --- a/src/compiler/nir/nir_opt_copy_propagate.c +++ b/src/compiler/nir/nir_opt_copy_propagate.c @@ -146,16 +146,15 @@ copy_prop_instr(nir_function_impl *impl, nir_instr *instr) bool progress = false; - nir_foreach_use_safe(src, &mov->dest.dest.ssa) { - if (src->parent_instr->type == nir_instr_type_alu) + nir_foreach_use_including_if_safe(src, &mov->dest.dest.ssa) { + if (src->is_if) + progress |= copy_propagate_if(src, mov); + else if (src->parent_instr->type == nir_instr_type_alu) progress |= copy_propagate_alu(impl, container_of(src, nir_alu_src, src), mov); else progress |= copy_propagate(src, mov); } - nir_foreach_if_use_safe(src, &mov->dest.dest.ssa) - progress |= copy_propagate_if(src, mov); - if (progress && nir_ssa_def_is_unused(&mov->dest.dest.ssa)) nir_instr_remove(&mov->instr); diff --git a/src/compiler/nir/nir_opt_dead_cf.c b/src/compiler/nir/nir_opt_dead_cf.c index 5354a7f9228..fbb935842fe 100644 --- a/src/compiler/nir/nir_opt_dead_cf.c +++ b/src/compiler/nir/nir_opt_dead_cf.c @@ -141,7 +141,14 @@ def_only_used_in_cf_node(nir_ssa_def *def, void *_node) nir_block *before = nir_cf_node_as_block(nir_cf_node_prev(node)); nir_block *after = nir_cf_node_as_block(nir_cf_node_next(node)); - nir_foreach_use(use, def) { + nir_foreach_use_including_if(use, def) { + nir_block *block; + + if (use->is_if) + block = nir_cf_node_as_block(nir_cf_node_prev(&use->parent_if->cf_node)); + else + block = use->parent_instr->block; + /* Because NIR is structured, we can easily determine whether or not a * value escapes a CF node by looking at the block indices of its uses * to see if they lie outside the bounds of the CF node. @@ -155,18 +162,7 @@ def_only_used_in_cf_node(nir_ssa_def *def, void *_node) * corresponding predecessor is inside the loop or not because the value * can go through the phi into the outside world and escape the loop. */ - if (use->parent_instr->block->index <= before->index || - use->parent_instr->block->index >= after->index) - return false; - } - - /* Same check for if-condition uses */ - nir_foreach_if_use(use, def) { - nir_block *use_block = - nir_cf_node_as_block(nir_cf_node_prev(&use->parent_if->cf_node)); - - if (use_block->index <= before->index || - use_block->index >= after->index) + if (block->index <= before->index || block->index >= after->index) return false; } diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c index 3711c15959e..4a54ae4569a 100644 --- a/src/compiler/nir/nir_opt_if.c +++ b/src/compiler/nir/nir_opt_if.c @@ -492,12 +492,11 @@ opt_split_alu_of_phi(nir_builder *b, nir_loop *loop) if (!is_prev_result_undef && !is_prev_result_const) { /* check if the only user is a trivial bcsel */ - if (!list_is_empty(&alu->dest.dest.ssa.if_uses) || - !list_is_singular(&alu->dest.dest.ssa.uses)) + if (!list_is_singular(&alu->dest.dest.ssa.uses)) continue; nir_src *use = list_first_entry(&alu->dest.dest.ssa.uses, nir_src, use_link); - if (!is_trivial_bcsel(use->parent_instr, true)) + if (use->is_if || !is_trivial_bcsel(use->parent_instr, true)) continue; } @@ -1286,11 +1285,10 @@ clone_alu_and_replace_src_defs(nir_builder *b, const nir_alu_instr *alu, */ static bool propagate_condition_eval(nir_builder *b, nir_if *nif, nir_src *use_src, - nir_src *alu_use, nir_alu_instr *alu, - bool is_if_condition) + nir_src *alu_use, nir_alu_instr *alu) { bool bool_value; - b->cursor = nir_before_src(alu_use, is_if_condition); + b->cursor = nir_before_src(alu_use, alu_use->is_if); if (!evaluate_if_condition(nif, b->cursor, &bool_value)) return false; @@ -1308,7 +1306,7 @@ propagate_condition_eval(nir_builder *b, nir_if *nif, nir_src *use_src, /* Rewrite use to use new alu instruction */ nir_src new_src = nir_src_for_ssa(nalu); - if (is_if_condition) + if (alu_use->is_if) nir_if_rewrite_condition(alu_use->parent_if, new_src); else nir_instr_rewrite_src(alu_use->parent_instr, alu_use, new_src); @@ -1337,18 +1335,17 @@ can_propagate_through_alu(nir_src *src) } static bool -evaluate_condition_use(nir_builder *b, nir_if *nif, nir_src *use_src, - bool is_if_condition) +evaluate_condition_use(nir_builder *b, nir_if *nif, nir_src *use_src) { bool progress = false; - b->cursor = nir_before_src(use_src, is_if_condition); + b->cursor = nir_before_src(use_src, use_src->is_if); bool bool_value; if (evaluate_if_condition(nif, b->cursor, &bool_value)) { /* Rewrite use to use const */ nir_src imm_src = nir_src_for_ssa(nir_imm_bool(b, bool_value)); - if (is_if_condition) + if (use_src->is_if) nir_if_rewrite_condition(use_src->parent_if, imm_src); else nir_instr_rewrite_src(use_src->parent_instr, use_src, imm_src); @@ -1356,18 +1353,11 @@ evaluate_condition_use(nir_builder *b, nir_if *nif, nir_src *use_src, progress = true; } - if (!is_if_condition && can_propagate_through_alu(use_src)) { + if (!use_src->is_if && can_propagate_through_alu(use_src)) { nir_alu_instr *alu = nir_instr_as_alu(use_src->parent_instr); - nir_foreach_use_safe(alu_use, &alu->dest.dest.ssa) { - progress |= propagate_condition_eval(b, nif, use_src, alu_use, alu, - false); - } - - nir_foreach_if_use_safe(alu_use, &alu->dest.dest.ssa) { - progress |= propagate_condition_eval(b, nif, use_src, alu_use, alu, - true); - } + nir_foreach_use_including_if_safe(alu_use, &alu->dest.dest.ssa) + progress |= propagate_condition_eval(b, nif, use_src, alu_use, alu); } return progress; @@ -1380,13 +1370,9 @@ opt_if_evaluate_condition_use(nir_builder *b, nir_if *nif) /* Evaluate any uses of the if condition inside the if branches */ assert(nif->condition.is_ssa); - nir_foreach_use_safe(use_src, nif->condition.ssa) { - progress |= evaluate_condition_use(b, nif, use_src, false); - } - - nir_foreach_if_use_safe(use_src, nif->condition.ssa) { - if (use_src->parent_if != nif) - progress |= evaluate_condition_use(b, nif, use_src, true); + nir_foreach_use_including_if_safe(use_src, nif->condition.ssa) { + if (!(use_src->is_if && use_src->parent_if == nif)) + progress |= evaluate_condition_use(b, nif, use_src); } return progress; diff --git a/src/compiler/nir/nir_opt_intrinsics.c b/src/compiler/nir/nir_opt_intrinsics.c index 11c26b54828..0c5b33991ac 100644 --- a/src/compiler/nir/nir_opt_intrinsics.c +++ b/src/compiler/nir/nir_opt_intrinsics.c @@ -39,8 +39,10 @@ src_is_single_use_shuffle(nir_src src, nir_ssa_def **data, nir_ssa_def **index) * uses is reasonable. If we ever want to use this from an if statement, * we can change it then. */ - if (!list_is_empty(&shuffle->dest.ssa.if_uses) || - !list_is_singular(&shuffle->dest.ssa.uses)) + if (!list_is_singular(&shuffle->dest.ssa.uses)) + return false; + + if (nir_ssa_def_used_by_if(&shuffle->dest.ssa)) return false; assert(shuffle->src[0].is_ssa); diff --git a/src/compiler/nir/nir_opt_peephole_select.c b/src/compiler/nir/nir_opt_peephole_select.c index 8593f9cfdf0..7a1fde07289 100644 --- a/src/compiler/nir/nir_opt_peephole_select.c +++ b/src/compiler/nir/nir_opt_peephole_select.c @@ -221,13 +221,10 @@ block_check_for_allowed_instrs(nir_block *block, unsigned *count, if (mov->dest.saturate) return false; - /* It cannot have any if-uses */ - if (!list_is_empty(&mov->dest.dest.ssa.if_uses)) - return false; - /* The only uses of this definition must be phis in the successor */ - nir_foreach_use(use, &mov->dest.dest.ssa) { - if (use->parent_instr->type != nir_instr_type_phi || + nir_foreach_use_including_if(use, &mov->dest.dest.ssa) { + if (use->is_if || + use->parent_instr->type != nir_instr_type_phi || use->parent_instr->block != block->successors[0]) return false; } diff --git a/src/compiler/nir/nir_opt_phi_precision.c b/src/compiler/nir/nir_opt_phi_precision.c index caef5c465ff..03b8ef1dbe3 100644 --- a/src/compiler/nir/nir_opt_phi_precision.c +++ b/src/compiler/nir/nir_opt_phi_precision.c @@ -205,7 +205,13 @@ try_move_narrowing_dst(nir_builder *b, nir_phi_instr *phi) /* Are the only uses of the phi conversion instructions, and * are they all the same conversion? */ - nir_foreach_use (use, &phi->dest.ssa) { + nir_foreach_use_including_if (use, &phi->dest.ssa) { + /* an if use means the phi is used directly in a conditional, ie. + * without a conversion + */ + if (use->is_if) + return false; + op = narrowing_conversion_op(use->parent_instr, op); /* Not a (compatible) narrowing conversion: */ @@ -213,12 +219,6 @@ try_move_narrowing_dst(nir_builder *b, nir_phi_instr *phi) return false; } - /* an if_uses means the phi is used directly in a conditional, ie. - * without a conversion - */ - if (!list_is_empty(&phi->dest.ssa.if_uses)) - return false; - /* If the phi has no uses, then nothing to do: */ if (op == INVALID_OP) return false; diff --git a/src/compiler/nir/nir_opt_ray_queries.c b/src/compiler/nir/nir_opt_ray_queries.c index a95e5173687..c55ade9ac4a 100644 --- a/src/compiler/nir/nir_opt_ray_queries.c +++ b/src/compiler/nir/nir_opt_ray_queries.c @@ -77,8 +77,7 @@ nir_find_ray_queries_read(struct set *queries, nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr); switch (intrin->intrinsic) { case nir_intrinsic_rq_proceed: - if (list_length(&intrin->dest.ssa.uses) > 0 || - list_length(&intrin->dest.ssa.if_uses) > 0) + if (list_length(&intrin->dest.ssa.uses) > 0) mark_query_read(queries, intrin); break; case nir_intrinsic_rq_load: @@ -120,10 +119,8 @@ nir_replace_unread_queries_instr(nir_builder *b, nir_instr *instr, void *data) if (entry) return false; - if (intrin->intrinsic == nir_intrinsic_rq_load) { + if (intrin->intrinsic == nir_intrinsic_rq_load) assert(list_is_empty(&intrin->dest.ssa.uses)); - assert(list_is_empty(&intrin->dest.ssa.if_uses)); - } nir_instr_remove(instr); diff --git a/src/compiler/nir/nir_opt_rematerialize_compares.c b/src/compiler/nir/nir_opt_rematerialize_compares.c index b87dad18570..76a43968520 100644 --- a/src/compiler/nir/nir_opt_rematerialize_compares.c +++ b/src/compiler/nir/nir_opt_rematerialize_compares.c @@ -124,50 +124,50 @@ nir_opt_rematerialize_compares_impl(nir_shader *shader, nir_function_impl *impl) * instruction must be duplicated only once in each block because CSE * cannot be run after this pass. */ - nir_foreach_use_safe(use, &alu->dest.dest.ssa) { - nir_instr *const use_instr = use->parent_instr; + nir_foreach_use_including_if_safe(use, &alu->dest.dest.ssa) { + if (use->is_if) { + nir_if *const if_stmt = use->parent_if; - /* If the use is in the same block as the def, don't - * rematerialize. - */ - if (use_instr->block == alu->instr.block) - continue; + nir_block *const prev_block = + nir_cf_node_as_block(nir_cf_node_prev(&if_stmt->cf_node)); - nir_alu_instr *clone = nir_alu_instr_clone(shader, alu); + /* If the compare is from the previous block, don't + * rematerialize. + */ + if (prev_block == alu->instr.block) + continue; - nir_instr_insert_before(use_instr, &clone->instr); + nir_alu_instr *clone = nir_alu_instr_clone(shader, alu); - nir_alu_instr *const use_alu = nir_instr_as_alu(use_instr); - for (unsigned i = 0; i < nir_op_infos[use_alu->op].num_inputs; i++) { - if (use_alu->src[i].src.ssa == &alu->dest.dest.ssa) { - nir_instr_rewrite_src(&use_alu->instr, - &use_alu->src[i].src, + nir_instr_insert_after_block(prev_block, &clone->instr); + + nir_if_rewrite_condition(if_stmt, nir_src_for_ssa(&clone->dest.dest.ssa)); - progress = true; + progress = true; + } else { + nir_instr *const use_instr = use->parent_instr; + + /* If the use is in the same block as the def, don't + * rematerialize. + */ + if (use_instr->block == alu->instr.block) + continue; + + nir_alu_instr *clone = nir_alu_instr_clone(shader, alu); + + nir_instr_insert_before(use_instr, &clone->instr); + + nir_alu_instr *const use_alu = nir_instr_as_alu(use_instr); + for (unsigned i = 0; i < nir_op_infos[use_alu->op].num_inputs; i++) { + if (use_alu->src[i].src.ssa == &alu->dest.dest.ssa) { + nir_instr_rewrite_src(&use_alu->instr, + &use_alu->src[i].src, + nir_src_for_ssa(&clone->dest.dest.ssa)); + progress = true; + } } } } - - nir_foreach_if_use_safe(use, &alu->dest.dest.ssa) { - nir_if *const if_stmt = use->parent_if; - - nir_block *const prev_block = - nir_cf_node_as_block(nir_cf_node_prev(&if_stmt->cf_node)); - - /* If the compare is from the previous block, don't - * rematerialize. - */ - if (prev_block == alu->instr.block) - continue; - - nir_alu_instr *clone = nir_alu_instr_clone(shader, alu); - - nir_instr_insert_after_block(prev_block, &clone->instr); - - nir_if_rewrite_condition(if_stmt, - nir_src_for_ssa(&clone->dest.dest.ssa)); - progress = true; - } } } diff --git a/src/compiler/nir/nir_opt_sink.c b/src/compiler/nir/nir_opt_sink.c index a1423f7b274..40eaae4a104 100644 --- a/src/compiler/nir/nir_opt_sink.c +++ b/src/compiler/nir/nir_opt_sink.c @@ -141,37 +141,37 @@ get_preferred_block(nir_ssa_def *def, bool sink_out_of_loops) { nir_block *lca = NULL; - nir_foreach_use(use, def) { - nir_instr *instr = use->parent_instr; - nir_block *use_block = instr->block; + nir_foreach_use_including_if(use, def) { + nir_block *use_block; - /* - * Kind of an ugly special-case, but phi instructions - * need to appear first in the block, so by definition - * we can't move an instruction into a block where it is - * consumed by a phi instruction. We could conceivably - * move it into a dominator block. - */ - if (instr->type == nir_instr_type_phi) { - nir_phi_instr *phi = nir_instr_as_phi(instr); - nir_block *phi_lca = NULL; - nir_foreach_phi_src(src, phi) { - if (&src->src == use) - phi_lca = nir_dominance_lca(phi_lca, src->pred); + if (use->is_if) { + use_block = + nir_cf_node_as_block(nir_cf_node_prev(&use->parent_if->cf_node)); + } else { + nir_instr *instr = use->parent_instr; + use_block = instr->block; + + /* + * Kind of an ugly special-case, but phi instructions + * need to appear first in the block, so by definition + * we can't move an instruction into a block where it is + * consumed by a phi instruction. We could conceivably + * move it into a dominator block. + */ + if (instr->type == nir_instr_type_phi) { + nir_phi_instr *phi = nir_instr_as_phi(instr); + nir_block *phi_lca = NULL; + nir_foreach_phi_src(src, phi) { + if (&src->src == use) + phi_lca = nir_dominance_lca(phi_lca, src->pred); + } + use_block = phi_lca; } - use_block = phi_lca; } lca = nir_dominance_lca(lca, use_block); } - nir_foreach_if_use(use, def) { - nir_block *use_block = - nir_cf_node_as_block(nir_cf_node_prev(&use->parent_if->cf_node)); - - lca = nir_dominance_lca(lca, use_block); - } - /* return in case, we didn't find a reachable user */ if (!lca) return NULL; diff --git a/src/compiler/nir/nir_opt_uniform_atomics.c b/src/compiler/nir/nir_opt_uniform_atomics.c index ab39ea2dc44..e5a5791d968 100644 --- a/src/compiler/nir/nir_opt_uniform_atomics.c +++ b/src/compiler/nir/nir_opt_uniform_atomics.c @@ -262,7 +262,6 @@ optimize_and_rewrite_atomic(nir_builder *b, nir_intrinsic_instr *intrin) nir_ssa_def old_result = intrin->dest.ssa; list_replace(&intrin->dest.ssa.uses, &old_result.uses); - list_replace(&intrin->dest.ssa.if_uses, &old_result.if_uses); nir_ssa_dest_init(&intrin->instr, &intrin->dest, 1, intrin->dest.ssa.bit_size, NULL); nir_ssa_def *result = optimize_atomic(b, intrin, return_prev); diff --git a/src/compiler/nir/nir_repair_ssa.c b/src/compiler/nir/nir_repair_ssa.c index 2c9073ffc1e..3aae5fa9366 100644 --- a/src/compiler/nir/nir_repair_ssa.c +++ b/src/compiler/nir/nir_repair_ssa.c @@ -70,21 +70,21 @@ repair_ssa_def(nir_ssa_def *def, void *void_state) struct repair_ssa_state *state = void_state; bool is_valid = true; - nir_foreach_use(src, def) { - if (nir_block_is_unreachable(get_src_block(src)) || - !nir_block_dominates(def->parent_instr->block, get_src_block(src))) { - is_valid = false; - break; - } - } - - nir_foreach_if_use(src, def) { - nir_block *block_before_if = - nir_cf_node_as_block(nir_cf_node_prev(&src->parent_if->cf_node)); - if (nir_block_is_unreachable(block_before_if) || - !nir_block_dominates(def->parent_instr->block, block_before_if)) { - is_valid = false; - break; + nir_foreach_use_including_if(src, def) { + if (src->is_if) { + nir_block *block_before_if = + nir_cf_node_as_block(nir_cf_node_prev(&src->parent_if->cf_node)); + if (nir_block_is_unreachable(block_before_if) || + !nir_block_dominates(def->parent_instr->block, block_before_if)) { + is_valid = false; + break; + } + } else { + if (nir_block_is_unreachable(get_src_block(src)) || + !nir_block_dominates(def->parent_instr->block, get_src_block(src))) { + is_valid = false; + break; + } } } @@ -101,7 +101,24 @@ repair_ssa_def(nir_ssa_def *def, void *void_state) nir_phi_builder_value_set_block_def(val, def->parent_instr->block, def); - nir_foreach_use_safe(src, def) { + nir_foreach_use_including_if_safe(src, def) { + if (src->is_if) { + nir_block *block_before_if = + nir_cf_node_as_block(nir_cf_node_prev(&src->parent_if->cf_node)); + if (block_before_if == def->parent_instr->block) { + assert(nir_phi_builder_value_get_block_def(val, block_before_if) == def); + continue; + } + + nir_ssa_def *block_def = + nir_phi_builder_value_get_block_def(val, block_before_if); + if (block_def == def) + continue; + + nir_if_rewrite_condition(src->parent_if, nir_src_for_ssa(block_def)); + continue; + } + nir_block *src_block = get_src_block(src); if (src_block == def->parent_instr->block) { assert(nir_phi_builder_value_get_block_def(val, src_block) == def); @@ -140,22 +157,6 @@ repair_ssa_def(nir_ssa_def *def, void *void_state) nir_instr_rewrite_src(src->parent_instr, src, nir_src_for_ssa(block_def)); } - nir_foreach_if_use_safe(src, def) { - nir_block *block_before_if = - nir_cf_node_as_block(nir_cf_node_prev(&src->parent_if->cf_node)); - if (block_before_if == def->parent_instr->block) { - assert(nir_phi_builder_value_get_block_def(val, block_before_if) == def); - continue; - } - - nir_ssa_def *block_def = - nir_phi_builder_value_get_block_def(val, block_before_if); - if (block_def == def) - continue; - - nir_if_rewrite_condition(src->parent_if, nir_src_for_ssa(block_def)); - } - return true; } diff --git a/src/compiler/nir/nir_search_helpers.h b/src/compiler/nir/nir_search_helpers.h index b0288511210..0ba0965e8fb 100644 --- a/src/compiler/nir/nir_search_helpers.h +++ b/src/compiler/nir/nir_search_helpers.h @@ -352,35 +352,19 @@ is_not_const_and_not_fsign(struct hash_table *ht, const nir_alu_instr *instr, static inline bool is_used_once(const nir_alu_instr *instr) { - bool zero_if_use = list_is_empty(&instr->dest.dest.ssa.if_uses); - bool zero_use = list_is_empty(&instr->dest.dest.ssa.uses); - - if (zero_if_use && zero_use) - return false; - - if (!zero_if_use && list_is_singular(&instr->dest.dest.ssa.uses)) - return false; - - if (!zero_use && list_is_singular(&instr->dest.dest.ssa.if_uses)) - return false; - - if (!list_is_singular(&instr->dest.dest.ssa.if_uses) && - !list_is_singular(&instr->dest.dest.ssa.uses)) - return false; - - return true; + return list_is_singular(&instr->dest.dest.ssa.uses); } static inline bool is_used_by_if(const nir_alu_instr *instr) { - return !list_is_empty(&instr->dest.dest.ssa.if_uses); + return nir_ssa_def_used_by_if(&instr->dest.dest.ssa); } static inline bool is_not_used_by_if(const nir_alu_instr *instr) { - return list_is_empty(&instr->dest.dest.ssa.if_uses); + return !is_used_by_if(instr); } static inline bool diff --git a/src/compiler/nir/nir_serialize.c b/src/compiler/nir/nir_serialize.c index ae490e07348..e105b2daf3d 100644 --- a/src/compiler/nir/nir_serialize.c +++ b/src/compiler/nir/nir_serialize.c @@ -464,7 +464,6 @@ read_register(read_ctx *ctx) list_inithead(®->uses); list_inithead(®->defs); - list_inithead(®->if_uses); return reg; } diff --git a/src/compiler/nir/nir_to_lcssa.c b/src/compiler/nir/nir_to_lcssa.c index 9d043a43630..49a43a2006e 100644 --- a/src/compiler/nir/nir_to_lcssa.c +++ b/src/compiler/nir/nir_to_lcssa.c @@ -203,7 +203,14 @@ convert_loop_exit_for_ssa(nir_ssa_def *def, void *void_state) return true; } - nir_foreach_use(use, def) { + nir_foreach_use_including_if(use, def) { + if (use->is_if) { + if (!is_if_use_inside_loop(use, state->loop)) + all_uses_inside_loop = false; + + continue; + } + if (use->parent_instr->type == nir_instr_type_phi && use->parent_instr->block == state->block_after_loop) { continue; @@ -214,12 +221,6 @@ convert_loop_exit_for_ssa(nir_ssa_def *def, void *void_state) } } - nir_foreach_if_use(use, def) { - if (!is_if_use_inside_loop(use, state->loop)) { - all_uses_inside_loop = false; - } - } - /* There where no sources that had defs outside the loop */ if (all_uses_inside_loop) return true; @@ -261,7 +262,14 @@ convert_loop_exit_for_ssa(nir_ssa_def *def, void *void_state) /* Run through all uses and rewrite those outside the loop to point to * the phi instead of pointing to the ssa-def. */ - nir_foreach_use_safe(use, def) { + nir_foreach_use_including_if_safe(use, def) { + if (use->is_if) { + if (!is_if_use_inside_loop(use, state->loop)) + nir_if_rewrite_condition(use->parent_if, nir_src_for_ssa(dest)); + + continue; + } + if (use->parent_instr->type == nir_instr_type_phi && state->block_after_loop == use->parent_instr->block) { continue; @@ -272,12 +280,6 @@ convert_loop_exit_for_ssa(nir_ssa_def *def, void *void_state) } } - nir_foreach_if_use_safe(use, def) { - if (!is_if_use_inside_loop(use, state->loop)) { - nir_if_rewrite_condition(use->parent_if, nir_src_for_ssa(dest)); - } - } - state->progress = true; return true; } diff --git a/src/compiler/nir/nir_validate.c b/src/compiler/nir/nir_validate.c index 8edb1cf85fe..06259c38f03 100644 --- a/src/compiler/nir/nir_validate.c +++ b/src/compiler/nir/nir_validate.c @@ -49,7 +49,7 @@ typedef struct { * equivalent to the uses and defs in nir_register, but built up by the * validator. At the end, we verify that the sets have the same entries. */ - struct set *uses, *if_uses, *defs; + struct set *uses, *defs; nir_function_impl *where_defined; /* NULL for global registers */ } reg_validate_state; @@ -163,7 +163,8 @@ validate_reg_src(nir_src *src, validate_state *state, _mesa_set_add(reg_state->uses, src); } else { validate_assert(state, state->if_stmt); - _mesa_set_add(reg_state->if_uses, src); + validate_assert(state, src->is_if); + _mesa_set_add(reg_state->uses, src); } validate_assert(state, reg_state->where_defined == state->impl && @@ -200,7 +201,7 @@ validate_ssa_src(nir_src *src, validate_state *state, * our use is seen in a use list. */ struct set_entry *entry; - if (state->instr) { + if (!src->is_if && state->instr) { entry = _mesa_set_search(state->ssa_srcs, src); } else { entry = _mesa_set_search(state->ssa_srcs, SET_PTR_BIT(src, 0)); @@ -300,22 +301,16 @@ validate_ssa_def(nir_ssa_def *def, validate_state *state) validate_num_components(state, def->num_components); list_validate(&def->uses); - nir_foreach_use(src, def) { + nir_foreach_use_including_if(src, def) { validate_assert(state, src->is_ssa); validate_assert(state, src->ssa == def); bool already_seen = false; - _mesa_set_search_and_add(state->ssa_srcs, src, &already_seen); - /* A nir_src should only appear once and only in one SSA def use list */ - validate_assert(state, !already_seen); - } - list_validate(&def->if_uses); - nir_foreach_if_use(src, def) { - validate_assert(state, src->is_ssa); - validate_assert(state, src->ssa == def); - bool already_seen = false; - _mesa_set_search_and_add(state->ssa_srcs, SET_PTR_BIT(src, 0), - &already_seen); + nir_src *ssa_src_ptr = src; + if (src->is_if) + ssa_src_ptr = SET_PTR_BIT(ssa_src_ptr, 0); + + _mesa_set_search_and_add(state->ssa_srcs, ssa_src_ptr, &already_seen); /* A nir_src should only appear once and only in one SSA def use list */ validate_assert(state, !already_seen); } @@ -549,7 +544,7 @@ validate_deref_instr(nir_deref_instr *instr, validate_state *state) * conditions expect well-formed Booleans. If you want to compare with * NULL, an explicit comparison operation should be used. */ - validate_assert(state, list_is_empty(&instr->dest.ssa.if_uses)); + validate_assert(state, !nir_ssa_def_used_by_if(&instr->dest.ssa)); /* Certain modes cannot be used as sources for phi instructions because * way too many passes assume that they can always chase deref chains. @@ -1412,6 +1407,7 @@ validate_if(nir_if *if_stmt, validate_state *state) nir_cf_node *next_node = nir_cf_node_next(&if_stmt->cf_node); validate_assert(state, next_node->type == nir_cf_node_block); + validate_assert(state, if_stmt->condition.is_if); validate_src(&if_stmt->condition, state, 0, 1); validate_assert(state, !exec_list_is_empty(&if_stmt->then_list)); @@ -1500,11 +1496,9 @@ prevalidate_reg_decl(nir_register *reg, validate_state *state) list_validate(®->uses); list_validate(®->defs); - list_validate(®->if_uses); reg_validate_state *reg_state = ralloc(state->regs, reg_validate_state); reg_state->uses = _mesa_pointer_set_create(reg_state); - reg_state->if_uses = _mesa_pointer_set_create(reg_state); reg_state->defs = _mesa_pointer_set_create(reg_state); reg_state->where_defined = state->impl; @@ -1520,20 +1514,13 @@ postvalidate_reg_decl(nir_register *reg, validate_state *state) assume(entry); reg_validate_state *reg_state = (reg_validate_state *) entry->data; - nir_foreach_use(src, reg) { + nir_foreach_use_including_if(src, reg) { struct set_entry *entry = _mesa_set_search(reg_state->uses, src); validate_assert(state, entry); _mesa_set_remove(reg_state->uses, entry); } validate_assert(state, reg_state->uses->entries == 0); - nir_foreach_if_use(src, reg) { - struct set_entry *entry = _mesa_set_search(reg_state->if_uses, src); - validate_assert(state, entry); - _mesa_set_remove(reg_state->if_uses, entry); - } - validate_assert(state, reg_state->if_uses->entries == 0); - nir_foreach_def(src, reg) { struct set_entry *entry = _mesa_set_search(reg_state->defs, src); validate_assert(state, entry); diff --git a/src/compiler/nir/tests/ssa_def_bits_used_tests.cpp b/src/compiler/nir/tests/ssa_def_bits_used_tests.cpp index b38eef0d2a0..4c9aa295057 100644 --- a/src/compiler/nir/tests/ssa_def_bits_used_tests.cpp +++ b/src/compiler/nir/tests/ssa_def_bits_used_tests.cpp @@ -50,8 +50,7 @@ protected: static bool is_used_once(const nir_ssa_def *def) { - return list_is_singular(&def->uses) && - list_is_empty(&def->if_uses); + return list_is_singular(&def->uses); } nir_alu_instr * diff --git a/src/freedreno/ir3/ir3_nir_opt_preamble.c b/src/freedreno/ir3/ir3_nir_opt_preamble.c index 2dfacd1fd80..59ad228bb7c 100644 --- a/src/freedreno/ir3/ir3_nir_opt_preamble.c +++ b/src/freedreno/ir3/ir3_nir_opt_preamble.c @@ -47,11 +47,10 @@ def_size(nir_ssa_def *def, unsigned *size, unsigned *align) static bool all_uses_float(nir_ssa_def *def, bool allow_src2) { - nir_foreach_if_use (use, def) { - return false; - } + nir_foreach_use_including_if (use, def) { + if (use->is_if) + return false; - nir_foreach_use (use, def) { nir_instr *use_instr = use->parent_instr; if (use_instr->type != nir_instr_type_alu) return false; @@ -78,11 +77,10 @@ all_uses_float(nir_ssa_def *def, bool allow_src2) static bool all_uses_bit(nir_ssa_def *def) { - nir_foreach_if_use (use, def) { - return false; - } + nir_foreach_use_including_if (use, def) { + if (use->is_if) + return false; - nir_foreach_use (use, def) { nir_instr *use_instr = use->parent_instr; if (use_instr->type != nir_instr_type_alu) return false; diff --git a/src/gallium/auxiliary/nir/nir_to_tgsi.c b/src/gallium/auxiliary/nir/nir_to_tgsi.c index f2316c7a739..236a45b5506 100644 --- a/src/gallium/auxiliary/nir/nir_to_tgsi.c +++ b/src/gallium/auxiliary/nir/nir_to_tgsi.c @@ -763,7 +763,7 @@ ntt_output_decl(struct ntt_compile *c, nir_intrinsic_instr *instr, uint32_t *fra */ static bool ntt_try_store_in_tgsi_output(struct ntt_compile *c, struct ureg_dst *dst, - struct list_head *uses, struct list_head *if_uses) + struct list_head *uses) { *dst = ureg_dst_undef(); @@ -779,10 +779,12 @@ ntt_try_store_in_tgsi_output(struct ntt_compile *c, struct ureg_dst *dst, return false; } - if (!list_is_empty(if_uses) || !list_is_singular(uses)) + if (!list_is_singular(uses)) return false; nir_src *src = list_first_entry(uses, nir_src, use_link); + if (src->is_if) + return false; if (src->parent_instr->type != nir_instr_type_intrinsic) return false; @@ -1099,7 +1101,7 @@ ntt_setup_registers(struct ntt_compile *c, struct exec_list *list) if (nir_reg->num_array_elems == 0) { struct ureg_dst decl; uint32_t write_mask = BITFIELD_MASK(nir_reg->num_components); - if (!ntt_try_store_in_tgsi_output(c, &decl, &nir_reg->uses, &nir_reg->if_uses)) { + if (!ntt_try_store_in_tgsi_output(c, &decl, &nir_reg->uses)) { if (nir_reg->bit_size == 64) { if (nir_reg->num_components > 2) { fprintf(stderr, "NIR-to-TGSI: error: %d-component NIR r%d\n", @@ -1253,7 +1255,7 @@ ntt_get_ssa_def_decl(struct ntt_compile *c, nir_ssa_def *ssa) writemask = ntt_64bit_write_mask(writemask); struct ureg_dst dst; - if (!ntt_try_store_in_tgsi_output(c, &dst, &ssa->uses, &ssa->if_uses)) + if (!ntt_try_store_in_tgsi_output(c, &dst, &ssa->uses)) dst = ntt_temp(c); c->ssa_temp[ssa->index] = ntt_swizzle_for_write_mask(ureg_src(dst), writemask); diff --git a/src/gallium/drivers/etnaviv/etnaviv_compiler_nir.c b/src/gallium/drivers/etnaviv/etnaviv_compiler_nir.c index 70f152c4179..dd39af16f07 100644 --- a/src/gallium/drivers/etnaviv/etnaviv_compiler_nir.c +++ b/src/gallium/drivers/etnaviv/etnaviv_compiler_nir.c @@ -858,9 +858,9 @@ lower_alu(struct etna_compile *c, nir_alu_instr *alu) nir_ssa_def *ssa = alu->src[i].src.ssa; /* check that vecN instruction is only user of this */ - bool need_mov = list_length(&ssa->if_uses) != 0; - nir_foreach_use(use_src, ssa) { - if (use_src->parent_instr != &alu->instr) + bool need_mov = false; + nir_foreach_use_including_if(use_src, ssa) { + if (use_src->is_if || use_src->parent_instr != &alu->instr) need_mov = true; } diff --git a/src/gallium/drivers/etnaviv/etnaviv_compiler_nir.h b/src/gallium/drivers/etnaviv/etnaviv_compiler_nir.h index 4d6ff43bc4e..d1cde4ca0c4 100644 --- a/src/gallium/drivers/etnaviv/etnaviv_compiler_nir.h +++ b/src/gallium/drivers/etnaviv/etnaviv_compiler_nir.h @@ -127,7 +127,7 @@ real_dest(nir_dest *dest, unsigned *swiz, unsigned *mask) if (!dest || !dest->is_ssa) return dest; - bool can_bypass_src = !list_length(&dest->ssa.if_uses); + bool can_bypass_src = nir_ssa_def_used_by_if(&dest->ssa); nir_instr *p_instr = dest->ssa.parent_instr; /* if used by a vecN, the "real" destination becomes the vecN destination @@ -161,7 +161,7 @@ real_dest(nir_dest *dest, unsigned *swiz, unsigned *mask) case nir_op_vec2: case nir_op_vec3: case nir_op_vec4: - assert(list_length(&dest->ssa.if_uses) == 0); + assert(!nir_ssa_def_used_by_if(&dest->ssa)); nir_foreach_use(use_src, &dest->ssa) assert(use_src->parent_instr == instr); @@ -175,7 +175,8 @@ real_dest(nir_dest *dest, unsigned *swiz, unsigned *mask) default: continue; } - if (list_length(&dest->ssa.if_uses) || list_length(&dest->ssa.uses) > 1) + if (nir_ssa_def_used_by_if(&dest->ssa) || + list_length(&dest->ssa.uses) > 1) continue; update_swiz_mask(alu, NULL, swiz, mask); diff --git a/src/gallium/drivers/vc4/vc4_program.c b/src/gallium/drivers/vc4/vc4_program.c index 2a35af61c70..023967fe4f6 100644 --- a/src/gallium/drivers/vc4/vc4_program.c +++ b/src/gallium/drivers/vc4/vc4_program.c @@ -806,14 +806,7 @@ add_output(struct vc4_compile *c, static bool ntq_src_is_only_ssa_def_user(nir_src *src) { - if (!src->is_ssa) - return false; - - if (!list_is_empty(&src->ssa->if_uses)) - return false; - - return (src->ssa->uses.next == &src->use_link && - src->ssa->uses.next->next == &src->ssa->uses); + return src->is_ssa && list_is_singular(&src->ssa->uses); } /** diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp index ba4354d9b8d..75424f30bc8 100644 --- a/src/intel/compiler/brw_fs.cpp +++ b/src/intel/compiler/brw_fs.cpp @@ -7122,7 +7122,10 @@ fs_visitor::run_mesh(bool allow_spilling) static bool is_used_in_not_interp_frag_coord(nir_ssa_def *def) { - nir_foreach_use(src, def) { + nir_foreach_use_including_if(src, def) { + if (src->is_if) + return true; + if (src->parent_instr->type != nir_instr_type_intrinsic) return true; @@ -7131,9 +7134,6 @@ is_used_in_not_interp_frag_coord(nir_ssa_def *def) return true; } - nir_foreach_if_use(src, def) - return true; - return false; } diff --git a/src/intel/compiler/brw_nir_opt_peephole_ffma.c b/src/intel/compiler/brw_nir_opt_peephole_ffma.c index 6969a7edae8..49fa3c35c01 100644 --- a/src/intel/compiler/brw_nir_opt_peephole_ffma.c +++ b/src/intel/compiler/brw_nir_opt_peephole_ffma.c @@ -32,12 +32,11 @@ static inline bool are_all_uses_fadd(nir_ssa_def *def) { - if (!list_is_empty(&def->if_uses)) - return false; + nir_foreach_use_including_if(use_src, def) { + if (use_src->is_if) + return false; - nir_foreach_use(use_src, def) { nir_instr *use_instr = use_src->parent_instr; - if (use_instr->type != nir_instr_type_alu) return false; @@ -148,10 +147,8 @@ any_alu_src_is_a_constant(nir_alu_src srcs[]) nir_load_const_instr *load_const = nir_instr_as_load_const (srcs[i].src.ssa->parent_instr); - if (list_is_singular(&load_const->def.uses) && - list_is_empty(&load_const->def.if_uses)) { + if (list_is_singular(&load_const->def.uses)) return true; - } } } diff --git a/src/microsoft/compiler/dxil_nir_tess.c b/src/microsoft/compiler/dxil_nir_tess.c index 4b94ea25d1d..daff2a10516 100644 --- a/src/microsoft/compiler/dxil_nir_tess.c +++ b/src/microsoft/compiler/dxil_nir_tess.c @@ -230,16 +230,16 @@ dxil_nir_split_tess_ctrl(nir_shader *nir, nir_function **patch_const_func) continue; nir_intrinsic_instr *intr = nir_instr_as_intrinsic(instr); if (intr->intrinsic != nir_intrinsic_load_invocation_id || - list_length(&intr->dest.ssa.uses) + - list_length(&intr->dest.ssa.if_uses) <= 1) + list_length(&intr->dest.ssa.uses) <= 1) continue; - nir_foreach_use_safe(src, &intr->dest.ssa) { - b.cursor = nir_before_src(src, false); - nir_instr_rewrite_src_ssa(src->parent_instr, src, nir_load_invocation_id(&b)); - } - nir_foreach_if_use_safe(src, &intr->dest.ssa) { - b.cursor = nir_before_src(src, true); - nir_if_rewrite_condition_ssa(src->parent_if, src, nir_load_invocation_id(&b)); + nir_foreach_use_including_if_safe(src, &intr->dest.ssa) { + b.cursor = nir_before_src(src, src->is_if); + nir_ssa_def *id = nir_load_invocation_id(&b); + + if (src->is_if) + nir_if_rewrite_condition_ssa(src->parent_if, src, id); + else + nir_instr_rewrite_src_ssa(src->parent_instr, src, id); } nir_instr_remove(instr); } diff --git a/src/panfrost/.clang-format b/src/panfrost/.clang-format index 2d75ff1089f..e6829560e59 100644 --- a/src/panfrost/.clang-format +++ b/src/panfrost/.clang-format @@ -46,6 +46,8 @@ ForEachMacros: - nir_foreach_register_safe - nir_foreach_use - nir_foreach_use_safe + - nir_foreach_use_including_if + - nir_foreach_use_including_if_safe - nir_foreach_if_use - nir_foreach_if_use_safe - nir_foreach_def diff --git a/src/panfrost/midgard/nir_fuse_io_16.c b/src/panfrost/midgard/nir_fuse_io_16.c index f4b052ea438..5fe4761f969 100644 --- a/src/panfrost/midgard/nir_fuse_io_16.c +++ b/src/panfrost/midgard/nir_fuse_io_16.c @@ -77,13 +77,10 @@ nir_fuse_io_16(nir_shader *shader) if (!intr->dest.is_ssa) continue; - if (!list_is_empty(&intr->dest.ssa.if_uses)) - return false; - bool valid = true; - nir_foreach_use(src, &intr->dest.ssa) - valid &= nir_src_is_f2fmp(src); + nir_foreach_use_including_if(src, &intr->dest.ssa) + valid &= !src->is_if && nir_src_is_f2fmp(src); if (!valid) continue; diff --git a/src/panfrost/util/nir_mod_helpers.c b/src/panfrost/util/nir_mod_helpers.c index 7a1fef990a7..466a4d4252a 100644 --- a/src/panfrost/util/nir_mod_helpers.c +++ b/src/panfrost/util/nir_mod_helpers.c @@ -86,13 +86,13 @@ pan_has_dest_mod(nir_dest **odest, nir_op op) return false; /* Check the uses. We want a single use, with the op `op` */ - if (!list_is_empty(&dest->ssa.if_uses)) - return false; - if (!list_is_singular(&dest->ssa.uses)) return false; nir_src *use = list_first_entry(&dest->ssa.uses, nir_src, use_link); + if (use->is_if) + return false; + nir_instr *parent = use->parent_instr; /* Check if the op is `op` */