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

[Antía Puentes]

Hi Rob,

On jue, 2016-05-05 at 14:40 -0400, Rob Clark 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 had noticed this as well. The patch looks good to me:

Reviewed-by: Antia Puentes <apuentes at igalia.com>

> 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");
>     }