<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Aug 18, 2017 at 4:31 AM, Eduardo Lima Mitev <<a href="mailto:elima@igalia.com" target="_blank">elima@igalia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 08/17/2017 09:08 PM, Jason Ekstrand wrote: > On Thu, Jul 13, 2017 at 7:35 AM, Alejandro Piñeiro <<a href="mailto:apinheiro@igalia.com">apinheiro@igalia.com</a> > <mailto:<a href="mailto:apinheiro@igalia.com">apinheiro@igalia.com</a>>> wrote: > > From: Jose Maria Casanova Crespo <<a href="mailto:jmcasanova@igalia.com">jmcasanova@igalia.com</a> > <mailto:<a href="mailto:jmcasanova@igalia.com">jmcasanova@igalia.com</a>>> <div><div class="h5">> > 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. > </div></div>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. <div><div class="h5"></div></div></blockquote><div> </div><div>Right. Sorry for the noise. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> > > > # 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. > </div></div>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 > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <mailto:<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a>> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > <<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>> <div class="HOEnZb"><div class="h5">> > > > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > </div></div></blockquote></div> </div></div>