[Mesa-dev] [PATCH v2 14/25] nir: add nir_type_conversion_op()

[Jason Ekstrand]

I've left some inline comments but this patch has gotten me thinking about
the larger question of how we want to do conversion operations in NIR.  Ian
already has patches for int64 but float16, int8, and int16 are all coming.
If we keep coming up with one-letter prefixes (long, half, short, char?)
and doing a2b for every opcode.  We could, but that would be very
confusing.  It would be good to cut down on the combinatorics a bit.

Here's my straw-man proposal:  For each conversion, we leave the
destination as variable bit width and keep the source fixed.  Then we have
u2f32, u2f64, u2f16, etc. and we have one opcode regardless of destination
bit width.  This doesn't completely break the combinatorial explosion but
it does let us cut it by a factor of 3 or 4.

Another thing that Connor and I discussed was to let conversion operations
be the one exception to the "source and destination bit-widths must match
for variable bit-width operations" and add i2i and f2f opcodes.  That would
drop the number of opcodes even further but I'm not sure how bad the
special-casing would end up being.

Thoughts?

--Jason Ekstrand

On Fri, Dec 16, 2016 at 6:49 AM, Juan A. Suarez Romero <jasuarez at igalia.com>
wrote:

> From: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
>
> This function returns the nir_op corresponding to the conversion between
> the given nir_alu_type arguments.
>
> This function lacks support for integer-based types with bit_size != 32
> and for float16 conversion ops.
>
> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> ---
>  src/compiler/nir/nir.c | 83 ++++++++++++++++++++++++++++++
> ++++++++++++++++++++
>  src/compiler/nir/nir.h |  2 ++
>  2 files changed, 85 insertions(+)
>
> diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
> index 2d882f7..3e00452 100644
> --- a/src/compiler/nir/nir.c
> +++ b/src/compiler/nir/nir.c
> @@ -1953,3 +1953,86 @@ nir_system_value_from_intrinsic(nir_intrinsic_op
> intrin)
>        unreachable("intrinsic doesn't produce a system value");
>     }
>  }
> +
> +nir_op
> +nir_type_conversion_op(nir_alu_type src, nir_alu_type dst)
> +{
> +   nir_alu_type src_base_type = (nir_alu_type) nir_alu_type_get_base_type(
> src);
> +   nir_alu_type dst_base_type = (nir_alu_type) nir_alu_type_get_base_type(
> dst);
> +   unsigned src_bitsize = nir_alu_type_get_type_size(src);
> +   unsigned dst_bitsize = nir_alu_type_get_type_size(dst);
> +
> +   if (src_base_type == dst_base_type) {
> +      if (src_bitsize == dst_bitsize)
> +         return (src_base_type == nir_type_float) ? nir_op_fmov :
> nir_op_imov;
> +
> +      switch (src_base_type) {
> +      case nir_type_bool:
> +      case nir_type_uint:
> +      case nir_type_int:
> +         return nir_op_imov;
>

These three cases can't happen right now.  We should just assert that it's
float.


> +      case nir_type_float:
> +         /* TODO: implement support for float16 */
> +         assert(src_bitsize == 64 || dst_bitsize == 64);
> +         return (src_bitsize == 64) ? nir_op_d2f : nir_op_f2d;
> +      default:
> +         assert(!"Invalid conversion");
> +      };
> +   }
> +
> +   /* Different base type but same bit_size */
> +   if (src_bitsize == dst_bitsize) {
> +      /* TODO: This does not include specific conversions between
> +       * signed or unsigned integer types of bit size different of 32 yet.
> +       */
> +      assert(src_bitsize == 32);
> +      switch (src_base_type) {
> +      case nir_type_uint:
> +         return (dst_base_type == nir_type_float) ? nir_op_u2f :
> nir_op_imov;
> +      case nir_type_int:
> +         return (dst_base_type == nir_type_float) ? nir_op_i2f :
> nir_op_imov;
> +      case nir_type_bool:
> +         return (dst_base_type == nir_type_float) ? nir_op_b2f :
> nir_op_imov;
>

There's also a b2i opcode we should handle...


> +      case nir_type_float:
> +         return (dst_base_type == nir_type_uint) ? nir_op_f2u :
> +            (dst_base_type == nir_type_bool) ? nir_op_f2b : nir_op_f2i;
>

Switch?


> +      default:
> +         assert(!"Invalid conversion");
> +      };
> +   }
> +
> +   /* Different bit_size and different base type */
> +   /* TODO: Implement integer support for types with bit_size != 32 */
> +   switch (src_base_type) {
> +   case nir_type_uint:
> +      assert(dst == nir_type_float64);
> +      return nir_op_u2d;
> +   case nir_type_int:
> +      assert(dst == nir_type_float64);
> +      return nir_op_i2d;
> +   case nir_type_bool:
> +      assert(dst == nir_type_float64);
> +      return nir_op_u2d;
> +   case nir_type_float:
> +      assert(src_bitsize == 32 || src_bitsize == 64);
> +      if (src_bitsize != 64) {
> +         assert(dst == nir_type_float64);
> +         return nir_op_f2d;
> +      }
> +      assert(dst_bitsize == 32);
> +      switch (dst_base_type) {
> +      case nir_type_uint:
> +         return nir_op_d2u;
> +      case nir_type_int:
> +         return nir_op_d2i;
> +      case nir_type_bool:
> +         return nir_op_d2b;
> +      case nir_type_float:
> +         return nir_op_d2f;
> +      default:
> +         assert(!"Invalid conversion");
> +      };
> +   default:
> +      assert(!"Invalid conversion");
> +   };
> +}
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index 9310dab..9f3abb7 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -689,6 +689,8 @@ nir_get_nir_type_for_glsl_type(const struct glsl_type
> *type)
>     unreachable("unknown type");
>  }
>
> +nir_op nir_type_conversion_op(nir_alu_type src, nir_alu_type dst);
> +
>  typedef enum {
>     NIR_OP_IS_COMMUTATIVE = (1 << 0),
>     NIR_OP_IS_ASSOCIATIVE = (1 << 1),
> --
> 2.9.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/20170102/a56126cc/attachment-0001.html>