[Mesa-dev] [PATCH] nir: fix block iterator to not forget fxn->end_block

[Jason Ekstrand]

Before you push this, of like to register my official skepticism as to
whether or not this is the right fix. Given that the end block is special,
anyone who depends on iterating over it should know that and handle it
specially anyway.  I'm pretty sure that block numbering is the only thing
that actually uses it and fixing that is a 1-line change.

Not a NAK just a thought.
On May 5, 2016 11:41 AM, "Rob Clark" <robdclark at gmail.com> wrote:

> From: Rob Clark <robclark at freedesktop.org>
>
> With the switch to new block iterator macro, we silently stopped
> iterating over the end-block.  Which caused nir_index_blocks() to not
> index the end-block.  Resulting in funny looking nir_print's like:
>
> impl main {
>         block block_0:
>         /* preds: */
>         intrinsic copy_var () (gl_FragColor at out-temp, gl_Color) ()
>         intrinsic copy_var () (gl_FragColor, gl_FragColor at out-temp) ()
>         /* succs: block_0 */
>         block block_0:
> }
>
> I don't *think* there are any more serious consequences of not iterating
> the end_block, but it's probably also a good idea to not subtly change
> behavior compared to the old fxn-callback based iterator.
>
> Signed-off-by: Rob Clark <robclark at freedesktop.org>
> ---
>  src/compiler/nir/nir.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
> index 867a43c..1f51b31 100644
> --- a/src/compiler/nir/nir.c
> +++ b/src/compiler/nir/nir.c
> @@ -1512,12 +1512,18 @@ nir_block_cf_tree_next(nir_block *block)
>        return NULL;
>     }
>
> -   nir_cf_node *cf_next = nir_cf_node_next(&block->cf_node);
> +   nir_cf_node *parent = block->cf_node.parent;
> +   nir_cf_node *cf_next;
> +
> +   if ((parent->type == nir_cf_node_function) &&
> +       (block == nir_cf_node_as_function(parent)->end_block))
> +      cf_next = NULL;
> +   else
> +      cf_next = nir_cf_node_next(&block->cf_node);
> +
>     if (cf_next)
>        return nir_cf_node_cf_tree_first(cf_next);
>
> -   nir_cf_node *parent = block->cf_node.parent;
> -
>     switch (parent->type) {
>     case nir_cf_node_if: {
>        /* Are we at the end of the if? Go to the beginning of the else */
> @@ -1532,9 +1538,12 @@ nir_block_cf_tree_next(nir_block *block)
>     case nir_cf_node_loop:
>        return nir_cf_node_as_block(nir_cf_node_next(parent));
>
> -   case nir_cf_node_function:
> +   case nir_cf_node_function: {
> +      nir_function_impl *func = nir_cf_node_as_function(parent);
> +      if (func->end_block != block)
> +         return func->end_block;
>        return NULL;
> -
> +   }
>     default:
>        unreachable("unknown cf node type");
>     }
> --
> 2.5.5
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160508/7a606657/attachment-0001.html>