[Mesa-dev] [PATCH 06/45] nir: Handle fp16 rounding modes at nir_type_conversion_op

Jason Ekstrand jason at jlekstrand.net
Thu Aug 17 19:08:10 UTC 2017


On Thu, Jul 13, 2017 at 7:35 AM, Alejandro PiƱeiro <apinheiro at igalia.com>
wrote:

> From: Jose Maria Casanova Crespo <jmcasanova at igalia.com>
>
> nir_type_conversion enables new operations to handle rounding modes to
> convert to fp16 values. Two new opcodes are enabled nir_op_f2f16_rtne
> and nir_op_f2f16_rtz.
>
> The undefined behaviour doesn't has any effect and uses the original
> nir_op_f2f16 operation.
> ---
>  src/compiler/glsl/glsl_to_nir.cpp |  3 ++-
>  src/compiler/nir/nir.h            |  3 ++-
>  src/compiler/nir/nir_opcodes.py   | 10 ++++++++--
>  src/compiler/nir/nir_opcodes_c.py | 15 ++++++++++++++-
>  src/compiler/spirv/vtn_alu.c      |  2 +-
>  5 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/src/compiler/glsl/glsl_to_nir.cpp
> b/src/compiler/glsl/glsl_to_nir.cpp
> index 2153004..23e6121 100644
> --- a/src/compiler/glsl/glsl_to_nir.cpp
> +++ b/src/compiler/glsl/glsl_to_nir.cpp
> @@ -1509,7 +1509,8 @@ nir_visitor::visit(ir_expression *ir)
>     case ir_unop_u642i64: {
>        nir_alu_type src_type = nir_get_nir_type_for_glsl_
> base_type(types[0]);
>        nir_alu_type dst_type = nir_get_nir_type_for_glsl_
> base_type(out_type);
> -      result = nir_build_alu(&b, nir_type_conversion_op(src_type,
> dst_type),
> +      result = nir_build_alu(&b, nir_type_conversion_op(src_type,
> dst_type,
> +                                 nir_rounding_mode_undef),
>                                   srcs[0], NULL, NULL, NULL);
>        /* b2i and b2f don't have fixed bit-size versions so the builder
> will
>         * just assume 32 and we have to fix it up here.
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index 115ec1b..7e48e18 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -741,7 +741,8 @@ nir_get_nir_type_for_glsl_type(const struct glsl_type
> *type)
>     return nir_get_nir_type_for_glsl_base_type(glsl_get_base_type(type));
>  }
>
> -nir_op nir_type_conversion_op(nir_alu_type src, nir_alu_type dst);
> +nir_op nir_type_conversion_op(nir_alu_type src, nir_alu_type dst,
> +                              nir_rounding_mode rnd);
>
>  typedef enum {
>     NIR_OP_IS_COMMUTATIVE = (1 << 0),
> diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_
> opcodes.py
> index 39c01a7..bd342de 100644
> --- a/src/compiler/nir/nir_opcodes.py
> +++ b/src/compiler/nir/nir_opcodes.py
> @@ -179,8 +179,14 @@ for src_t in [tint, tuint, tfloat]:
>        else:
>           bit_sizes = [8, 16, 32, 64]
>        for bit_size in bit_sizes:
> -         unop_convert("{0}2{1}{2}".format(src_t[0], dst_t[0], bit_size),
> -                      dst_t + str(bit_size), src_t, "src0")
> +          if bit_size == 16 and dst_t == tfloat and src_t == tfloat:
> +              rnd_modes = ['rtne', 'rtz']
> +              for rnd_mode in rnd_modes:
> +                  unop_convert("{0}2{1}{2}_{3}".format(src_t[0],
> dst_t[0],
> +                                                       bit_size,
> rnd_mode),
> +                               dst_t + str(bit_size), src_t, "src0")
> +          unop_convert("{0}2{1}{2}".format(src_t[0], dst_t[0], bit_size),
> +                       dst_t + str(bit_size), src_t, "src0")
>

You could do this a bit shorter if you make rnd_modes = ['', '_rtne',
'rtz'] and then just loop over them.


>
>  # We'll hand-code the to/from bool conversion opcodes.  Because bool
> doesn't
>  # have multiple bit-sizes, we can always infer the size from the other
> type.
> diff --git a/src/compiler/nir/nir_opcodes_c.py b/src/compiler/nir/nir_
> opcodes_c.py
> index a1db54f..a7721d3 100644
> --- a/src/compiler/nir/nir_opcodes_c.py
> +++ b/src/compiler/nir/nir_opcodes_c.py
> @@ -30,7 +30,7 @@ template = Template("""
>  #include "nir.h"
>
>  nir_op
> -nir_type_conversion_op(nir_alu_type src, nir_alu_type dst)
> +nir_type_conversion_op(nir_alu_type src, nir_alu_type dst,
> nir_rounding_mode rnd)
>  {
>     nir_alu_type src_base = (nir_alu_type) nir_alu_type_get_base_type(
> src);
>     nir_alu_type dst_base = (nir_alu_type) nir_alu_type_get_base_type(
> dst);
> @@ -64,7 +64,20 @@ nir_type_conversion_op(nir_alu_type src, nir_alu_type
> dst)
>                 switch (dst_bit_size) {
>  %                 for dst_bits in [32, 64]:
>                    case ${dst_bits}:
> +%                   if src_t == 'float' and dst_t == 'float'  and
> dst_bits == 16:
> +                     switch(rnd) {
> +%                      for rnd_t in ['rtne' , 'rtz']:
> +                       case nir_rounding_mode_${rnd_t}:
> +                            return ${'nir_op_{0}2{1}{2}_{3}'.format(src_t[0],
> dst_t[0],
> +
> dst_bits, rnd_t)};
> +%                     endfor
> +                      default:
>

This default makes me a bit uncomfortable.  I think I'd rather assert-fail
if it's a rounding mode we don't have an opcode for.  Otherwise, this
function is just silently ignoring rounding modes which seems bad.


> +                            return ${'nir_op_{0}2{1}{2}'.format(src_t[0],
> dst_t[0],
> +
> dst_bits)};
> +                       }
> +%                   else:
>

The indentation on the above is a bit weird.  I'm not really sure what to
do with it.  What I would say is that you shouldn't worry too much about
making the resulting C nicely indented.  Just make the generator readable.


>                       return ${'nir_op_{0}2{1}{2}'.format(src_t[0],
> dst_t[0], dst_bits)};
> +%                   endif
>  %                 endfor
>                    default:
>                       unreachable("Invalid nir alu bit size");
> diff --git a/src/compiler/spirv/vtn_alu.c b/src/compiler/spirv/vtn_alu.c
> index ecf9cbc..7ec30b8 100644
> --- a/src/compiler/spirv/vtn_alu.c
> +++ b/src/compiler/spirv/vtn_alu.c
> @@ -355,7 +355,7 @@ vtn_nir_alu_op_for_spirv_opcode(SpvOp opcode, bool
> *swap,
>     case SpvOpConvertUToF:
>     case SpvOpSConvert:
>     case SpvOpFConvert:
> -      return nir_type_conversion_op(src, dst);
> +      return nir_type_conversion_op(src, dst, nir_rounding_mode_undef);
>
>     /* Derivatives: */
>     case SpvOpDPdx:         return nir_op_fddx;
> --
> 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/20170817/7fbbc13d/attachment.html>


More information about the mesa-dev mailing list