[Mesa-dev] [PATCH 03/11] nir/cf: Don't break outer-block successors in split_block_beginning().

[Jason Ekstrand]

First three are

Reviewed-by: Jason Ekstrand <jason.ekstrand at 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 <kenneth at 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 <kenneth at whitecape.org>
> Reviewed-by: Connor Abbott <cwabbott0 at 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
>