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

2017-12-01 Thread Chema Casanova 
On 30/11/17 21:24, Jason Ekstrand wrote:
> I sprinkled a few mostly trivial comments below.  With those fixed,
> 
> Reviewed-by: Jason Ekstrand  >
> 
> On Wed, Nov 29, 2017 at 6:07 PM, Jose Maria Casanova Crespo
> > wrote:
> 
> From: Eduardo Lima Mitev >
> 
> 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  >
> Signed-off-by: Eduardo Lima >
> ---
>  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()

Fixed locally.

> +      }
>        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.

Let it be:

 val->constant->values->u64[0] = vtn_u64_literal([3]);


> -      } else {
> -         assert(bit_size == 32);
> +         break;
> +      }
> 
> 
> You don't need braces around the 64-bit case.

Ok.

> +      case 32:
>           val->constant->values->u32[0] = w[3];
> +         break;
> +      case 16:
> +         

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

2017-11-30 Thread Jason Ekstrand 
I sprinkled a few mostly trivial comments below.  With those fixed,

Reviewed-by: Jason Ekstrand 

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

> From: Eduardo Lima Mitev 
>
> 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 
> Signed-off-by: Eduardo Lima 
> ---
>  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([3]));
> -  else
> + break;
> +  }
>

Same here.


> +  case 32:
>   val->constant->values[0].u32[0] = get_specialization(b, val,
> w[3]);

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

2017-11-29 Thread Jose Maria Casanova Crespo 
From: Eduardo Lima Mitev 

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 
Signed-off-by: Eduardo Lima 
---
 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");
+  }
   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];
-  } else {
- assert(bit_size == 32);
+ break;
+  }
+  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([3]));
-  else
+ break;
+  }
+  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: {