From 36b29201faa19263a5f95e056e9cb3c3590747ec Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Mon, 22 May 2023 13:04:05 -0400 Subject: [PATCH] nir: Produce intrinsics in lower_{phis,ssa_defs}_to_regs A number of passes lower SSA partially to registers, do work that would be invalid in SSA, and then go back into SSA with nir_lower_regs_to_ssa. As a step towards replacing nir_register with intrinsics, the nir_lower_{phis,ssa_defs}_to_regs passes are changed to produce intrinsics instead of nir_registers, and their callers are updated to call nir_lower_reg_intrinsics_to_ssa instead of nir_lower_regs_to_ssa to compensate. Jointly authored with Faith. Signed-off-by: Alyssa Rosenzweig Reviewed-by: Faith Ekstrand Part-of: --- src/compiler/nir/nir.h | 4 + src/compiler/nir/nir_from_ssa.c | 123 +++++++++++++----- .../nir/nir_lower_continue_constructs.c | 2 +- src/compiler/nir/nir_lower_goto_ifs.c | 2 +- src/compiler/nir/nir_lower_shader_calls.c | 1 + src/compiler/nir/nir_opt_if.c | 8 +- src/compiler/nir/nir_opt_loop_unroll.c | 2 +- src/compiler/nir/nir_opt_trivial_continues.c | 2 +- 8 files changed, 102 insertions(+), 42 deletions(-) diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index 1f3319467f2..4ae2410c33e 100644 --- a/src/compiler/nir/nir.h +++ b/src/compiler/nir/nir.h @@ -5989,6 +5989,10 @@ void nir_divergence_analysis(nir_shader *shader); bool nir_update_instr_divergence(nir_shader *shader, nir_instr *instr); bool nir_has_divergent_loop(nir_shader *shader); +void +nir_rewrite_uses_to_load_reg(struct nir_builder *b, nir_ssa_def *old, + nir_ssa_def *reg); + /* If phi_webs_only is true, only convert SSA values involved in phi nodes to * registers. If false, convert all values (even those not involved in a phi * node) to registers. If reg_intrinsics is true, it will use diff --git a/src/compiler/nir/nir_from_ssa.c b/src/compiler/nir/nir_from_ssa.c index e53c7a1e115..5d216ca88b6 100644 --- a/src/compiler/nir/nir_from_ssa.c +++ b/src/compiler/nir/nir_from_ssa.c @@ -23,6 +23,7 @@ #include "nir.h" #include "nir_builder.h" +#include "nir_builder_opcodes.h" #include "nir_vla.h" /* @@ -525,6 +526,44 @@ create_reg_for_ssa_def(nir_ssa_def *def, nir_function_impl *impl) return reg; } +static nir_ssa_def * +decl_reg_for_ssa_def(nir_builder *b, nir_ssa_def *def) +{ + return nir_decl_reg(b, def->num_components, def->bit_size, 0); +} + +void +nir_rewrite_uses_to_load_reg(nir_builder *b, nir_ssa_def *old, + nir_ssa_def *reg) +{ + nir_foreach_use_including_if_safe(use, old) { + b->cursor = nir_before_src(use); + + /* If the immediate preceding instruction is a load_reg from the same + * register, use it instead of creating a new load_reg. This helps when + * a register is referenced in multiple sources in the same instruction, + * which otherwise would turn into piles of unnecessary moves. + */ + nir_ssa_def *load = NULL; + if (b->cursor.option == nir_cursor_before_instr) { + nir_instr *prev = nir_instr_prev(b->cursor.instr); + + if (prev != NULL && prev->type == nir_instr_type_intrinsic) { + nir_intrinsic_instr *intr = nir_instr_as_intrinsic(prev); + if (intr->intrinsic == nir_intrinsic_load_reg && + intr->src[0].ssa == reg && + nir_intrinsic_base(intr) == 0) + load = &intr->dest.ssa; + } + } + + if (load == NULL) + load = nir_load_reg(b, reg); + + nir_src_rewrite_ssa(use, load); + } +} + static bool rewrite_ssa_def(nir_ssa_def *def, void *void_state) { @@ -921,7 +960,7 @@ nir_convert_from_ssa(nir_shader *shader, static void -place_phi_read(nir_builder *b, nir_register *reg, +place_phi_read(nir_builder *b, nir_ssa_def *reg, nir_ssa_def *def, nir_block *block, struct set *visited_blocks) { /* Search already visited blocks to avoid back edges in tree */ @@ -952,7 +991,7 @@ place_phi_read(nir_builder *b, nir_register *reg, } b->cursor = nir_after_block_before_jump(block); - nir_store_register(b, reg, def, ~0); + nir_store_reg(b, def, reg); } /** Lower all of the phi nodes in a block to movs to and from a register @@ -994,24 +1033,17 @@ nir_lower_phis_to_regs_block(nir_block *block) bool progress = false; nir_foreach_phi_safe(phi, block) { assert(phi->dest.is_ssa); - nir_register *reg = create_reg_for_ssa_def(&phi->dest.ssa, b.impl); + nir_ssa_def *reg = decl_reg_for_ssa_def(&b, &phi->dest.ssa); b.cursor = nir_after_instr(&phi->instr); - nir_ssa_def *def = nir_load_register(&b, reg); - - nir_ssa_def_rewrite_uses(&phi->dest.ssa, def); + nir_ssa_def_rewrite_uses(&phi->dest.ssa, nir_load_reg(&b, reg)); nir_foreach_phi_src(src, phi) { - if (src->src.is_ssa) { - _mesa_set_add(visited_blocks, src->src.ssa->parent_instr->block); - place_phi_read(&b, reg, src->src.ssa, src->pred, visited_blocks); - _mesa_set_clear(visited_blocks, NULL); - } else { - b.cursor = nir_after_block_before_jump(src->pred); - nir_ssa_def *src_ssa = - nir_ssa_for_src(&b, src->src, phi->dest.ssa.num_components); - nir_store_register(&b, reg, src_ssa, ~0); - } + assert(src->src.is_ssa); + + _mesa_set_add(visited_blocks, src->src.ssa->parent_instr->block); + place_phi_read(&b, reg, src->src.ssa, src->pred, visited_blocks); + _mesa_set_clear(visited_blocks, NULL); } nir_instr_remove(&phi->instr); @@ -1037,14 +1069,14 @@ dest_replace_ssa_with_reg(nir_dest *dest, void *void_state) if (!dest->is_ssa) return true; - nir_register *reg = create_reg_for_ssa_def(&dest->ssa, state->impl); + nir_builder b; + nir_builder_init(&b, state->impl); - nir_ssa_def_rewrite_uses_src(&dest->ssa, nir_src_for_reg(reg)); + nir_ssa_def *reg = decl_reg_for_ssa_def(&b, &dest->ssa); + nir_rewrite_uses_to_load_reg(&b, &dest->ssa, reg); - nir_instr *instr = dest->ssa.parent_instr; - *dest = nir_dest_for_reg(reg); - dest->reg.parent_instr = instr; - list_addtail(&dest->reg.def_link, ®->defs); + b.cursor = nir_after_instr(dest->ssa.parent_instr); + nir_store_reg(&b, &dest->ssa, reg); state->progress = true; @@ -1066,6 +1098,22 @@ ssa_def_is_local_to_block(nir_ssa_def *def, UNUSED void *state) return true; } +static bool +instr_is_load_new_reg(nir_instr *instr, unsigned old_num_ssa) +{ + if (instr->type != nir_instr_type_intrinsic) + return false; + + nir_intrinsic_instr *load = nir_instr_as_intrinsic(instr); + if (load->intrinsic != nir_intrinsic_load_reg) + return false; + + assert(load->src[0].is_ssa); + nir_ssa_def *reg = load->src[0].ssa; + + return reg->index >= old_num_ssa; +} + /** Lower all of the SSA defs in a block to registers * * This performs the very simple operation of blindly replacing all of the SSA @@ -1077,30 +1125,37 @@ bool nir_lower_ssa_defs_to_regs_block(nir_block *block) { nir_function_impl *impl = nir_cf_node_get_function(&block->cf_node); - nir_shader *shader = impl->function->shader; + nir_builder b = nir_builder_create(impl); struct ssa_def_to_reg_state state = { .impl = impl, .progress = false, }; - nir_foreach_instr(instr, block) { + /* Save off the current number of SSA defs so we can detect which regs + * we've added vs. regs that were already there. + */ + const unsigned num_ssa = impl->ssa_alloc; + + nir_foreach_instr_safe(instr, block) { if (instr->type == nir_instr_type_ssa_undef) { /* Undefs are just a read of something never written. */ nir_ssa_undef_instr *undef = nir_instr_as_ssa_undef(instr); - nir_register *reg = create_reg_for_ssa_def(&undef->def, state.impl); - nir_ssa_def_rewrite_uses_src(&undef->def, nir_src_for_reg(reg)); + nir_ssa_def *reg = decl_reg_for_ssa_def(&b, &undef->def); + nir_rewrite_uses_to_load_reg(&b, &undef->def, reg); } else if (instr->type == nir_instr_type_load_const) { - /* Constant loads are SSA-only, we need to insert a move */ nir_load_const_instr *load = nir_instr_as_load_const(instr); - nir_register *reg = create_reg_for_ssa_def(&load->def, state.impl); - nir_ssa_def_rewrite_uses_src(&load->def, nir_src_for_reg(reg)); + nir_ssa_def *reg = decl_reg_for_ssa_def(&b, &load->def); + nir_rewrite_uses_to_load_reg(&b, &load->def, reg); - nir_alu_instr *mov = nir_alu_instr_create(shader, nir_op_mov); - mov->src[0].src = nir_src_for_ssa(&load->def); - mov->dest.dest = nir_dest_for_reg(reg); - mov->dest.write_mask = (1 << reg->num_components) - 1; - nir_instr_insert(nir_after_instr(&load->instr), &mov->instr); + b.cursor = nir_after_instr(instr); + nir_store_reg(&b, &load->def, reg); + } else if (instr_is_load_new_reg(instr, num_ssa)) { + /* Calls to nir_rewrite_uses_to_load_reg() may place new load_reg + * intrinsics in this block with new SSA destinations. To avoid + * infinite recursion, we don't want to lower any newly placed + * load_reg instructions to yet anoter load/store_reg. + */ } else if (nir_foreach_ssa_def(instr, ssa_def_is_local_to_block, NULL)) { /* If the SSA def produced by this instruction is only in the block * in which it is defined and is not used by ifs or phis, then we diff --git a/src/compiler/nir/nir_lower_continue_constructs.c b/src/compiler/nir/nir_lower_continue_constructs.c index e8619f1dcff..7d55d7730f4 100644 --- a/src/compiler/nir/nir_lower_continue_constructs.c +++ b/src/compiler/nir/nir_lower_continue_constructs.c @@ -144,7 +144,7 @@ lower_continue_constructs_impl(nir_function_impl *impl) nir_metadata_preserve(impl, nir_metadata_none); /* Merge the Phis from Header and Continue Target */ - nir_lower_regs_to_ssa_impl(impl); + nir_lower_reg_intrinsics_to_ssa_impl(impl); /* Re-inserting the Continue Target at the beginning of the loop * violates the dominance property if instructions in the continue diff --git a/src/compiler/nir/nir_lower_goto_ifs.c b/src/compiler/nir/nir_lower_goto_ifs.c index 50f42971d15..038c3161c57 100644 --- a/src/compiler/nir/nir_lower_goto_ifs.c +++ b/src/compiler/nir/nir_lower_goto_ifs.c @@ -975,7 +975,7 @@ nir_lower_goto_ifs_impl(nir_function_impl *impl) nir_metadata_preserve(impl, nir_metadata_none); nir_repair_ssa_impl(impl); - nir_lower_regs_to_ssa_impl(impl); + nir_lower_reg_intrinsics_to_ssa_impl(impl); return true; } diff --git a/src/compiler/nir/nir_lower_shader_calls.c b/src/compiler/nir/nir_lower_shader_calls.c index 565d647198f..a2d4f652313 100644 --- a/src/compiler/nir/nir_lower_shader_calls.c +++ b/src/compiler/nir/nir_lower_shader_calls.c @@ -1261,6 +1261,7 @@ lower_resume(nir_shader *shader, int call_idx) * of all those pesky registers we just added. */ NIR_PASS_V(shader, nir_lower_regs_to_ssa); + NIR_PASS_V(shader, nir_lower_reg_intrinsics_to_ssa); } /* Re-index nir_ssa_def::index. We don't care about actual liveness in diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c index 992f1e9af3e..854ecba6207 100644 --- a/src/compiler/nir/nir_opt_if.c +++ b/src/compiler/nir/nir_opt_if.c @@ -806,9 +806,9 @@ opt_if_loop_last_continue(nir_loop *loop, bool aggressive_last_continue) else nir_cf_reinsert(&tmp, nir_after_cf_list(&nif->then_list)); - /* In order to avoid running nir_lower_regs_to_ssa_impl() every time an if - * opt makes progress we leave nir_opt_trivial_continues() to remove the - * continue now that the end of the loop has been simplified. + /* In order to avoid running nir_lower_reg_intrinsics_to_ssa_impl() every + * time an if opt makes progress we leave nir_opt_trivial_continues() to + * remove the continue now that the end of the loop has been simplified. */ return true; @@ -1700,7 +1700,7 @@ nir_opt_if(nir_shader *shader, nir_opt_if_options options) * need to convert registers back into SSA defs and clean up SSA defs * that don't dominate their uses. */ - nir_lower_regs_to_ssa_impl(impl); + nir_lower_reg_intrinsics_to_ssa_impl(impl); } if (preserve) { diff --git a/src/compiler/nir/nir_opt_loop_unroll.c b/src/compiler/nir/nir_opt_loop_unroll.c index 63208eb807d..801187a1e50 100644 --- a/src/compiler/nir/nir_opt_loop_unroll.c +++ b/src/compiler/nir/nir_opt_loop_unroll.c @@ -1142,7 +1142,7 @@ nir_opt_loop_unroll_impl(nir_function_impl *impl, if (progress) { nir_metadata_preserve(impl, nir_metadata_none); - nir_lower_regs_to_ssa_impl(impl); + nir_lower_reg_intrinsics_to_ssa_impl(impl); } else { nir_metadata_preserve(impl, nir_metadata_all); } diff --git a/src/compiler/nir/nir_opt_trivial_continues.c b/src/compiler/nir/nir_opt_trivial_continues.c index fab4c337cb3..d24725ba5c4 100644 --- a/src/compiler/nir/nir_opt_trivial_continues.c +++ b/src/compiler/nir/nir_opt_trivial_continues.c @@ -126,7 +126,7 @@ nir_opt_trivial_continues(nir_shader *shader) nir_metadata_preserve(impl, nir_metadata_none); /* If that made progress, we're no longer really in SSA form. */ - nir_lower_regs_to_ssa_impl(impl); + nir_lower_reg_intrinsics_to_ssa_impl(impl); progress = true; } else { nir_metadata_preserve(impl, nir_metadata_all);