[Mesa-stable] [Mesa-dev] [PATCH 1/5] nir/cf: Remove phi sources if needed in nir_handle_add_jump

[Iago Toral]

On Sat, 2018-09-22 at 16:39 -0500, Jason Ekstrand wrote:
> If the block in which the jump is inserted is the predecessor of a
> phi
> then we need to remove phi sources otherwise the phi may end up with
> things improperly connected.  Found by running the Vulkan CTS with
> SPIR-V optimizations enabled.
> 
> Cc: mesa-stable at lists.freedesktop.org
> ---
>  src/compiler/nir/nir_control_flow.c | 36 +++++++++++++++----------
> ----
>  1 file changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/src/compiler/nir/nir_control_flow.c
> b/src/compiler/nir/nir_control_flow.c
> index 3b0a0f1a5b0..a82f35550b8 100644
> --- a/src/compiler/nir/nir_control_flow.c
> +++ b/src/compiler/nir/nir_control_flow.c
> @@ -437,6 +437,23 @@ nearest_loop(nir_cf_node *node)
>     return nir_cf_node_as_loop(node);
>  }
>  
> +static void
> +remove_phi_src(nir_block *block, nir_block *pred)
> +{
> +   nir_foreach_instr(instr, block) {
> +      if (instr->type != nir_instr_type_phi)
> +         break;
> +
> +      nir_phi_instr *phi = nir_instr_as_phi(instr);
> +      nir_foreach_phi_src_safe(src, phi) {
> +         if (src->pred == pred) {
> +            list_del(&src->src.use_link);
> +            exec_node_remove(&src->node);
> +         }
> +      }
> +   }
> +}
> +
>  /*
>   * update the CFG after a jump instruction has been added to the end
> of a block
>   */
> @@ -447,6 +464,8 @@ nir_handle_add_jump(nir_block *block)
>     nir_instr *instr = nir_block_last_instr(block);
>     nir_jump_instr *jump_instr = nir_instr_as_jump(instr);
>  
> +   if (block->successors[0])
> +      remove_phi_src(block->successors[0], block);

Don't we need to do the same for block->successors[1]?

Iago

>     unlink_block_successors(block);
>  
>     nir_function_impl *impl = nir_cf_node_get_function(&block-
> >cf_node);
> @@ -470,23 +489,6 @@ nir_handle_add_jump(nir_block *block)
>     }
>  }
>  
> -static void
> -remove_phi_src(nir_block *block, nir_block *pred)
> -{
> -   nir_foreach_instr(instr, block) {
> -      if (instr->type != nir_instr_type_phi)
> -         break;
> -
> -      nir_phi_instr *phi = nir_instr_as_phi(instr);
> -      nir_foreach_phi_src_safe(src, phi) {
> -         if (src->pred == pred) {
> -            list_del(&src->src.use_link);
> -            exec_node_remove(&src->node);
> -         }
> -      }
> -   }
> -}
> -
>  /* Removes the successor of a block with a jump. Note that the jump
> to be
>   * eliminated may be free-floating.
>   */