[Mesa-dev] [PATCH 10/12] nir: add bit_size parameter to system values with multiple allowed bit sizes

Jason Ekstrand jason at jlekstrand.net
Mon Jan 7 17:20:42 UTC 2019


First off, thank you very much for cleaning this up!

On Tue, Dec 4, 2018 at 12:27 PM Karol Herbst <kherbst at redhat.com> wrote:

> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> ---
>  src/compiler/nir/nir_builder_opcodes_h.py     | 14 ++++++++++++--
>  src/compiler/nir/nir_lower_system_values.c    |  4 ++--
>  src/gallium/drivers/vc4/vc4_nir_lower_blend.c |  4 ++--
>  3 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/src/compiler/nir/nir_builder_opcodes_h.py
> b/src/compiler/nir/nir_builder_opcodes_h.py
> index 34b8c4371e1..f2e33071c6d 100644
> --- a/src/compiler/nir/nir_builder_opcodes_h.py
> +++ b/src/compiler/nir/nir_builder_opcodes_h.py
> @@ -44,13 +44,14 @@ nir_${name}(nir_builder *build,
> ${src_decl_list(opcode.num_inputs)})
>
>  /* Generic builder for system values. */
>  static inline nir_ssa_def *
> -nir_load_system_value(nir_builder *build, nir_intrinsic_op op, int index)
> +nir_load_system_value(nir_builder *build, nir_intrinsic_op op, int index,
> +                      unsigned bit_size)
>  {
>     nir_intrinsic_instr *load = nir_intrinsic_instr_create(build->shader,
> op);
>     load->num_components = nir_intrinsic_infos[op].dest_components;
>     load->const_index[0] = index;
>     nir_ssa_dest_init(&load->instr, &load->dest,
> -                     nir_intrinsic_infos[op].dest_components, 32, NULL);
> +                     nir_intrinsic_infos[op].dest_components, bit_size,
> NULL);
>     nir_builder_instr_insert(build, &load->instr);
>     return &load->dest.ssa;
>  }
> @@ -60,6 +61,8 @@ def sysval_decl_list(opcode):
>     res = ''
>     if opcode.indices:
>        res += ', unsigned ' + opcode.indices[0].lower()
> +   if len(opcode.bit_sizes) > 1:
> +      res += ', unsigned bit_size'
>

This if statement is out-of-sync with the one below.  Perhaps
"len(opcodes.bit_sizes) != 1" instead?  It may also be good to add "assert
len(opcode.bit_sizes) > 0" somewhere to make it clear what our assumptions
are.


>     return res
>
>  def sysval_arg_list(opcode):
> @@ -68,6 +71,13 @@ def sysval_arg_list(opcode):
>        args.append(opcode.indices[0].lower())
>     else:
>        args.append('0')
> +
> +   if len(opcode.bit_sizes) == 1:
> +      bit_size = opcode.bit_sizes[0]
> +      args.append(str(bit_size))
> +   else:
> +      args.append('bit_size')
> +
>     return ', '.join(args)
>  %>
>
> diff --git a/src/compiler/nir/nir_lower_system_values.c
> b/src/compiler/nir/nir_lower_system_values.c
> index 08a9e8be44a..68b0ea89c8d 100644
> --- a/src/compiler/nir/nir_lower_system_values.c
> +++ b/src/compiler/nir/nir_lower_system_values.c
> @@ -261,8 +261,8 @@ convert_block(nir_block *block, nir_builder *b)
>        if (sysval == NULL) {
>           nir_intrinsic_op sysval_op =
>              nir_intrinsic_from_system_value(var->data.location);
> -         sysval = nir_load_system_value(b, sysval_op, 0);
> -         sysval->bit_size = load_deref->dest.ssa.bit_size;
> +         sysval = nir_load_system_value(b, sysval_op, 0,
> +                                        load_deref->dest.ssa.bit_size);
>

This is so gross.... I'm happy to see it gone!


>        }
>
>        nir_ssa_def_rewrite_uses(&load_deref->dest.ssa,
> nir_src_for_ssa(sysval));
> diff --git a/src/gallium/drivers/vc4/vc4_nir_lower_blend.c
> b/src/gallium/drivers/vc4/vc4_nir_lower_blend.c
> index 60eccb4fc00..f80558722a1 100644
> --- a/src/gallium/drivers/vc4/vc4_nir_lower_blend.c
> +++ b/src/gallium/drivers/vc4/vc4_nir_lower_blend.c
> @@ -130,7 +130,7 @@ vc4_blend_channel_f(nir_builder *b,
>                  return nir_load_system_value(b,
>
> nir_intrinsic_load_blend_const_color_r_float +
>                                               channel,
> -                                             0);
> +                                             0, 32);
>          case PIPE_BLENDFACTOR_CONST_ALPHA:
>                  return nir_load_blend_const_color_a_float(b);
>          case PIPE_BLENDFACTOR_ZERO:
> @@ -148,7 +148,7 @@ vc4_blend_channel_f(nir_builder *b,
>                                  nir_load_system_value(b,
>
>  nir_intrinsic_load_blend_const_color_r_float +
>                                                        channel,
> -                                                      0));
> +                                                      0, 32));
>          case PIPE_BLENDFACTOR_INV_CONST_ALPHA:
>                  return nir_fsub(b, nir_imm_float(b, 1.0),
>                                  nir_load_blend_const_color_a_float(b));
> --
> 2.19.2
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190107/d2aaa4b0/attachment.html>


More information about the mesa-dev mailing list