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

Chema Casanova jmcasanova at igalia.com
Thu Nov 2 14:47:24 UTC 2017



El 30/10/17 a las 22:26, Jason Ekstrand escribió:
> On Thu, Oct 12, 2017 at 11:37 AM, Jose Maria Casanova Crespo
> <jmcasanova at igalia.com <mailto:jmcasanova at igalia.com>> wrote:
>
>     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.
>
>     v2: Indentation fixed (Jason Ekstrand)
>     ---
>      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 9f25e30678..5738979b19 100644
>     --- a/src/compiler/glsl/glsl_to_nir.cpp
>     +++ b/src/compiler/glsl/glsl_to_nir.cpp
>     @@ -1575,7 +1575,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 fb269fcb28..93f0d52804 100644
>     --- a/src/compiler/nir/nir.h
>     +++ b/src/compiler/nir/nir.h
>     @@ -753,7 +753,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 06ae820c3e..0abc34f037 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")
>
>      # 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 02bb4738ed..95a76ea39f 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 [16, 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:
>     +                           return
>     ${'nir_op_{0}2{1}{2}'.format(src_t[0], dst_t[0],
>     +                                                             
>      dst_bits)};
>
>
> I commented on an earlier version of this series that having a default
> here makes me nervous.  Someone will pass a rounding mode in for a
> double->float conversion or pass in a rounding mode of
> nir_rounding_mode_ru and we'll happily ignore it.  I'd be more
> comfortable if _undefined had an explicit case and the default was
> unreachable()

Ups, I've forgot to address this one. I'm sending an v3.

>  
>
>     +                     }
>     +%                    else:
>                           return
>     ${'nir_op_{0}2{1}{2}'.format(src_t[0], dst_t[0], dst_bits)};
>
>
> and an assert(rnd == nir_rounding_mode_undefined) here

Makes sense.

>  
>
>     +%                    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 ecf9cbc34d..7ec30b8a63 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.13.6
>
>     _______________________________________________
>     mesa-dev mailing list
>     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>     <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
>
>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev



More information about the mesa-dev mailing list