[Mesa-dev] [PATCH 06/45] nir: Handle fp16 rounding modes at nir_type_conversion_op
Jason Ekstrand
jason at jlekstrand.net
Fri Aug 18 18:02:24 UTC 2017
On Fri, Aug 18, 2017 at 4:31 AM, Eduardo Lima Mitev <elima at igalia.com>
wrote:
> On 08/17/2017 09:08 PM, Jason Ekstrand wrote:
> > On Thu, Jul 13, 2017 at 7:35 AM, Alejandro Piñeiro <apinheiro at igalia.com
> > <mailto:apinheiro at igalia.com>> wrote:
> >
> > From: Jose Maria Casanova Crespo <jmcasanova at igalia.com
> > <mailto: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.
> >
>
> Hmm, I don't think we can reduce it that way. Notice that the loop over
> rnd_modes is only for bit_size==16, but the last unop_convert call,
> outside the loop, applies to all bit sizes.
>
Right. Sorry for the noise.
> >
> >
> > # 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.
> >
>
> Ok, we'll fix that.
>
> >
> > + 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.
> >
>
> Ok, note taken.
>
> >
> > 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 <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
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170818/ae317710/attachment-0001.html>
More information about the mesa-dev
mailing list