[Mesa-dev] [PATCH v4 07/44] spirv/nir: Handle 16-bit types

Jason Ekstrand jason at jlekstrand.net
Thu Nov 30 20:24:25 UTC 2017


I sprinkled a few mostly trivial comments below.  With those fixed,

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

On Wed, Nov 29, 2017 at 6:07 PM, Jose Maria Casanova Crespo <
jmcasanova at igalia.com> wrote:

> From: Eduardo Lima Mitev <elima at igalia.com>
>
> v2: Added more missing implementations of 16-bit types. (Jason Ekstrand)
>
> v3: Store values in values[0].u16[i] (Jason Ekstrand)
>     Include switches based on bitsize for 16-bit types
>     (Chema Casanova)
>
> Signed-off-by: Jose Maria Casanova Crespo <jmcasanova at igalia.com>
> Signed-off-by: Eduardo Lima <elima at igalia.com>
> ---
>  src/compiler/spirv/spirv_to_nir.c  | 111 ++++++++++++++++++++++++++++++
> +------
>  src/compiler/spirv/vtn_variables.c |  21 +++++++
>  2 files changed, 115 insertions(+), 17 deletions(-)
>
> diff --git a/src/compiler/spirv/spirv_to_nir.c
> b/src/compiler/spirv/spirv_to_nir.c
> index 027efab88d..f745373473 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -104,10 +104,13 @@ vtn_const_ssa_value(struct vtn_builder *b,
> nir_constant *constant,
>     switch (glsl_get_base_type(type)) {
>     case GLSL_TYPE_INT:
>     case GLSL_TYPE_UINT:
> +   case GLSL_TYPE_INT16:
> +   case GLSL_TYPE_UINT16:
>     case GLSL_TYPE_INT64:
>     case GLSL_TYPE_UINT64:
>     case GLSL_TYPE_BOOL:
>     case GLSL_TYPE_FLOAT:
> +   case GLSL_TYPE_FLOAT16:
>     case GLSL_TYPE_DOUBLE: {
>        int bit_size = glsl_get_bit_size(type);
>        if (glsl_type_is_vector_or_scalar(type)) {
> @@ -751,16 +754,38 @@ vtn_handle_type(struct vtn_builder *b, SpvOp opcode,
>        int bit_size = w[2];
>        const bool signedness = w[3];
>        val->type->base_type = vtn_base_type_scalar;
> -      if (bit_size == 64)
> +      switch (bit_size) {
> +      case 64:
>           val->type->type = (signedness ? glsl_int64_t_type() :
> glsl_uint64_t_type());
> -      else
> +         break;
> +      case 32:
>           val->type->type = (signedness ? glsl_int_type() :
> glsl_uint_type());
> +         break;
> +      case 16:
> +         val->type->type = (signedness ? glsl_int16_t_type() :
> glsl_uint16_t_type());
> +         break;
> +      default:
> +         unreachable("Invalid int bit size");
> +      }
>        break;
>     }
> +
>     case SpvOpTypeFloat: {
>        int bit_size = w[2];
>        val->type->base_type = vtn_base_type_scalar;
> -      val->type->type = bit_size == 64 ? glsl_double_type() :
> glsl_float_type();
> +      switch (bit_size) {
> +      case 16:
> +         val->type->type = glsl_float16_t_type();
> +         break;
> +      case 32:
> +         val->type->type = glsl_float_type();
> +         break;
> +      case 64:
> +         val->type->type = glsl_double_type();
> +         break;
> +      default:
> +         assert(!"Invalid float bit size");
>

unreachable()


> +      }
>        break;
>     }
>
> @@ -980,10 +1005,13 @@ vtn_null_constant(struct vtn_builder *b, const
> struct glsl_type *type)
>     switch (glsl_get_base_type(type)) {
>     case GLSL_TYPE_INT:
>     case GLSL_TYPE_UINT:
> +   case GLSL_TYPE_INT16:
> +   case GLSL_TYPE_UINT16:
>     case GLSL_TYPE_INT64:
>     case GLSL_TYPE_UINT64:
>     case GLSL_TYPE_BOOL:
>     case GLSL_TYPE_FLOAT:
> +   case GLSL_TYPE_FLOAT16:
>     case GLSL_TYPE_DOUBLE:
>        /* Nothing to do here.  It's already initialized to zero */
>        break;
> @@ -1106,12 +1134,20 @@ vtn_handle_constant(struct vtn_builder *b, SpvOp
> opcode,
>     case SpvOpConstant: {
>        assert(glsl_type_is_scalar(val->const_type));
>        int bit_size = glsl_get_bit_size(val->const_type);
> -      if (bit_size == 64) {
> +      switch (bit_size) {
> +      case 64: {
>           val->constant->values->u32[0] = w[3];
>           val->constant->values->u32[1] = w[4];
>

A bit unrelated but this should be using vtn_u64_literal and setting u64[0]
instead of assuming the aliasing works out between u32 and u64.


> -      } else {
> -         assert(bit_size == 32);
> +         break;
> +      }
>

You don't need braces around the 64-bit case.


> +      case 32:
>           val->constant->values->u32[0] = w[3];
> +         break;
> +      case 16:
> +         val->constant->values->u16[0] = w[3];
> +         break;
> +      default:
> +         unreachable("Unsupported SpvOpConstant bit size");
>        }
>        break;
>     }
> @@ -1119,11 +1155,21 @@ vtn_handle_constant(struct vtn_builder *b, SpvOp
> opcode,
>        assert(glsl_type_is_scalar(val->const_type));
>        val->constant->values[0].u32[0] = get_specialization(b, val, w[3]);
>        int bit_size = glsl_get_bit_size(val->const_type);
> -      if (bit_size == 64)
> +      switch (bit_size) {
> +      case 64:{
>           val->constant->values[0].u64[0] =
>              get_specialization64(b, val, vtn_u64_literal(&w[3]));
> -      else
> +         break;
> +      }
>

Same here.


> +      case 32:
>           val->constant->values[0].u32[0] = get_specialization(b, val,
> w[3]);
> +         break;
> +      case 16:
> +         val->constant->values[0].u16[0] = get_specialization(b, val,
> w[3]);
> +         break;
> +      default:
> +         unreachable("Unsupported SpvOpSpecConstant bit size");
> +      }
>        break;
>     }
>     case SpvOpSpecConstantComposite:
> @@ -1136,9 +1182,12 @@ vtn_handle_constant(struct vtn_builder *b, SpvOp
> opcode,
>        switch (glsl_get_base_type(val->const_type)) {
>        case GLSL_TYPE_UINT:
>        case GLSL_TYPE_INT:
> +      case GLSL_TYPE_UINT16:
> +      case GLSL_TYPE_INT16:
>        case GLSL_TYPE_UINT64:
>        case GLSL_TYPE_INT64:
>        case GLSL_TYPE_FLOAT:
> +      case GLSL_TYPE_FLOAT16:
>        case GLSL_TYPE_BOOL:
>        case GLSL_TYPE_DOUBLE: {
>           int bit_size = glsl_get_bit_size(val->const_type);
> @@ -1150,11 +1199,18 @@ vtn_handle_constant(struct vtn_builder *b, SpvOp
> opcode,
>              assert(glsl_type_is_vector(val->const_type));
>              assert(glsl_get_vector_elements(val->const_type) ==
> elem_count);
>              for (unsigned i = 0; i < elem_count; i++) {
> -               if (bit_size == 64) {
> +               switch (bit_size) {
> +               case 64:
>                    val->constant->values[0].u64[i] =
> elems[i]->values[0].u64[0];
> -               } else {
> -                  assert(bit_size == 32);
> +                  break;
> +               case 32:
>                    val->constant->values[0].u32[i] =
> elems[i]->values[0].u32[0];
> +                  break;
> +               case 16:
> +                  val->constant->values[0].u16[i] =
> elems[i]->values[0].u16[0];
> +                  break;
> +               default:
> +                  unreachable("Invalid SpvOpConstantComposite bit size");
>                 }
>              }
>           }
> @@ -1228,6 +1284,7 @@ vtn_handle_constant(struct vtn_builder *b, SpvOp
> opcode,
>                    val->constant->values[0].u64[j] = u64[comp];
>              }
>           } else {
> +            /* This is for both 32-bit and 16-bit values */
>              uint32_t u32[8];
>              if (v0->value_type == vtn_value_type_constant) {
>                 for (unsigned i = 0; i < len0; i++)
> @@ -1276,9 +1333,12 @@ vtn_handle_constant(struct vtn_builder *b, SpvOp
> opcode,
>              switch (glsl_get_base_type(type)) {
>              case GLSL_TYPE_UINT:
>              case GLSL_TYPE_INT:
> +            case GLSL_TYPE_UINT16:
> +            case GLSL_TYPE_INT16:
>              case GLSL_TYPE_UINT64:
>              case GLSL_TYPE_INT64:
>              case GLSL_TYPE_FLOAT:
> +            case GLSL_TYPE_FLOAT16:
>              case GLSL_TYPE_DOUBLE:
>              case GLSL_TYPE_BOOL:
>                 /* If we hit this granularity, we're picking off an
> element */
> @@ -1316,11 +1376,18 @@ vtn_handle_constant(struct vtn_builder *b, SpvOp
> opcode,
>                 unsigned num_components = glsl_get_vector_elements(type);
>                 unsigned bit_size = glsl_get_bit_size(type);
>                 for (unsigned i = 0; i < num_components; i++)
> -                  if (bit_size == 64) {
> +                  switch(bit_size) {
> +                  case 64:
>                       val->constant->values[0].u64[i] =
> (*c)->values[col].u64[elem + i];
> -                  } else {
> -                     assert(bit_size == 32);
> +                     break;
> +                  case 32:
>                       val->constant->values[0].u32[i] =
> (*c)->values[col].u32[elem + i];
> +                     break;
> +                  case 16:
> +                     val->constant->values[0].u16[i] =
> (*c)->values[col].u16[elem + i];
> +                     break;
> +                  default:
> +                     unreachable("Invalid SpvOpCompositeExtract bit
> size");
>                    }
>              }
>           } else {
> @@ -1333,11 +1400,18 @@ vtn_handle_constant(struct vtn_builder *b, SpvOp
> opcode,
>                 unsigned num_components = glsl_get_vector_elements(type);
>                 unsigned bit_size = glsl_get_bit_size(type);
>                 for (unsigned i = 0; i < num_components; i++)
> -                  if (bit_size == 64) {
> +                  switch (bit_size) {
> +                  case 64:
>                       (*c)->values[col].u64[elem + i] =
> insert->constant->values[0].u64[i];
> -                  } else {
> -                     assert(bit_size == 32);
> +                     break;
> +                  case 32:
>                       (*c)->values[col].u32[elem + i] =
> insert->constant->values[0].u32[i];
> +                     break;
> +                  case 16:
> +                     (*c)->values[col].u16[elem + i] =
> insert->constant->values[0].u16[i];
> +                     break;
> +                  default:
> +                     unreachable("Invalid SpvOpCompositeInsert bit size");
>                    }
>              }
>           }
> @@ -1449,10 +1523,13 @@ vtn_create_ssa_value(struct vtn_builder *b, const
> struct glsl_type *type)
>           switch (glsl_get_base_type(type)) {
>           case GLSL_TYPE_INT:
>           case GLSL_TYPE_UINT:
> +         case GLSL_TYPE_INT16:
> +         case GLSL_TYPE_UINT16:
>           case GLSL_TYPE_INT64:
>           case GLSL_TYPE_UINT64:
>           case GLSL_TYPE_BOOL:
>           case GLSL_TYPE_FLOAT:
> +         case GLSL_TYPE_FLOAT16:
>           case GLSL_TYPE_DOUBLE:
>              child_type = glsl_get_column_type(type);
>              break;
> diff --git a/src/compiler/spirv/vtn_variables.c b/src/compiler/spirv/vtn_
> variables.c
> index 9a69b4f6fc..7eb0b6d22a 100644
> --- a/src/compiler/spirv/vtn_variables.c
> +++ b/src/compiler/spirv/vtn_variables.c
> @@ -186,9 +186,12 @@ vtn_ssa_offset_pointer_dereference(struct
> vtn_builder *b,
>        switch (glsl_get_base_type(type->type)) {
>        case GLSL_TYPE_UINT:
>        case GLSL_TYPE_INT:
> +      case GLSL_TYPE_UINT16:
> +      case GLSL_TYPE_INT16:
>        case GLSL_TYPE_UINT64:
>        case GLSL_TYPE_INT64:
>        case GLSL_TYPE_FLOAT:
> +      case GLSL_TYPE_FLOAT16:
>        case GLSL_TYPE_DOUBLE:
>        case GLSL_TYPE_BOOL:
>        case GLSL_TYPE_ARRAY: {
> @@ -298,9 +301,12 @@ vtn_pointer_to_deref(struct vtn_builder *b, struct
> vtn_pointer *ptr)
>        switch (base_type) {
>        case GLSL_TYPE_UINT:
>        case GLSL_TYPE_INT:
> +      case GLSL_TYPE_UINT16:
> +      case GLSL_TYPE_INT16:
>        case GLSL_TYPE_UINT64:
>        case GLSL_TYPE_INT64:
>        case GLSL_TYPE_FLOAT:
> +      case GLSL_TYPE_FLOAT16:
>        case GLSL_TYPE_DOUBLE:
>        case GLSL_TYPE_BOOL:
>        case GLSL_TYPE_ARRAY: {
> @@ -523,9 +529,12 @@ vtn_pointer_to_offset(struct vtn_builder *b, struct
> vtn_pointer *ptr,
>        switch (base_type) {
>        case GLSL_TYPE_UINT:
>        case GLSL_TYPE_INT:
> +      case GLSL_TYPE_UINT16:
> +      case GLSL_TYPE_INT16:
>        case GLSL_TYPE_UINT64:
>        case GLSL_TYPE_INT64:
>        case GLSL_TYPE_FLOAT:
> +      case GLSL_TYPE_FLOAT16:
>        case GLSL_TYPE_DOUBLE:
>        case GLSL_TYPE_BOOL:
>        case GLSL_TYPE_ARRAY:
> @@ -567,9 +576,12 @@ vtn_type_block_size(struct vtn_type *type)
>     switch (base_type) {
>     case GLSL_TYPE_UINT:
>     case GLSL_TYPE_INT:
> +   case GLSL_TYPE_UINT16:
> +   case GLSL_TYPE_INT16:
>     case GLSL_TYPE_UINT64:
>     case GLSL_TYPE_INT64:
>     case GLSL_TYPE_FLOAT:
> +   case GLSL_TYPE_FLOAT16:
>     case GLSL_TYPE_BOOL:
>     case GLSL_TYPE_DOUBLE: {
>        unsigned cols = type->row_major ? glsl_get_vector_elements(type->type)
> :
> @@ -698,9 +710,12 @@ _vtn_block_load_store(struct vtn_builder *b,
> nir_intrinsic_op op, bool load,
>     switch (base_type) {
>     case GLSL_TYPE_UINT:
>     case GLSL_TYPE_INT:
> +   case GLSL_TYPE_UINT16:
> +   case GLSL_TYPE_INT16:
>     case GLSL_TYPE_UINT64:
>     case GLSL_TYPE_INT64:
>     case GLSL_TYPE_FLOAT:
> +   case GLSL_TYPE_FLOAT16:
>     case GLSL_TYPE_DOUBLE:
>     case GLSL_TYPE_BOOL:
>        /* This is where things get interesting.  At this point, we've hit
> @@ -873,9 +888,12 @@ _vtn_variable_load_store(struct vtn_builder *b, bool
> load,
>     switch (base_type) {
>     case GLSL_TYPE_UINT:
>     case GLSL_TYPE_INT:
> +   case GLSL_TYPE_UINT16:
> +   case GLSL_TYPE_INT16:
>     case GLSL_TYPE_UINT64:
>     case GLSL_TYPE_INT64:
>     case GLSL_TYPE_FLOAT:
> +   case GLSL_TYPE_FLOAT16:
>     case GLSL_TYPE_BOOL:
>     case GLSL_TYPE_DOUBLE:
>        /* At this point, we have a scalar, vector, or matrix so we know
> that
> @@ -953,9 +971,12 @@ _vtn_variable_copy(struct vtn_builder *b, struct
> vtn_pointer *dest,
>     switch (base_type) {
>     case GLSL_TYPE_UINT:
>     case GLSL_TYPE_INT:
> +   case GLSL_TYPE_UINT16:
> +   case GLSL_TYPE_INT16:
>     case GLSL_TYPE_UINT64:
>     case GLSL_TYPE_INT64:
>     case GLSL_TYPE_FLOAT:
> +   case GLSL_TYPE_FLOAT16:
>     case GLSL_TYPE_DOUBLE:
>     case GLSL_TYPE_BOOL:
>        /* At this point, we have a scalar, vector, or matrix so we know
> that
> --
> 2.14.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171130/dfb8c4de/attachment-0001.html>


More information about the mesa-dev mailing list