[Mesa-dev] [PATCH 3/3] nir: Convert the builder to use the new NIR cursor API.
Connor Abbott
cwabbott0 at gmail.com
Thu Aug 6 16:12:24 PDT 2015
So, I was sort of imagining that we'd want to go a little farther with
this, and make nir_instr_insert_before/after_* a wrapper around a
cursor based API just called "nir_instr_insert". Then
nir_builder_instr_insert() would become even simpler, since it would
just call nir_instr_insert() and then point the cursor to after the
instruction we just inserted (that part seems missing here, btw...
what if I set the cursor to after an instruction and insert multiple
instructions?) and it would be a lot more similar to how control flow
modification works. It would also let us remove a little duplication
from nir_instr_insert_* since we would only need to set the block and
call add_defs_uses() once, and wrappers like
nir_instr_insert_before_cf() would become a lot smaller with the
cursor API handling most of the work. This could be a follow-on, but
doing it before this patch would simplify the patch, so if you're
willing to do the work...
On Thu, Aug 6, 2015 at 10:15 AM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> The NIR cursor API is exactly what we want for the builder's insertion
> point. This simplifies the API, the implementation, and is actually
> more flexible as well.
>
> This required a bit of reworking of TGSI->NIR's if/loop stack handling;
> we now store cursors instead of cf_node_lists, for better or worse.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
> src/gallium/auxiliary/nir/tgsi_to_nir.c | 34 +++++++-------
> .../drivers/freedreno/ir3/ir3_nir_lower_if_else.c | 2 +-
> src/gallium/drivers/vc4/vc4_nir_lower_io.c | 6 +--
> src/glsl/nir/nir_builder.h | 54 ++++++++--------------
> src/glsl/nir/nir_lower_idiv.c | 2 +-
> src/glsl/nir/nir_lower_load_const_to_scalar.c | 2 +-
> src/glsl/nir/nir_lower_tex_projector.c | 2 +-
> src/glsl/nir/nir_normalize_cubemap_coords.c | 2 +-
> src/mesa/program/prog_to_nir.c | 2 +-
> 9 files changed, 46 insertions(+), 60 deletions(-)
>
> I haven't tested the TGSI->NIR changes - I'm honestly not sure how. Could
> someone try them out? Thanks!
>
> diff --git a/src/gallium/auxiliary/nir/tgsi_to_nir.c b/src/gallium/auxiliary/nir/tgsi_to_nir.c
> index b17ac04..4485868 100644
> --- a/src/gallium/auxiliary/nir/tgsi_to_nir.c
> +++ b/src/gallium/auxiliary/nir/tgsi_to_nir.c
> @@ -65,24 +65,24 @@ struct ttn_compile {
> nir_register *addr_reg;
>
> /**
> - * Stack of cf_node_lists where instructions should be pushed as we pop
> + * Stack of nir_cursors where instructions should be pushed as we pop
> * back out of the control flow stack.
> *
> * For each IF/ELSE/ENDIF block, if_stack[if_stack_pos] has where the else
> * instructions should be placed, and if_stack[if_stack_pos - 1] has where
> * the next instructions outside of the if/then/else block go.
> */
> - struct exec_list **if_stack;
> + nir_cursor *if_stack;
> unsigned if_stack_pos;
>
> /**
> - * Stack of cf_node_lists where instructions should be pushed as we pop
> + * Stack of nir_cursors where instructions should be pushed as we pop
> * back out of the control flow stack.
> *
> * loop_stack[loop_stack_pos - 1] contains the cf_node_list for the outside
> * of the loop.
> */
> - struct exec_list **loop_stack;
> + nir_cursor *loop_stack;
> unsigned loop_stack_pos;
>
> /* How many TGSI_FILE_IMMEDIATE vec4s have been parsed so far. */
> @@ -922,7 +922,7 @@ ttn_if(struct ttn_compile *c, nir_ssa_def *src, bool is_uint)
> nir_builder *b = &c->build;
>
> /* Save the outside-of-the-if-statement node list. */
> - c->if_stack[c->if_stack_pos] = b->cf_node_list;
> + c->if_stack[c->if_stack_pos] = b->cursor;
> c->if_stack_pos++;
>
> src = ttn_channel(b, src, X);
> @@ -933,11 +933,11 @@ ttn_if(struct ttn_compile *c, nir_ssa_def *src, bool is_uint)
> } else {
> if_stmt->condition = nir_src_for_ssa(nir_fne(b, src, nir_imm_int(b, 0)));
> }
> - nir_cf_node_insert_end(b->cf_node_list, &if_stmt->cf_node);
> + nir_builder_cf_insert(b, &if_stmt->cf_node);
>
> - nir_builder_insert_after_cf_list(b, &if_stmt->then_list);
> + b->cursor = nir_after_cf_list(&if_stmt->then_list);
>
> - c->if_stack[c->if_stack_pos] = &if_stmt->else_list;
> + c->if_stack[c->if_stack_pos] = nir_after_cf_list(&if_stmt->else_list);
> c->if_stack_pos++;
> }
>
> @@ -946,7 +946,7 @@ ttn_else(struct ttn_compile *c)
> {
> nir_builder *b = &c->build;
>
> - nir_builder_insert_after_cf_list(b, c->if_stack[c->if_stack_pos - 1]);
> + b->cursor = c->if_stack[c->if_stack_pos - 1];
> }
>
> static void
> @@ -955,7 +955,7 @@ ttn_endif(struct ttn_compile *c)
> nir_builder *b = &c->build;
>
> c->if_stack_pos -= 2;
> - nir_builder_insert_after_cf_list(b, c->if_stack[c->if_stack_pos]);
> + b->cursor = c->if_stack[c->if_stack_pos];
> }
>
> static void
> @@ -964,13 +964,13 @@ ttn_bgnloop(struct ttn_compile *c)
> nir_builder *b = &c->build;
>
> /* Save the outside-of-the-loop node list. */
> - c->loop_stack[c->loop_stack_pos] = b->cf_node_list;
> + c->loop_stack[c->loop_stack_pos] = b->cursor;
> c->loop_stack_pos++;
>
> nir_loop *loop = nir_loop_create(b->shader);
> - nir_cf_node_insert_end(b->cf_node_list, &loop->cf_node);
> + nir_builder_cf_insert(b, &loop->cf_node);
>
> - nir_builder_insert_after_cf_list(b, &loop->body);
> + b->cursor = nir_after_cf_list(&loop->body);
> }
>
> static void
> @@ -993,7 +993,7 @@ ttn_endloop(struct ttn_compile *c)
> nir_builder *b = &c->build;
>
> c->loop_stack_pos--;
> - nir_builder_insert_after_cf_list(b, c->loop_stack[c->loop_stack_pos]);
> + b->cursor = c->loop_stack[c->loop_stack_pos];
> }
>
> static void
> @@ -1780,7 +1780,7 @@ tgsi_to_nir(const void *tgsi_tokens,
> nir_function_impl *impl = nir_function_impl_create(overload);
>
> nir_builder_init(&c->build, impl);
> - nir_builder_insert_after_cf_list(&c->build, &impl->body);
> + c->build.cursor = nir_after_cf_list(&impl->body);
>
> tgsi_scan_shader(tgsi_tokens, &scan);
> c->scan = &scan;
> @@ -1799,10 +1799,10 @@ tgsi_to_nir(const void *tgsi_tokens,
> c->num_samp_types = scan.file_max[TGSI_FILE_SAMPLER_VIEW] + 1;
> c->samp_types = rzalloc_array(c, nir_alu_type, c->num_samp_types);
>
> - c->if_stack = rzalloc_array(c, struct exec_list *,
> + c->if_stack = rzalloc_array(c, nir_cursor,
> (scan.opcode_count[TGSI_OPCODE_IF] +
> scan.opcode_count[TGSI_OPCODE_UIF]) * 2);
> - c->loop_stack = rzalloc_array(c, struct exec_list *,
> + c->loop_stack = rzalloc_array(c, nir_cursor,
> scan.opcode_count[TGSI_OPCODE_BGNLOOP]);
>
> ret = tgsi_parse_init(&parser, tgsi_tokens);
> diff --git a/src/gallium/drivers/freedreno/ir3/ir3_nir_lower_if_else.c b/src/gallium/drivers/freedreno/ir3/ir3_nir_lower_if_else.c
> index dc9e462..f5142b6 100644
> --- a/src/gallium/drivers/freedreno/ir3/ir3_nir_lower_if_else.c
> +++ b/src/gallium/drivers/freedreno/ir3/ir3_nir_lower_if_else.c
> @@ -171,7 +171,7 @@ flatten_block(nir_builder *bld, nir_block *if_block, nir_block *prev_block,
> (intr->intrinsic == nir_intrinsic_discard_if)) {
> nir_ssa_def *discard_cond;
>
> - nir_builder_insert_after_instr(bld,
> + bld->cursor = nir_after_instr(
> nir_block_last_instr(prev_block));
>
> if (invert) {
> diff --git a/src/gallium/drivers/vc4/vc4_nir_lower_io.c b/src/gallium/drivers/vc4/vc4_nir_lower_io.c
> index ffc120e..8a87178 100644
> --- a/src/gallium/drivers/vc4/vc4_nir_lower_io.c
> +++ b/src/gallium/drivers/vc4/vc4_nir_lower_io.c
> @@ -59,7 +59,7 @@ vc4_nir_lower_input(struct vc4_compile *c, nir_builder *b,
> /* All TGSI-to-NIR inputs are vec4. */
> assert(intr->num_components == 4);
>
> - nir_builder_insert_before_instr(b, &intr->instr);
> + b->cursor = nir_before_instr(b, &intr->instr);
>
> nir_variable *input_var = NULL;
> foreach_list_typed(nir_variable, var, node, &c->s->inputs) {
> @@ -148,7 +148,7 @@ vc4_nir_lower_output(struct vc4_compile *c, nir_builder *b,
> /* All TGSI-to-NIR outputs are VEC4. */
> assert(intr->num_components == 4);
>
> - nir_builder_insert_before_instr(b, &intr->instr);
> + b->cursor = nir_before_instr(&intr->instr);
>
> for (unsigned i = 0; i < intr->num_components; i++) {
> nir_intrinsic_instr *intr_comp =
> @@ -173,7 +173,7 @@ vc4_nir_lower_uniform(struct vc4_compile *c, nir_builder *b,
> /* All TGSI-to-NIR uniform loads are vec4. */
> assert(intr->num_components == 4);
>
> - nir_builder_insert_before_instr(b, &intr->instr);
> + b->cursor = nir_before_instr(&intr->instr);
>
> /* Generate scalar loads equivalent to the original VEC4. */
> nir_ssa_def *dests[4];
> diff --git a/src/glsl/nir/nir_builder.h b/src/glsl/nir/nir_builder.h
> index 9223e83..28f602a 100644
> --- a/src/glsl/nir/nir_builder.h
> +++ b/src/glsl/nir/nir_builder.h
> @@ -24,12 +24,12 @@
> #ifndef NIR_BUILDER_H
> #define NIR_BUILDER_H
>
> +#include "nir_control_flow.h"
> +
> struct exec_list;
>
> typedef struct nir_builder {
> - struct exec_list *cf_node_list;
> - nir_instr *before_instr;
> - nir_instr *after_instr;
> + nir_cursor cursor;
>
> nir_shader *shader;
> nir_function_impl *impl;
> @@ -44,42 +44,28 @@ nir_builder_init(nir_builder *build, nir_function_impl *impl)
> }
>
> static inline void
> -nir_builder_insert_after_cf_list(nir_builder *build,
> - struct exec_list *cf_node_list)
> -{
> - build->cf_node_list = cf_node_list;
> - build->before_instr = NULL;
> - build->after_instr = NULL;
> -}
> -
> -static inline void
> -nir_builder_insert_before_instr(nir_builder *build, nir_instr *before_instr)
> -{
> - build->cf_node_list = NULL;
> - build->before_instr = before_instr;
> - build->after_instr = NULL;
> -}
> -
> -static inline void
> -nir_builder_insert_after_instr(nir_builder *build, nir_instr *after_instr)
> +nir_builder_instr_insert(nir_builder *build, nir_instr *instr)
> {
> - build->cf_node_list = NULL;
> - build->before_instr = NULL;
> - build->after_instr = after_instr;
> + switch (build->cursor.option) {
> + case nir_cursor_before_block:
> + nir_instr_insert_before_block(build->cursor.block, instr);
> + break;
> + case nir_cursor_after_block:
> + nir_instr_insert_after_block(build->cursor.block, instr);
> + break;
> + case nir_cursor_before_instr:
> + nir_instr_insert_before(build->cursor.instr, instr);
> + break;
> + case nir_cursor_after_instr:
> + nir_instr_insert_after(build->cursor.instr, instr);
> + break;
> + }
> }
>
> static inline void
> -nir_builder_instr_insert(nir_builder *build, nir_instr *instr)
> +nir_builder_cf_insert(nir_builder *build, nir_cf_node *cf)
> {
> - if (build->cf_node_list) {
> - nir_instr_insert_after_cf_list(build->cf_node_list, instr);
> - } else if (build->before_instr) {
> - nir_instr_insert_before(build->before_instr, instr);
> - } else {
> - assert(build->after_instr);
> - nir_instr_insert_after(build->after_instr, instr);
> - build->after_instr = instr;
> - }
> + nir_cf_node_insert(build->cursor, cf);
> }
>
> static inline nir_ssa_def *
> diff --git a/src/glsl/nir/nir_lower_idiv.c b/src/glsl/nir/nir_lower_idiv.c
> index 7b68032..0e1653d 100644
> --- a/src/glsl/nir/nir_lower_idiv.c
> +++ b/src/glsl/nir/nir_lower_idiv.c
> @@ -50,7 +50,7 @@ convert_instr(nir_builder *bld, nir_alu_instr *alu)
>
> is_signed = (op == nir_op_idiv);
>
> - nir_builder_insert_before_instr(bld, &alu->instr);
> + bld->cursor = nir_before_instr(&alu->instr);
>
> numer = nir_ssa_for_src(bld, alu->src[0].src,
> nir_ssa_alu_instr_src_components(alu, 0));
> diff --git a/src/glsl/nir/nir_lower_load_const_to_scalar.c b/src/glsl/nir/nir_lower_load_const_to_scalar.c
> index a90e524..b83ef05 100644
> --- a/src/glsl/nir/nir_lower_load_const_to_scalar.c
> +++ b/src/glsl/nir/nir_lower_load_const_to_scalar.c
> @@ -43,7 +43,7 @@ lower_load_const_instr_scalar(nir_load_const_instr *lower)
>
> nir_builder b;
> nir_builder_init(&b, nir_cf_node_get_function(&lower->instr.block->cf_node));
> - nir_builder_insert_before_instr(&b, &lower->instr);
> + b.cursor = nir_before_instr(&lower->instr);
>
> /* Emit the individual loads. */
> nir_ssa_def *loads[4];
> diff --git a/src/glsl/nir/nir_lower_tex_projector.c b/src/glsl/nir/nir_lower_tex_projector.c
> index 357131c..8a482b1 100644
> --- a/src/glsl/nir/nir_lower_tex_projector.c
> +++ b/src/glsl/nir/nir_lower_tex_projector.c
> @@ -46,7 +46,7 @@ nir_lower_tex_projector_block(nir_block *block, void *void_state)
> continue;
>
> nir_tex_instr *tex = nir_instr_as_tex(instr);
> - nir_builder_insert_before_instr(b, &tex->instr);
> + b->cursor = nir_before_instr(&tex->instr);
>
> /* Find the projector in the srcs list, if present. */
> int proj_index;
> diff --git a/src/glsl/nir/nir_normalize_cubemap_coords.c b/src/glsl/nir/nir_normalize_cubemap_coords.c
> index 0da8447..75b647f 100644
> --- a/src/glsl/nir/nir_normalize_cubemap_coords.c
> +++ b/src/glsl/nir/nir_normalize_cubemap_coords.c
> @@ -52,7 +52,7 @@ normalize_cubemap_coords_block(nir_block *block, void *void_state)
> if (tex->sampler_dim != GLSL_SAMPLER_DIM_CUBE)
> continue;
>
> - nir_builder_insert_before_instr(b, &tex->instr);
> + b->cursor = nir_before_instr(&tex->instr);
>
> for (unsigned i = 0; i < tex->num_srcs; i++) {
> if (tex->src[i].src_type != nir_tex_src_coord)
> diff --git a/src/mesa/program/prog_to_nir.c b/src/mesa/program/prog_to_nir.c
> index 29e0c19..d6eb13b 100644
> --- a/src/mesa/program/prog_to_nir.c
> +++ b/src/mesa/program/prog_to_nir.c
> @@ -1102,7 +1102,7 @@ prog_to_nir(const struct gl_program *prog,
>
> c->build.shader = s;
> c->build.impl = impl;
> - nir_builder_insert_after_cf_list(&c->build, &impl->body);
> + c->build.cursor = nir_after_cf_list(&impl->body);
>
> setup_registers_and_variables(c);
> if (unlikely(c->error))
> --
> 2.4.6
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list