From 673cc9323ac1214dbfa759946c2ffa869ca68962 Mon Sep 17 00:00:00 2001 From: Emma Anholt Date: Wed, 7 Jul 2021 13:24:45 -0700 Subject: [PATCH] nir: Move phi src setup to a helper. Cleans up the ralloc/list push code all over the tree. Reviewed-by: Matt Turner Part-of: --- src/compiler/nir/nir.c | 22 ++++++++++++++++ src/compiler/nir/nir.h | 1 + src/compiler/nir/nir_builder.h | 12 ++------- src/compiler/nir/nir_clone.c | 13 +--------- src/compiler/nir/nir_control_flow.c | 9 +------ src/compiler/nir/nir_lower_bit_size.c | 11 ++------ src/compiler/nir/nir_lower_phis_to_scalar.c | 6 +---- src/compiler/nir/nir_opt_if.c | 35 +++++++------------------- src/compiler/nir/nir_opt_phi_precision.c | 10 ++------ src/compiler/nir/nir_phi_builder.c | 7 ++---- src/compiler/nir/nir_serialize.c | 10 +++----- src/compiler/nir/nir_to_lcssa.c | 6 +---- src/compiler/nir/tests/lower_returns_tests.cpp | 6 +---- src/compiler/nir/tests/opt_if_tests.cpp | 16 +++--------- src/microsoft/compiler/dxil_nir.c | 6 +---- 15 files changed, 52 insertions(+), 118 deletions(-) diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c index 46a971e..7b1e7a5 100644 --- a/src/compiler/nir/nir.c +++ b/src/compiler/nir/nir.c @@ -749,6 +749,28 @@ nir_phi_instr_create(nir_shader *shader) return instr; } +/** + * Adds a new source to a NIR instruction. + * + * Note that this does not update the def/use relationship for src, assuming + * that the instr is not in the shader. If it is, you have to do: + * + * list_addtail(&phi_src->src.use_link, &src.ssa->uses); + */ +nir_phi_src * +nir_phi_instr_add_src(nir_phi_instr *instr, nir_block *pred, nir_src src) +{ + nir_phi_src *phi_src; + + phi_src = rzalloc(instr, nir_phi_src); + phi_src->pred = pred; + phi_src->src = src; + phi_src->src.parent_instr = &instr->instr; + exec_list_push_tail(&instr->srcs, &phi_src->node); + + return phi_src; +} + nir_parallel_copy_instr * nir_parallel_copy_instr_create(nir_shader *shader) { diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index 167806a..f750035 100644 --- a/src/compiler/nir/nir.h +++ b/src/compiler/nir/nir.h @@ -3887,6 +3887,7 @@ nir_call_instr *nir_call_instr_create(nir_shader *shader, nir_tex_instr *nir_tex_instr_create(nir_shader *shader, unsigned num_srcs); nir_phi_instr *nir_phi_instr_create(nir_shader *shader); +nir_phi_src *nir_phi_instr_add_src(nir_phi_instr *instr, nir_block *pred, nir_src src); nir_parallel_copy_instr *nir_parallel_copy_instr_create(nir_shader *shader); diff --git a/src/compiler/nir/nir_builder.h b/src/compiler/nir/nir_builder.h index b1def3e..e1e248b 100644 --- a/src/compiler/nir/nir_builder.h +++ b/src/compiler/nir/nir_builder.h @@ -208,16 +208,8 @@ nir_if_phi(nir_builder *build, nir_ssa_def *then_def, nir_ssa_def *else_def) nir_if *nif = nir_cf_node_as_if(nir_cf_node_prev(&block->cf_node)); nir_phi_instr *phi = nir_phi_instr_create(build->shader); - - nir_phi_src *src = ralloc(phi, nir_phi_src); - src->pred = nir_if_last_then_block(nif); - src->src = nir_src_for_ssa(then_def); - exec_list_push_tail(&phi->srcs, &src->node); - - src = ralloc(phi, nir_phi_src); - src->pred = nir_if_last_else_block(nif); - src->src = nir_src_for_ssa(else_def); - exec_list_push_tail(&phi->srcs, &src->node); + nir_phi_instr_add_src(phi, nir_if_last_then_block(nif), nir_src_for_ssa(then_def)); + nir_phi_instr_add_src(phi, nir_if_last_else_block(nif), nir_src_for_ssa(else_def)); assert(then_def->num_components == else_def->num_components); assert(then_def->bit_size == else_def->bit_size); diff --git a/src/compiler/nir/nir_clone.c b/src/compiler/nir/nir_clone.c index c3ad328..db46d40 100644 --- a/src/compiler/nir/nir_clone.c +++ b/src/compiler/nir/nir_clone.c @@ -448,23 +448,12 @@ clone_phi(clone_state *state, const nir_phi_instr *phi, nir_block *nblk) nir_instr_insert_after_block(nblk, &nphi->instr); foreach_list_typed(nir_phi_src, src, node, &phi->srcs) { - nir_phi_src *nsrc = ralloc(nphi, nir_phi_src); - - /* Just copy the old source for now. */ - memcpy(nsrc, src, sizeof(*src)); - - /* Since we're not letting nir_insert_instr handle use/def stuff for us, - * we have to set the parent_instr manually. It doesn't really matter - * when we do it, so we might as well do it here. - */ - nsrc->src.parent_instr = &nphi->instr; + nir_phi_src *nsrc = nir_phi_instr_add_src(nphi, src->pred, src->src); /* Stash it in the list of phi sources. We'll walk this list and fix up * sources at the very end of clone_function_impl. */ list_add(&nsrc->src.use_link, &state->phi_srcs); - - exec_list_push_tail(&nphi->srcs, &nsrc->node); } return nphi; diff --git a/src/compiler/nir/nir_control_flow.c b/src/compiler/nir/nir_control_flow.c index 59a7524..6cf0f1a 100644 --- a/src/compiler/nir/nir_control_flow.c +++ b/src/compiler/nir/nir_control_flow.c @@ -240,15 +240,8 @@ nir_insert_phi_undef(nir_block *block, nir_block *pred) phi->dest.ssa.num_components, phi->dest.ssa.bit_size); nir_instr_insert_before_cf_list(&impl->body, &undef->instr); - nir_phi_src *src = ralloc(phi, nir_phi_src); - src->pred = pred; - src->src.parent_instr = &phi->instr; - src->src.is_ssa = true; - src->src.ssa = &undef->def; - + nir_phi_src *src = nir_phi_instr_add_src(phi, pred, nir_src_for_ssa(&undef->def)); list_addtail(&src->src.use_link, &undef->def.uses); - - exec_list_push_tail(&phi->srcs, &src->node); } } diff --git a/src/compiler/nir/nir_lower_bit_size.c b/src/compiler/nir/nir_lower_bit_size.c index 5473ea7..e31d9c8 100644 --- a/src/compiler/nir/nir_lower_bit_size.c +++ b/src/compiler/nir/nir_lower_bit_size.c @@ -292,15 +292,8 @@ split_phi(nir_builder *b, nir_phi_instr *phi) nir_ssa_def *x = nir_unpack_64_2x32_split_x(b, src->src.ssa); nir_ssa_def *y = nir_unpack_64_2x32_split_y(b, src->src.ssa); - nir_phi_src *xsrc = rzalloc(lowered[0], nir_phi_src); - xsrc->pred = src->pred; - xsrc->src = nir_src_for_ssa(x); - exec_list_push_tail(&lowered[0]->srcs, &xsrc->node); - - nir_phi_src *ysrc = rzalloc(lowered[1], nir_phi_src); - ysrc->pred = src->pred; - ysrc->src = nir_src_for_ssa(y); - exec_list_push_tail(&lowered[1]->srcs, &ysrc->node); + nir_phi_instr_add_src(lowered[0], src->pred, nir_src_for_ssa(x)); + nir_phi_instr_add_src(lowered[1], src->pred, nir_src_for_ssa(y)); } nir_ssa_dest_init(&lowered[0]->instr, &lowered[0]->dest, diff --git a/src/compiler/nir/nir_lower_phis_to_scalar.c b/src/compiler/nir/nir_lower_phis_to_scalar.c index 694fdb3..2fcd71d 100644 --- a/src/compiler/nir/nir_lower_phis_to_scalar.c +++ b/src/compiler/nir/nir_lower_phis_to_scalar.c @@ -248,11 +248,7 @@ lower_phis_to_scalar_block(nir_block *block, else nir_instr_insert_after_block(src->pred, &mov->instr); - nir_phi_src *new_src = ralloc(new_phi, nir_phi_src); - new_src->pred = src->pred; - new_src->src = nir_src_for_ssa(&mov->dest.dest.ssa); - - exec_list_push_tail(&new_phi->srcs, &new_src->node); + nir_phi_instr_add_src(new_phi, src->pred, nir_src_for_ssa(&mov->dest.dest.ssa)); } nir_instr_insert_before(&phi->instr, &new_phi->instr); diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c index 55eff27..20a72d6 100644 --- a/src/compiler/nir/nir_opt_if.c +++ b/src/compiler/nir/nir_opt_if.c @@ -521,17 +521,8 @@ opt_split_alu_of_phi(nir_builder *b, nir_loop *loop) * result of the new instruction from continue_block. */ nir_phi_instr *const phi = nir_phi_instr_create(b->shader); - nir_phi_src *phi_src; - - phi_src = ralloc(phi, nir_phi_src); - phi_src->pred = prev_block; - phi_src->src = nir_src_for_ssa(prev_value); - exec_list_push_tail(&phi->srcs, &phi_src->node); - - phi_src = ralloc(phi, nir_phi_src); - phi_src->pred = continue_block; - phi_src->src = nir_src_for_ssa(alu_copy); - exec_list_push_tail(&phi->srcs, &phi_src->node); + nir_phi_instr_add_src(phi, prev_block, nir_src_for_ssa(prev_value)); + nir_phi_instr_add_src(phi, continue_block, nir_src_for_ssa(alu_copy)); nir_ssa_dest_init(&phi->instr, &phi->dest, alu_copy->num_components, alu_copy->bit_size, NULL); @@ -687,21 +678,13 @@ opt_simplify_bcsel_of_phi(nir_builder *b, nir_loop *loop) */ nir_block *continue_block = find_continue_block(loop); nir_phi_instr *const phi = nir_phi_instr_create(b->shader); - nir_phi_src *phi_src; - - phi_src = ralloc(phi, nir_phi_src); - phi_src->pred = prev_block; - phi_src->src = - nir_phi_get_src_from_block(nir_instr_as_phi(bcsel->src[entry_src].src.ssa->parent_instr), - prev_block)->src; - exec_list_push_tail(&phi->srcs, &phi_src->node); - - phi_src = ralloc(phi, nir_phi_src); - phi_src->pred = continue_block; - phi_src->src = - nir_phi_get_src_from_block(nir_instr_as_phi(bcsel->src[continue_src].src.ssa->parent_instr), - continue_block)->src; - exec_list_push_tail(&phi->srcs, &phi_src->node); + nir_phi_instr_add_src(phi, prev_block, + nir_phi_get_src_from_block(nir_instr_as_phi(bcsel->src[entry_src].src.ssa->parent_instr), + prev_block)->src); + + nir_phi_instr_add_src(phi, continue_block, + nir_phi_get_src_from_block(nir_instr_as_phi(bcsel->src[continue_src].src.ssa->parent_instr), + continue_block)->src); nir_ssa_dest_init(&phi->instr, &phi->dest, diff --git a/src/compiler/nir/nir_opt_phi_precision.c b/src/compiler/nir/nir_opt_phi_precision.c index 449e891..6213d5b 100644 --- a/src/compiler/nir/nir_opt_phi_precision.c +++ b/src/compiler/nir/nir_opt_phi_precision.c @@ -240,10 +240,7 @@ try_move_narrowing_dst(nir_builder *b, nir_phi_instr *phi) nir_ssa_def *new_src = nir_build_alu(b, op, old_src, NULL, NULL, NULL); /* and add corresponding phi_src to the new_phi: */ - nir_phi_src *phi_src = ralloc(new_phi, nir_phi_src); - phi_src->pred = src->pred; - phi_src->src = nir_src_for_ssa(new_src); - exec_list_push_tail(&new_phi->srcs, &phi_src->node); + nir_phi_instr_add_src(new_phi, src->pred, nir_src_for_ssa(new_src)); } /* And finally rewrite the original uses of the original phi uses to @@ -420,10 +417,7 @@ try_move_widening_src(nir_builder *b, nir_phi_instr *phi) } /* add corresponding phi_src to the new_phi: */ - nir_phi_src *phi_src = ralloc(new_phi, nir_phi_src); - phi_src->pred = src->pred; - phi_src->src = nir_src_for_ssa(new_src); - exec_list_push_tail(&new_phi->srcs, &phi_src->node); + nir_phi_instr_add_src(new_phi, src->pred, nir_src_for_ssa(new_src)); } /* And insert the new phi after all sources are in place: */ diff --git a/src/compiler/nir/nir_phi_builder.c b/src/compiler/nir/nir_phi_builder.c index 5525a71..0a9e81f 100644 --- a/src/compiler/nir/nir_phi_builder.c +++ b/src/compiler/nir/nir_phi_builder.c @@ -287,11 +287,8 @@ nir_phi_builder_finish(struct nir_phi_builder *pb) nir_block **preds = nir_block_get_predecessors_sorted(phi->instr.block, pb); for (unsigned i = 0; i < phi->instr.block->predecessors->entries; i++) { - nir_phi_src *src = ralloc(phi, nir_phi_src); - src->pred = preds[i]; - src->src = nir_src_for_ssa( - nir_phi_builder_value_get_block_def(val, preds[i])); - exec_list_push_tail(&phi->srcs, &src->node); + nir_phi_instr_add_src(phi, preds[i], + nir_src_for_ssa(nir_phi_builder_value_get_block_def(val, preds[i]))); } ralloc_free(preds); diff --git a/src/compiler/nir/nir_serialize.c b/src/compiler/nir/nir_serialize.c index f9fc319..78fa2ee 100644 --- a/src/compiler/nir/nir_serialize.c +++ b/src/compiler/nir/nir_serialize.c @@ -1590,11 +1590,9 @@ read_phi(read_ctx *ctx, nir_block *blk, union packed_instr header) nir_instr_insert_after_block(blk, &phi->instr); for (unsigned i = 0; i < header.phi.num_srcs; i++) { - nir_phi_src *src = ralloc(phi, nir_phi_src); - - src->src.is_ssa = true; - src->src.ssa = (nir_ssa_def *)(uintptr_t) blob_read_uint32(ctx->blob); - src->pred = (nir_block *)(uintptr_t) blob_read_uint32(ctx->blob); + nir_ssa_def *def = (nir_ssa_def *)(uintptr_t) blob_read_uint32(ctx->blob); + nir_block *pred = (nir_block *)(uintptr_t) blob_read_uint32(ctx->blob); + nir_phi_src *src = nir_phi_instr_add_src(phi, pred, nir_src_for_ssa(def)); /* Since we're not letting nir_insert_instr handle use/def stuff for us, * we have to set the parent_instr manually. It doesn't really matter @@ -1606,8 +1604,6 @@ read_phi(read_ctx *ctx, nir_block *blk, union packed_instr header) * sources at the very end of read_function_impl. */ list_add(&src->src.use_link, &ctx->phi_srcs); - - exec_list_push_tail(&phi->srcs, &src->node); } return phi; diff --git a/src/compiler/nir/nir_to_lcssa.c b/src/compiler/nir/nir_to_lcssa.c index 27b9b26..1a2c24d 100644 --- a/src/compiler/nir/nir_to_lcssa.c +++ b/src/compiler/nir/nir_to_lcssa.c @@ -234,11 +234,7 @@ convert_loop_exit_for_ssa(nir_ssa_def *def, void *void_state) */ uint32_t num_exits = state->block_after_loop->predecessors->entries; for (uint32_t i = 0; i < num_exits; i++) { - nir_phi_src *phi_src = ralloc(phi, nir_phi_src); - phi_src->src = nir_src_for_ssa(def); - phi_src->pred = state->exit_blocks[i]; - - exec_list_push_tail(&phi->srcs, &phi_src->node); + nir_phi_instr_add_src(phi, state->exit_blocks[i], nir_src_for_ssa(def)); } nir_instr_insert_before_block(state->block_after_loop, &phi->instr); diff --git a/src/compiler/nir/tests/lower_returns_tests.cpp b/src/compiler/nir/tests/lower_returns_tests.cpp index d34f26b..83b9fcd 100644 --- a/src/compiler/nir/tests/lower_returns_tests.cpp +++ b/src/compiler/nir/tests/lower_returns_tests.cpp @@ -56,11 +56,7 @@ nir_phi_instr *create_one_source_phi(nir_shader *shader, nir_block *pred, { nir_phi_instr *phi = nir_phi_instr_create(shader); - nir_phi_src *phi_src; - phi_src = ralloc(phi, nir_phi_src); - phi_src->pred = pred; - phi_src->src = nir_src_for_ssa(def); - exec_list_push_tail(&phi->srcs, &phi_src->node); + nir_phi_instr_add_src(phi, pred, nir_src_for_ssa(def)); nir_ssa_dest_init(&phi->instr, &phi->dest, def->num_components, def->bit_size, NULL); diff --git a/src/compiler/nir/tests/opt_if_tests.cpp b/src/compiler/nir/tests/opt_if_tests.cpp index 250c5a0..e636a33 100644 --- a/src/compiler/nir/tests/opt_if_tests.cpp +++ b/src/compiler/nir/tests/opt_if_tests.cpp @@ -123,11 +123,7 @@ TEST_F(nir_opt_if_test, opt_if_simplification_single_source_phi_after_if) nir_phi_instr *const phi = nir_phi_instr_create(bld.shader); - nir_phi_src *phi_src; - phi_src = ralloc(phi, nir_phi_src); - phi_src->pred = then_block; - phi_src->src = nir_src_for_ssa(one); - exec_list_push_tail(&phi->srcs, &phi_src->node); + nir_phi_instr_add_src(phi, then_block, nir_src_for_ssa(one)); nir_ssa_dest_init(&phi->instr, &phi->dest, one->num_components, one->bit_size, NULL); @@ -154,19 +150,13 @@ TEST_F(nir_opt_if_test, opt_if_alu_of_phi_progress) nir_ssa_dest_init(&phi->instr, &phi->dest, x->num_components, x->bit_size, NULL); - nir_phi_src *phi_src_x = ralloc(phi, nir_phi_src); - phi_src_x->pred = x->parent_instr->block; - phi_src_x->src = nir_src_for_ssa(x); - exec_list_push_tail(&phi->srcs, &phi_src_x->node); + nir_phi_instr_add_src(phi, x->parent_instr->block, nir_src_for_ssa(x)); nir_ssa_def *y = nir_iadd(&bld, &phi->dest.ssa, two); nir_store_var(&bld, out_var, nir_imul(&bld, &phi->dest.ssa, two), 1); - nir_phi_src *phi_src_y = ralloc(phi, nir_phi_src); - phi_src_y->pred = nir_cursor_current_block(bld.cursor); - phi_src_y->src = nir_src_for_ssa(y); - exec_list_push_tail(&phi->srcs, &phi_src_y->node); + nir_phi_instr_add_src(phi, nir_cursor_current_block(bld.cursor), nir_src_for_ssa(y)); } nir_pop_loop(&bld, loop); diff --git a/src/microsoft/compiler/dxil_nir.c b/src/microsoft/compiler/dxil_nir.c index 597d046..cb27054 100644 --- a/src/microsoft/compiler/dxil_nir.c +++ b/src/microsoft/compiler/dxil_nir.c @@ -1069,11 +1069,7 @@ cast_phi(nir_builder *b, nir_phi_instr *phi, unsigned new_bit_size) b->cursor = nir_after_instr_and_phis(src->src.ssa->parent_instr); nir_ssa_def *cast = nir_build_alu(b, upcast_op, src->src.ssa, NULL, NULL, NULL); - - nir_phi_src *new_src = rzalloc(lowered, nir_phi_src); - new_src->pred = src->pred; - new_src->src = nir_src_for_ssa(cast); - exec_list_push_tail(&lowered->srcs, &new_src->node); + nir_phi_instr_add_src(lowered, src->pred, nir_src_for_ssa(cast)); } nir_ssa_dest_init(&lowered->instr, &lowered->dest, -- 2.7.4