[Mesa-dev] [PATCH 6/6] nir: Rework conversion opcodes

[Jason Ekstrand]

On Wed, Mar 8, 2017 at 4:09 PM, Eric Anholt <eric at anholt.net> wrote:

> Jason Ekstrand <jason at jlekstrand.net> writes:
>
> > The NIR story on conversion opcodes is a mess.  We've had way too many
> > of them, naming is inconsistent, and which ones have explicit sizes was
> > sort-of random.  This commit re-organizes things and makes them all
> > consistent:
> >
> >  - All non-bool conversion opcodes now have the explicit size in the
> >    destination and are named <src_type>2<dst_type><size>.
> >
> >  - Integer <-> integer conversion opcodes now only come in i2i and u2u
> >    forms (i2u and u2i have been removed) since the only difference
> >    between the different integer conversions is whether or not they
> >    sign-extend when up-converting.
> >
> >  - Boolean conversion opcodes all have the explicit size on the bool and
> >    are named <src_type>2<dst_type>.
> >
> > Making things consistent also allows nir_type_conversion_op to be moved
> > to nir_opcodes.c and auto-generated using mako.  This will make adding
> > int8, int16, and float16 versions much easier when the time comes.
> > ---
>
> > diff --git a/src/compiler/nir/nir_opcodes.py b/src/compiler/nir/nir_
> opcodes.py
> > index b116fcf..f7d7d3b 100644
> > --- a/src/compiler/nir/nir_opcodes.py
> > +++ b/src/compiler/nir/nir_opcodes.py
> > @@ -166,42 +166,26 @@ unop("frsq", tfloat, "bit_size == 64 ? 1.0 /
> sqrt(src0) : 1.0f / sqrtf(src0)")
> >  unop("fsqrt", tfloat, "bit_size == 64 ? sqrt(src0) : sqrtf(src0)")
> >  unop("fexp2", tfloat, "exp2f(src0)")
> >  unop("flog2", tfloat, "log2f(src0)")
> > -unop_convert("f2i", tint32, tfloat32, "src0") # Float-to-integer
> conversion.
> > -unop_convert("f2u", tuint32, tfloat32, "src0") # Float-to-unsigned
> conversion
> > -unop_convert("d2i", tint32, tfloat64, "src0") # Double-to-integer
> conversion.
> > -unop_convert("d2u", tuint32, tfloat64, "src0") # Double-to-unsigned
> conversion.
> > -unop_convert("i2f", tfloat32, tint32, "src0") # Integer-to-float
> conversion.
> > -unop_convert("i2d", tfloat64, tint32, "src0") # Integer-to-double
> conversion.
> > -unop_convert("i2i32", tint32, tint, "src0")    # General int (int8_t,
> int64_t, etc.) to int32_t conversion
> > -unop_convert("u2i32", tint32, tuint, "src0")   # General uint (uint8_t,
> uint64_t, etc.) to int32_t conversion
> > -unop_convert("i2u32", tuint32, tint, "src0")   # General int (int8_t,
> int64_t, etc.) to uint32_t conversion
> > -unop_convert("u2u32", tuint32, tuint, "src0")  # General uint (uint8_t,
> uint64_t, etc.) to uint32_t conversion
> > -unop_convert("i2i64", tint64, tint, "src0")    # General int (int8_t,
> int32_t, etc.) to int64_t conversion
> > -unop_convert("u2i64", tint64, tuint, "src0")   # General uint (uint8_t,
> uint64_t, etc.) to int64_t conversion
> > -unop_convert("f2i64", tint64, tfloat, "src0")  # General float (float
> or double) to int64_t conversion
> > -unop_convert("i2u64", tuint64, tint,  "src0")  # General int (int8_t,
> int64_t, etc.) to uint64_t conversion
> > -unop_convert("u2u64", tuint64, tuint, "src0")  # General uint (uint8_t,
> uint32_t, etc.) to uint64_t conversion
> > -unop_convert("f2u64", tuint64, tfloat, "src0") # General float (float
> or double) to uint64_t conversion
> > -unop_convert("i642f", tfloat32, tint64, "src0")  # int64_t-to-float
> conversion.
> > -unop_convert("i642b", tbool, tint64, "src0")  # int64_t-to-bool
> conversion.
> > -unop_convert("i642d", tfloat64, tint64, "src0")  # int64_t-to-double
> conversion.
> > -unop_convert("u642f", tfloat32, tuint64, "src0") # uint64_t-to-float
> conversion.
> > -unop_convert("u642d", tfloat64, tuint64, "src0") # uint64_t-to-double
> conversion.
> > -
> > -# Float-to-boolean conversion
> > -unop_convert("f2b", tbool, tfloat32, "src0 != 0.0f")
> > -unop_convert("d2b", tbool, tfloat64, "src0 != 0.0")
> > -# Boolean-to-float conversion
> > -unop_convert("b2f", tfloat32, tbool, "src0 ? 1.0f : 0.0f")
> > -# Int-to-boolean conversion
> > +
> > +# Generate all of the numeric conversion opcodes
> > +for src_t in [tint, tuint, tfloat, tbool]:
> > +   if src_t in (tint, tuint):
> > +      dst_types = [tfloat, src_t]
> > +   elif src_t == tfloat:
> > +      dst_types = [tint, tuint, tfloat]
> > +
> > +   for dst_t in dst_types:
> > +      for bit_size in [32, 64]:
> > +         unop_convert("{}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.
> > +unop_convert("f2b", tbool, tfloat, "src0 != 0.0")
> >  unop_convert("i2b", tbool, tint, "src0 != 0")
> > -unop_convert("b2i", tint32, tbool, "src0 ? 1 : 0") # Boolean-to-int
> conversion
> > -unop_convert("b2i64", tint64, tbool, "src0 ? 1 : 0")  #
> Boolean-to-int64_t conversion.
> > -unop_convert("u2f", tfloat32, tuint32, "src0") # Unsigned-to-float
> conversion.
> > -unop_convert("u2d", tfloat64, tuint32, "src0") # Unsigned-to-double
> conversion.
> > -# double-to-float conversion
> > -unop_convert("d2f", tfloat32, tfloat64, "src0") # Double to single
> precision
> > -unop_convert("f2d", tfloat64, tfloat32, "src0") # Single to double
> precision
> > +unop_convert("b2f", tfloat, tbool, "src0 ? 1.0 : 0.0")
> > +unop_convert("b2i", tint, tbool, "src0 ? 1 : 0")
>
> I'm confused.  The loop includes "tbool" in src_t iteration, so that
> makes me think it would be generating a unop_convert() for, say, b2i64
> (despite the commit message saying bool conversions don't get sizes),
> but then there are these hand-written unop_converts for bools here and I
> don't see b2i64 in any of the code.
>

Oops...  You're right, we are creating bogus opcodes.  I must have failed
at refactoring at some point.  Fixed locally.


> Other than this question, patches 1-3 and 6 are:
>
> Reviewed-by: Eric Anholt <eric at anholt.net>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170308/a78d472b/attachment-0001.html>