On Aug 25, 2015 12:24 PM, "Kenneth Graunke" <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote: > > This patch implements a general nir_instr_insert() function that takes a > nir_cursor for the insertion point. It then reworks the existing API to > simply be a wrapper around that for compatibility. > > This largely involves moving the existing code into a new function. > > Suggested by Connor Abbott. > > Signed-off-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> > --- > src/glsl/nir/nir.c | 115 ++++++++++++++++++++++++++--------------------------- > src/glsl/nir/nir.h | 7 ++++ > 2 files changed, 63 insertions(+), 59 deletions(-) > > diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c > index ff758f4..a3e7966 100644 > --- a/src/glsl/nir/nir.c > +++ b/src/glsl/nir/nir.c > @@ -664,102 +664,99 @@ add_defs_uses(nir_instr *instr) > } > > void > +nir_instr_insert(nir_cursor cursor, nir_instr *instr) > +{ > + switch (cursor.option) { > + case nir_cursor_before_block: > + /* Only allow inserting jumps into empty blocks. */ > + if (instr->type == nir_instr_type_jump) > + assert(exec_list_is_empty(&cursor.block->instr_list)); > + > + instr->block = cursor.block; > + add_defs_uses(instr); > + exec_list_push_head(&cursor.block->instr_list, &instr->node); > + break; > + case nir_cursor_after_block: { > + /* Inserting instructions after a jump is illegal. */ > + nir_instr *last = nir_block_last_instr(cursor.block); > + assert(last == NULL || last->type != nir_instr_type_jump); > + (void) last; > + > + instr->block = cursor.block; > + add_defs_uses(instr); > + exec_list_push_tail(&cursor.block->instr_list, &instr->node); > + break; > + } > + case nir_cursor_before_instr: > + assert(instr->type != nir_instr_type_jump); > + instr->block = cursor.instr->block; > + add_defs_uses(instr); > + exec_node_insert_node_before(&cursor.instr->node, &instr->node); > + break; > + case nir_cursor_after_instr: > + /* Inserting instructions after a jump is illegal. */ > + assert(cursor.instr->type != nir_instr_type_jump); > + > + /* Only allow inserting jumps at the end of the block. */ > + if (instr->type == nir_instr_type_jump) > + assert(cursor.instr == nir_block_last_instr(cursor.instr->block)); > + > + instr->block = cursor.instr->block; > + add_defs_uses(instr); > + exec_node_insert_after(&cursor.instr->node, &instr->node); > + break; > + } > + > + if (instr->type == nir_instr_type_jump) > + nir_handle_add_jump(instr->block); > +} > + > +void > nir_instr_insert_before(nir_instr *instr, nir_instr *before) > { > - assert(before->type != nir_instr_type_jump); > - before->block = instr->block; > - add_defs_uses(before); > - exec_node_insert_node_before(&instr->node, &before->node); > + nir_instr_insert(nir_before_instr(instr), before); > } > > void > nir_instr_insert_after(nir_instr *instr, nir_instr *after) > { > - assert(instr->type != nir_instr_type_jump); > - > - if (after->type == nir_instr_type_jump) { > - assert(instr == nir_block_last_instr(instr->block)); > - } > - > - after->block = instr->block; > - add_defs_uses(after); > - exec_node_insert_after(&instr->node, &after->node); > - > - if (after->type == nir_instr_type_jump) > - nir_handle_add_jump(after->block); > + nir_instr_insert(nir_after_instr(instr), after); > } > > void > nir_instr_insert_before_block(nir_block *block, nir_instr *before) > { > - if (before->type == nir_instr_type_jump) > - assert(exec_list_is_empty(&block->instr_list)); > - > - before->block = block; > - add_defs_uses(before); > - exec_list_push_head(&block->instr_list, &before->node); > - > - if (before->type == nir_instr_type_jump) > - nir_handle_add_jump(block); > + nir_instr_insert(nir_before_block(block), before); > } > > void > nir_instr_insert_after_block(nir_block *block, nir_instr *after) > { > - nir_instr *last = nir_block_last_instr(block); > - assert(last == NULL || last->type != nir_instr_type_jump); > - (void) last; > - > - after->block = block; > - add_defs_uses(after); > - exec_list_push_tail(&block->instr_list, &after->node); > - > - if (after->type == nir_instr_type_jump) > - nir_handle_add_jump(block); > + nir_instr_insert(nir_after_block(block), after); > } > > void > nir_instr_insert_before_cf(nir_cf_node *node, nir_instr *before) > { > - if (node->type == nir_cf_node_block) { > - nir_instr_insert_before_block(nir_cf_node_as_block(node), before); > - } else { > - nir_cf_node *prev = nir_cf_node_prev(node); > - assert(prev->type == nir_cf_node_block); > - nir_block *prev_block = nir_cf_node_as_block(prev); > - > - nir_instr_insert_before_block(prev_block, before); > - } > + nir_instr_insert(nir_before_cf_node(node), before); > } > > void > nir_instr_insert_after_cf(nir_cf_node *node, nir_instr *after) > { > - if (node->type == nir_cf_node_block) { > - nir_instr_insert_after_block(nir_cf_node_as_block(node), after); > - } else { > - nir_cf_node *next = nir_cf_node_next(node); > - assert(next->type == nir_cf_node_block); > - nir_block *next_block = nir_cf_node_as_block(next); > - > - nir_instr_insert_before_block(next_block, after); > - } > + nir_instr_insert(nir_after_cf_node(node), after); > } > > void > nir_instr_insert_before_cf_list(struct exec_list *list, nir_instr *before) > { > - nir_cf_node *first_node = exec_node_data(nir_cf_node, > - exec_list_get_head(list), node); > - nir_instr_insert_before_cf(first_node, before); > + nir_instr_insert(nir_before_cf_list(list), before); > } > > void > nir_instr_insert_after_cf_list(struct exec_list *list, nir_instr *after) > { > - nir_cf_node *last_node = exec_node_data(nir_cf_node, > - exec_list_get_tail(list), node); > - nir_instr_insert_after_cf(last_node, after); > + nir_instr_insert(nir_after_cf_list(list), after); > } Since these are now just trivial wrappers, can you make them inline and put them in nir.h? Otherwise, LGTM. Sorry for not pushing my control flow patches - I'll do it when I get back this weekend, remind me if I forget! (Responding on phone so sorry for any horrible line-wrapping...) > > static bool > diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h > index 65e4daf..ac01c86 100644 > --- a/src/glsl/nir/nir.h > +++ b/src/glsl/nir/nir.h > @@ -1641,6 +1641,13 @@ nir_after_cf_list(struct exec_list *cf_list) > return nir_after_cf_node(last_node); > } > > +/** > + * Insert a NIR instruction at the given cursor. > + * > + * Note: This does not update the cursor. > + */ > +void nir_instr_insert(nir_cursor cursor, nir_instr *instr); > + > void nir_instr_insert_before(nir_instr *instr, nir_instr *before); > void nir_instr_insert_after(nir_instr *instr, nir_instr *after); > > -- > 2.5.0 > > _______________________________________________ > mesa-dev mailing list ><a href="mailto:mesa-dev@lists.freedesktop.org"> mesa-dev@lists.freedesktop.org</a> ><a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev"> http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>