On Jul 16, 2015 5:19 PM, "Kenneth Graunke" <kenn...@whitecape.org> wrote:
>
> From: Connor Abbott <connor.w.abb...@intel.com>
>
> This will split the block containing the instruction and put the CF node
> in between.
>
> v2: (by Kenneth Graunke)
> - Simplify split_block_after_instr()'s implementation by using
> split_block_end() rather than duplicating code.
> - Fix a bug in nir_cf_node_insert_after_instr() where inserting a
> non-block after the last instruction would cause update_if_uses()
> to be called twice, making us try to add the same SSA def to the
> if_uses list twice, corrupting the list.
> - Comment changes.
>
> Cc: Jason Ekstrand <ja...@jlekstrand.net>
> Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
> ---
> src/glsl/nir/nir.c | 62
++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/glsl/nir/nir.h | 3 +++
> 2 files changed, 65 insertions(+)
>
> Nothing uses this yet, but I've tested it with my SIMD8 geometry shader
patches,
> which use this to replace emit_vertex intrinsics with if blocks (for
"safety
> checks" that make sure the program hasn't emitted too many vertices). It
seems
> to work just fine, and seems like a really useful piece of infrastructure
to
> have, so I'm submitting it now.
>
> Jason, would you mind reviewing it, since Connor and I both hacked on it?
> It would be nice to have a non-author take a look at it :)
>
> diff --git a/src/glsl/nir/nir.c b/src/glsl/nir/nir.c
> index 78ff886..0c53bab 100644
> --- a/src/glsl/nir/nir.c
> +++ b/src/glsl/nir/nir.c
> @@ -843,6 +843,29 @@ split_block_end(nir_block *block)
> }
>
> /**
> + * Creates a new block, and moves all the instructions after the given
> + * instruction to the new block.
> + */
> +static nir_block *
> +split_block_after_instr(nir_instr *instr)
> +{
> + /* We don't have to do anything special for handling jump
instructions,
> + * as this will move the successors associated with the jump to the
new
> + * block already.
> + */
> + nir_block *new_block = split_block_end(instr->block);
> +
> + nir_instr *cur_instr;
> + while ((cur_instr = nir_instr_next(instr)) != NULL) {
Please use a for loop or pull the iteration expression out. This is really
obtuse.
Otherwise, at first glance this looks pretty good. I'd like to take a
longer look before I call it reviewed though.
> + exec_node_remove(&cur_instr->node);
> + exec_list_push_tail(&new_block->instr_list, &cur_instr->node);
At some point we should get a better mechanism for list splicing. Don't
bother with it now because I'm hoping to move NIR to the list in until
before too long. We can make the change then.
> + cur_instr->block = new_block;
> + }
> +
> + return new_block;
> +}
> +
> +/**
> * Inserts a non-basic block between two basic blocks and links them
together.
> */
>
> @@ -1124,6 +1147,45 @@ nir_cf_node_insert_after(nir_cf_node *node,
nir_cf_node *after)
> }
>
> void
> +nir_cf_node_insert_after_instr(nir_instr *instr, nir_cf_node *after)
> +{
> + /* If the instruction is the last in its block, then this is
equivalent
> + * to inserting the CF node after this block. Just call that, to
avoid
> + * attempting to split blocks unnecessarily.
> + */
> + if (nir_instr_is_last(instr)) {
> + nir_cf_node_insert_after(&instr->block->cf_node, after);
> + return;
> + }
> +
> + update_if_uses(after);
> +
> + if (after->type == nir_cf_node_block) {
> + /* We're attempting to insert a block after an instruction;
instead,
> + * just move all of the instructions into the existing block.
Actually
> + * removing and adding them would involve removing and adding
uses/defs,
> + * which we don't need to do, so just take them off the list
directly.
> + */
> + nir_block *after_block = nir_cf_node_as_block(after);
> + nir_foreach_instr_safe_reverse(after_block, new_instr) {
> + exec_node_remove(&new_instr->node);
> + new_instr->block = instr->block;
> + exec_node_insert_after(&instr->node, &new_instr->node);
> + }
> + } else {
> + /* We're inserting a loop or if after an instruction. Split up the
> + * basic block and insert it between those two blocks.
> + */
> + nir_block *before_block = instr->block;
> + nir_block *after_block = split_block_after_instr(instr);
> + insert_non_block(before_block, after, after_block);
> + }
> +
> + nir_function_impl *impl =
nir_cf_node_get_function(&instr->block->cf_node);
> + nir_metadata_preserve(impl, nir_metadata_none);
> +}
> +
> +void
> nir_cf_node_insert_before(nir_cf_node *node, nir_cf_node *before)
> {
> update_if_uses(before);
> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
> index 62cdbd4..6efbc18 100644
> --- a/src/glsl/nir/nir.h
> +++ b/src/glsl/nir/nir.h
> @@ -1506,6 +1506,9 @@ void nir_cf_node_insert_after(nir_cf_node *node,
nir_cf_node *after);
> /** puts a control flow node immediately before another control flow
node */
> void nir_cf_node_insert_before(nir_cf_node *node, nir_cf_node *before);
>
> +/** puts a control flow node immediately after a given instruction */
> +void nir_cf_node_insert_after_instr(nir_instr *instr, nir_cf_node
*after);
> +
> /** puts a control flow node at the beginning of a list from an if,
loop, or function */
> void nir_cf_node_insert_begin(struct exec_list *list, nir_cf_node *node);
>
> --
> 2.4.5
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
