First three are
Reviewed-by: Jason Ekstrand <jason.ekstr...@intel.com>
I'm going to need more brain power for the rest.
--Jason
On Tue, Sep 22, 2015 at 8:01 PM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> Consider the following NIR:
>
> block block_0;
> /* succs: block_1 block_2 */
> if (...) {
> block block_1;
> ...
> } else {
> block block_2;
> }
>
> Calling split_block_beginning() on block_1 would break block_0's
> successors: link_block() sets both successors of a block, so calling
> link_block(block_0, new_block, NULL) would throw away the second
> successor, leaving only /* succ: new_block */. This is invalid: the
> block before an if statement must have two successors.
>
> Changing the call to link_block(pred, new_block, pred->successors[0])
> would correctly leave both successors in place, but because unlink_block
> may shift successor[1] to successor[0], it may not preserve the original
> order. NIR maintains a convention that successor[0] must point to the
> "then" block, while successor[1] points to the "else" block, so we need
> to take care to preserve this ordering.
>
> This patch creates a new function that swaps out one successor for
> another, preserving the ordering. It then uses this to fix the issue.
>
> Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
> Reviewed-by: Connor Abbott <cwabbo...@gmail.com>
> ---
> src/glsl/nir/nir_control_flow.c | 21 ++++++++++++++++++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> ***UNCHANGED SINCE THE FIRST SEND***
>
> diff --git a/src/glsl/nir/nir_control_flow.c b/src/glsl/nir/nir_control_flow.c
> index 43e4e43..e2a151d 100644
> --- a/src/glsl/nir/nir_control_flow.c
> +++ b/src/glsl/nir/nir_control_flow.c
> @@ -200,6 +200,23 @@ link_block_to_non_block(nir_block *block, nir_cf_node
> *node)
> }
>
> /**
> + * Replace a block's successor with a different one.
> + */
> +static void
> +replace_successor(nir_block *block, nir_block *old_succ, nir_block *new_succ)
> +{
> + if (block->successors[0] == old_succ) {
> + block->successors[0] = new_succ;
> + } else {
> + assert(block->successors[1] == old_succ);
> + block->successors[1] = new_succ;
> + }
> +
> + block_remove_pred(old_succ, block);
> + block_add_pred(new_succ, block);
> +}
> +
> +/**
> * Takes a basic block and inserts a new empty basic block before it, making
> its
> * predecessors point to the new block. This essentially splits the block
> into
> * an empty header and a body so that another non-block CF node can be
> inserted
> @@ -217,9 +234,7 @@ split_block_beginning(nir_block *block)
> struct set_entry *entry;
> set_foreach(block->predecessors, entry) {
> nir_block *pred = (nir_block *) entry->key;
> -
> - unlink_blocks(pred, block);
> - link_blocks(pred, new_block, NULL);
> + replace_successor(pred, block, new_block);
> }
>
> /* Any phi nodes must stay part of the new block, or else their
> --
> 2.5.3
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
