On Fri, Aug 18, 2017 at 4:31 AM, Eduardo Lima Mitev <el...@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 <apinhe...@igalia.com > > <mailto:apinhe...@igalia.com>> wrote: > > > > From: Jose Maria Casanova Crespo <jmcasan...@igalia.com > > <mailto:jmcasan...@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@lists.freedesktop.org <mailto:mesa-dev@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@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
