Re: [Mesa-dev] [PATCH 12/22] spirv/nir: implement DF conversions

[Samuel Iglesias Gonsálvez]

On Thu, 2016-12-01 at 18:50 -0800, Jason Ekstrand wrote:
> On Fri, Nov 25, 2016 at 12:52 AM, Juan A. Suarez Romero  lia.com> wrote:
> > From: Samuel Iglesias Gonsálvez 
> > 
> > SPIR-V does not have special opcodes for DF conversions. We need to
> > identify
> > them by checking the bit size of the operand and the result.
> > 
> > Signed-off-by: Samuel Iglesias Gonsálvez 
> > ---
> >  src/compiler/spirv/spirv_to_nir.c | 29 ++-
> > --
> >  src/compiler/spirv/vtn_alu.c      | 37
> > +++--
> >  src/compiler/spirv/vtn_private.h  |  3 ++-
> >  3 files changed, 51 insertions(+), 18 deletions(-)
> > 
> > diff --git a/src/compiler/spirv/spirv_to_nir.c
> > b/src/compiler/spirv/spirv_to_nir.c
> > index a13f72a..81c73da 100644
> > --- a/src/compiler/spirv/spirv_to_nir.c
> > +++ b/src/compiler/spirv/spirv_to_nir.c
> > @@ -1211,12 +1211,21 @@ vtn_handle_constant(struct vtn_builder *b,
> > SpvOp opcode,
> > 
> >        default: {
> >           bool swap;
> > -         nir_op op = vtn_nir_alu_op_for_spirv_opcode(opcode,
> > );
> > -
> > -         unsigned num_components = glsl_get_vector_elements(val-
> > >const_type);
> >           unsigned bit_size =
> >              glsl_get_bit_size(val->const_type);
> > 
> > +         bool is_double_dst = bit_size == 64;
> > +         bool is_double_src = is_double_dst;
> > +         /* We assume there is no double conversion here */
> > +         assert(bit_size != 64 ||
> > +                (opcode != SpvOpConvertFToU && opcode !=
> > SpvOpConvertFToS &&
> > +                 opcode != SpvOpConvertSToF && opcode !=
> > SpvOpConvertUToF &&
> > +                 opcode != SpvOpFConvert));
> > +         nir_op op =
> > +            vtn_nir_alu_op_for_spirv_opcode(opcode, ,
> > +                                            is_double_dst,
> > is_double_src);
> > +
> > +         unsigned num_components = glsl_get_vector_elements(val-
> > >const_type);
> >           nir_const_value src[4];
> >           assert(count <= 7);
> >           for (unsigned i = 0; i < count - 4; i++) {
> > @@ -1224,16 +1233,22 @@ vtn_handle_constant(struct vtn_builder *b,
> > SpvOp opcode,
> >                 vtn_value(b, w[4 + i], vtn_value_type_constant)-
> > >constant;
> > 
> >              unsigned j = swap ? 1 - i : i;
> > -            assert(bit_size == 32);
> >              for (unsigned k = 0; k < num_components; k++)
> > -               src[j].u32[k] = c->value.u[k];
> > +               if (!is_double_src)
> > +                  src[j].u32[k] = c->value.u[k];
> > +               else
> > +                  src[j].f64[k] = c->value.d[k];
> >           }
> > 
> >           nir_const_value res = nir_eval_const_opcode(op,
> > num_components,
> >                                                       bit_size,
> > src);
> > 
> > -         for (unsigned k = 0; k < num_components; k++)
> > -            val->constant->value.u[k] = res.u32[k];
> > +         for (unsigned k = 0; k < num_components; k++) {
> > +            if (!is_double_dst)
> > +               val->constant->value.u[k] = res.u32[k];
> > +            else
> > +               val->constant->value.d[k] = res.f64[k];
> > +         }
> > 
> >           break;
> >        } /* default */
> > diff --git a/src/compiler/spirv/vtn_alu.c
> > b/src/compiler/spirv/vtn_alu.c
> > index 95ff2b1..e444d3f 100644
> > --- a/src/compiler/spirv/vtn_alu.c
> > +++ b/src/compiler/spirv/vtn_alu.c
> > @@ -211,7 +211,8 @@ vtn_handle_matrix_alu(struct vtn_builder *b,
> > SpvOp opcode,
> >  }
> > 
> >  nir_op
> > -vtn_nir_alu_op_for_spirv_opcode(SpvOp opcode, bool *swap)
> > +vtn_nir_alu_op_for_spirv_opcode(SpvOp opcode, bool *swap,
> > +                                bool is_double_dst, bool
> > is_double_src)
> 
> I think it would be better if we did this as dst_bit_size and
> src_bit_size.  That would make this simpler for basically every
> caller.  Also, it makes it more 8/16-bit ready.
>  

OK.

> >  {
> >     /* Indicates that the first two arguments should be swapped. 
> > This is
> >      * used for implementing greater-than and less-than-or-equal.
> > @@ -284,16 +285,21 @@ vtn_nir_alu_op_for_spirv_opcode(SpvOp opcode,
> > bool *swap)
> >     case SpvOpFUnordGreaterThanEqual:               return
> > nir_op_fge;
> > 
> >     /* Conversions: */
> > -   case SpvOpConvertFToU:           return nir_op_f2u;
> > -   case SpvOpConvertFToS:           return nir_op_f2i;
> > -   case SpvOpConvertSToF:           return nir_op_i2f;
> > -   case SpvOpConvertUToF:           return nir_op_u2f;
> > +   case SpvOpConvertFToU:           return is_double_src ?
> > nir_op_d2u : nir_op_f2u;
> > +   case SpvOpConvertFToS:           return is_double_src ?
> > nir_op_d2i : nir_op_f2i;
> > +   case SpvOpConvertSToF:           return is_double_dst ?
> > nir_op_i2d : nir_op_i2f;
> > +   case SpvOpConvertUToF:           return is_double_dst ?
> > nir_op_u2d : nir_op_u2f;
> 
> The 

Re: [Mesa-dev] [PATCH 12/22] spirv/nir: implement DF conversions

[Jason Ekstrand]

On Fri, Nov 25, 2016 at 12:52 AM, Juan A. Suarez Romero  wrote:

> From: Samuel Iglesias Gonsálvez 
>
> SPIR-V does not have special opcodes for DF conversions. We need to
> identify
> them by checking the bit size of the operand and the result.
>
> Signed-off-by: Samuel Iglesias Gonsálvez 
> ---
>  src/compiler/spirv/spirv_to_nir.c | 29 ++---
>  src/compiler/spirv/vtn_alu.c  | 37 +++---
> ---
>  src/compiler/spirv/vtn_private.h  |  3 ++-
>  3 files changed, 51 insertions(+), 18 deletions(-)
>
> diff --git a/src/compiler/spirv/spirv_to_nir.c
> b/src/compiler/spirv/spirv_to_nir.c
> index a13f72a..81c73da 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -1211,12 +1211,21 @@ vtn_handle_constant(struct vtn_builder *b, SpvOp
> opcode,
>
>default: {
>   bool swap;
> - nir_op op = vtn_nir_alu_op_for_spirv_opcode(opcode, );
> -
> - unsigned num_components = glsl_get_vector_elements(val->
> const_type);
>   unsigned bit_size =
>  glsl_get_bit_size(val->const_type);
>
> + bool is_double_dst = bit_size == 64;
> + bool is_double_src = is_double_dst;
> + /* We assume there is no double conversion here */
> + assert(bit_size != 64 ||
> +(opcode != SpvOpConvertFToU && opcode != SpvOpConvertFToS
> &&
> + opcode != SpvOpConvertSToF && opcode != SpvOpConvertUToF
> &&
> + opcode != SpvOpFConvert));
> + nir_op op =
> +vtn_nir_alu_op_for_spirv_opcode(opcode, ,
> +is_double_dst, is_double_src);
> +
> + unsigned num_components = glsl_get_vector_elements(val->
> const_type);
>   nir_const_value src[4];
>   assert(count <= 7);
>   for (unsigned i = 0; i < count - 4; i++) {
> @@ -1224,16 +1233,22 @@ vtn_handle_constant(struct vtn_builder *b, SpvOp
> opcode,
> vtn_value(b, w[4 + i], vtn_value_type_constant)->constant;
>
>  unsigned j = swap ? 1 - i : i;
> -assert(bit_size == 32);
>  for (unsigned k = 0; k < num_components; k++)
> -   src[j].u32[k] = c->value.u[k];
> +   if (!is_double_src)
> +  src[j].u32[k] = c->value.u[k];
> +   else
> +  src[j].f64[k] = c->value.d[k];
>   }
>
>   nir_const_value res = nir_eval_const_opcode(op, num_components,
>   bit_size, src);
>
> - for (unsigned k = 0; k < num_components; k++)
> -val->constant->value.u[k] = res.u32[k];
> + for (unsigned k = 0; k < num_components; k++) {
> +if (!is_double_dst)
> +   val->constant->value.u[k] = res.u32[k];
> +else
> +   val->constant->value.d[k] = res.f64[k];
> + }
>
>   break;
>} /* default */
> diff --git a/src/compiler/spirv/vtn_alu.c b/src/compiler/spirv/vtn_alu.c
> index 95ff2b1..e444d3f 100644
> --- a/src/compiler/spirv/vtn_alu.c
> +++ b/src/compiler/spirv/vtn_alu.c
> @@ -211,7 +211,8 @@ vtn_handle_matrix_alu(struct vtn_builder *b, SpvOp
> opcode,
>  }
>
>  nir_op
> -vtn_nir_alu_op_for_spirv_opcode(SpvOp opcode, bool *swap)
> +vtn_nir_alu_op_for_spirv_opcode(SpvOp opcode, bool *swap,
> +bool is_double_dst, bool is_double_src)
>

I think it would be better if we did this as dst_bit_size and
src_bit_size.  That would make this simpler for basically every caller.
Also, it makes it more 8/16-bit ready.


>  {
> /* Indicates that the first two arguments should be swapped.  This is
>  * used for implementing greater-than and less-than-or-equal.
> @@ -284,16 +285,21 @@ vtn_nir_alu_op_for_spirv_opcode(SpvOp opcode, bool
> *swap)
> case SpvOpFUnordGreaterThanEqual:   return nir_op_fge;
>
> /* Conversions: */
> -   case SpvOpConvertFToU:   return nir_op_f2u;
> -   case SpvOpConvertFToS:   return nir_op_f2i;
> -   case SpvOpConvertSToF:   return nir_op_i2f;
> -   case SpvOpConvertUToF:   return nir_op_u2f;
> +   case SpvOpConvertFToU:   return is_double_src ? nir_op_d2u :
> nir_op_f2u;
> +   case SpvOpConvertFToS:   return is_double_src ? nir_op_d2i :
> nir_op_f2i;
> +   case SpvOpConvertSToF:   return is_double_dst ? nir_op_i2d :
> nir_op_i2f;
> +   case SpvOpConvertUToF:   return is_double_dst ? nir_op_u2d :
> nir_op_u2f;
>

The time is soon coming (not sure if you want to do this now or not) to add
a nir helper:

nir_op nir_type2type_op(nir_alu_type src, nir_alu_type dst);

I'm OK open-coding it for now, but as soon as we add int64 or any 8 or
16-bit types, we'll want it.


> case SpvOpBitcast:   return nir_op_imov;
> case SpvOpUConvert:
> case 

[Mesa-dev] [PATCH 12/22] spirv/nir: implement DF conversions

[Juan A. Suarez Romero]

From: Samuel Iglesias Gonsálvez 

SPIR-V does not have special opcodes for DF conversions. We need to identify
them by checking the bit size of the operand and the result.

Signed-off-by: Samuel Iglesias Gonsálvez 
---
 src/compiler/spirv/spirv_to_nir.c | 29 ++---
 src/compiler/spirv/vtn_alu.c  | 37 +++--
 src/compiler/spirv/vtn_private.h  |  3 ++-
 3 files changed, 51 insertions(+), 18 deletions(-)

diff --git a/src/compiler/spirv/spirv_to_nir.c 
b/src/compiler/spirv/spirv_to_nir.c
index a13f72a..81c73da 100644
--- a/src/compiler/spirv/spirv_to_nir.c
+++ b/src/compiler/spirv/spirv_to_nir.c
@@ -1211,12 +1211,21 @@ vtn_handle_constant(struct vtn_builder *b, SpvOp opcode,
 
   default: {
  bool swap;
- nir_op op = vtn_nir_alu_op_for_spirv_opcode(opcode, );
-
- unsigned num_components = glsl_get_vector_elements(val->const_type);
  unsigned bit_size =
 glsl_get_bit_size(val->const_type);
 
+ bool is_double_dst = bit_size == 64;
+ bool is_double_src = is_double_dst;
+ /* We assume there is no double conversion here */
+ assert(bit_size != 64 ||
+(opcode != SpvOpConvertFToU && opcode != SpvOpConvertFToS &&
+ opcode != SpvOpConvertSToF && opcode != SpvOpConvertUToF &&
+ opcode != SpvOpFConvert));
+ nir_op op =
+vtn_nir_alu_op_for_spirv_opcode(opcode, ,
+is_double_dst, is_double_src);
+
+ unsigned num_components = glsl_get_vector_elements(val->const_type);
  nir_const_value src[4];
  assert(count <= 7);
  for (unsigned i = 0; i < count - 4; i++) {
@@ -1224,16 +1233,22 @@ vtn_handle_constant(struct vtn_builder *b, SpvOp opcode,
vtn_value(b, w[4 + i], vtn_value_type_constant)->constant;
 
 unsigned j = swap ? 1 - i : i;
-assert(bit_size == 32);
 for (unsigned k = 0; k < num_components; k++)
-   src[j].u32[k] = c->value.u[k];
+   if (!is_double_src)
+  src[j].u32[k] = c->value.u[k];
+   else
+  src[j].f64[k] = c->value.d[k];
  }
 
  nir_const_value res = nir_eval_const_opcode(op, num_components,
  bit_size, src);
 
- for (unsigned k = 0; k < num_components; k++)
-val->constant->value.u[k] = res.u32[k];
+ for (unsigned k = 0; k < num_components; k++) {
+if (!is_double_dst)
+   val->constant->value.u[k] = res.u32[k];
+else
+   val->constant->value.d[k] = res.f64[k];
+ }
 
  break;
   } /* default */
diff --git a/src/compiler/spirv/vtn_alu.c b/src/compiler/spirv/vtn_alu.c
index 95ff2b1..e444d3f 100644
--- a/src/compiler/spirv/vtn_alu.c
+++ b/src/compiler/spirv/vtn_alu.c
@@ -211,7 +211,8 @@ vtn_handle_matrix_alu(struct vtn_builder *b, SpvOp opcode,
 }
 
 nir_op
-vtn_nir_alu_op_for_spirv_opcode(SpvOp opcode, bool *swap)
+vtn_nir_alu_op_for_spirv_opcode(SpvOp opcode, bool *swap,
+bool is_double_dst, bool is_double_src)
 {
/* Indicates that the first two arguments should be swapped.  This is
 * used for implementing greater-than and less-than-or-equal.
@@ -284,16 +285,21 @@ vtn_nir_alu_op_for_spirv_opcode(SpvOp opcode, bool *swap)
case SpvOpFUnordGreaterThanEqual:   return nir_op_fge;
 
/* Conversions: */
-   case SpvOpConvertFToU:   return nir_op_f2u;
-   case SpvOpConvertFToS:   return nir_op_f2i;
-   case SpvOpConvertSToF:   return nir_op_i2f;
-   case SpvOpConvertUToF:   return nir_op_u2f;
+   case SpvOpConvertFToU:   return is_double_src ? nir_op_d2u : 
nir_op_f2u;
+   case SpvOpConvertFToS:   return is_double_src ? nir_op_d2i : 
nir_op_f2i;
+   case SpvOpConvertSToF:   return is_double_dst ? nir_op_i2d : 
nir_op_i2f;
+   case SpvOpConvertUToF:   return is_double_dst ? nir_op_u2d : 
nir_op_u2f;
case SpvOpBitcast:   return nir_op_imov;
case SpvOpUConvert:
case SpvOpQuantizeToF16: return nir_op_fquantize2f16;
-   /* TODO: NIR is 32-bit only; these are no-ops. */
+   /* TODO: int64 is not supported yet. This is a no-op. */
case SpvOpSConvert:  return nir_op_imov;
-   case SpvOpFConvert:  return nir_op_fmov;
+   case SpvOpFConvert:
+  if (is_double_src && !is_double_dst)
+ return nir_op_d2f;
+  if (!is_double_src && is_double_dst)
+ return nir_op_f2d;
+  return nir_op_fmov;
 
/* Derivatives: */
case SpvOpDPdx: return nir_op_fddx;
@@ -457,7 +463,10 @@ vtn_handle_alu(struct vtn_builder *b, SpvOp opcode,
case SpvOpFUnordLessThanEqual:
case SpvOpFUnordGreaterThanEqual: {
   bool swap;
-