[Mesa-dev] [PATCH] nir: add helper to get # of src/dest components

[Iago Toral]

On Mon, 2015-06-08 at 15:45 -0400, Rob Clark wrote:
> From: Rob Clark <robclark at freedesktop.org>
> 
> I need something like this in a couple places.  And didn't see anything
> like it anywhere.

We ended up doing something similar in our work-in-progress nir/vec4
pass, it makes the code a bit easier to read, so if nobody else has
objections this is:

Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

> Signed-off-by: Rob Clark <robclark at freedesktop.org>
> ---
> v2: Added similar helper for nir_src, and cleaned up a few places that
> open coded this.  There are a couple left (such as validate_alu_src())
> but that handle is_packed differently so I thought it best to leave
> them as-is.


A quick grep suggests that nobody is ever setting is_packed to true...
the only code that reads is_packed is in nir_validate and it is there
only to avoid doing validations on the number of components when
is_packed is set to true, so I guess it is okay to keep that code as it
is.

Iago

>  src/glsl/nir/nir.h          | 18 ++++++++++++++++++
>  src/glsl/nir/nir_from_ssa.c | 10 ++--------
>  src/glsl/nir/nir_validate.c |  4 +---
>  3 files changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
> index 697d37e..06bbb0c 100644
> --- a/src/glsl/nir/nir.h
> +++ b/src/glsl/nir/nir.h
> @@ -541,6 +541,24 @@ typedef struct {
>  #define nir_foreach_def_safe(reg, dest) \
>     list_for_each_entry_safe(nir_dest, dest, &(reg)->defs, reg.def_link)
>  
> +static inline unsigned
> +nir_dest_num_components(nir_dest *dest)
> +{
> +   if (dest->is_ssa)
> +      return dest->ssa.num_components;
> +   else
> +      return dest->reg.reg->num_components;
> +}
> +
> +static inline unsigned
> +nir_src_num_components(nir_src *src)
> +{
> +   if (src->is_ssa)
> +      return src->ssa->num_components;
> +   else
> +      return src->reg.reg->num_components;
> +}
> +
>  static inline nir_src
>  nir_src_for_ssa(nir_ssa_def *def)
>  {
> diff --git a/src/glsl/nir/nir_from_ssa.c b/src/glsl/nir/nir_from_ssa.c
> index 67733e6..23c798d 100644
> --- a/src/glsl/nir/nir_from_ssa.c
> +++ b/src/glsl/nir/nir_from_ssa.c
> @@ -553,10 +553,7 @@ emit_copy(nir_parallel_copy_instr *pcopy, nir_src src, nir_src dest_src,
>            dest_src.reg.indirect == NULL &&
>            dest_src.reg.base_offset == 0);
>  
> -   if (src.is_ssa)
> -      assert(src.ssa->num_components >= dest_src.reg.reg->num_components);
> -   else
> -      assert(src.reg.reg->num_components >= dest_src.reg.reg->num_components);
> +   assert(nir_src_num_components(&src) == nir_src_num_components(&dest_src));
>  
>     nir_alu_instr *mov = nir_alu_instr_create(mem_ctx, nir_op_imov);
>     nir_src_copy(&mov->src[0].src, &src, mem_ctx);
> @@ -712,10 +709,7 @@ resolve_parallel_copy(nir_parallel_copy_instr *pcopy,
>        nir_register *reg = nir_local_reg_create(state->impl);
>        reg->name = "copy_temp";
>        reg->num_array_elems = 0;
> -      if (values[b].is_ssa)
> -         reg->num_components = values[b].ssa->num_components;
> -      else
> -         reg->num_components = values[b].reg.reg->num_components;
> +      reg->num_components = nir_src_num_components(&values[b]);
>        values[num_vals].is_ssa = false;
>        values[num_vals].reg.reg = reg;
>  
> diff --git a/src/glsl/nir/nir_validate.c b/src/glsl/nir/nir_validate.c
> index da92ed9..c781912 100644
> --- a/src/glsl/nir/nir_validate.c
> +++ b/src/glsl/nir/nir_validate.c
> @@ -262,9 +262,7 @@ validate_dest(nir_dest *dest, validate_state *state)
>  static void
>  validate_alu_dest(nir_alu_dest *dest, validate_state *state)
>  {
> -   unsigned dest_size =
> -      dest->dest.is_ssa ? dest->dest.ssa.num_components
> -                        : dest->dest.reg.reg->num_components;
> +   unsigned dest_size = nir_dest_num_components(&dest->dest);
>     bool is_packed = !dest->dest.is_ssa && dest->dest.reg.reg->is_packed;
>     /*
>      * validate that the instruction doesn't write to components not in the