[Mesa-dev] [PATCH v2 14/25] nir: add nir_type_conversion_op()
Jason Ekstrand
jason at jlekstrand.net
Tue Jan 3 15:47:04 UTC 2017
On Tue, Jan 3, 2017 at 4:31 AM, Samuel Iglesias Gonsálvez <
siglesias at igalia.com> wrote:
> On Mon, 2017-01-02 at 10:34 -0800, Jason Ekstrand wrote:
> > I've left some inline comments but this patch has gotten me thinking
> > about the larger question of how we want to do conversion operations
> > in NIR. Ian already has patches for int64 but float16, int8, and
> > int16 are all coming. If we keep coming up with one-letter prefixes
> > (long, half, short, char?) and doing a2b for every opcode. We could,
> > but that would be very confusing. It would be good to cut down on
> > the combinatorics a bit.
> >
> > Here's my straw-man proposal: For each conversion, we leave the
> > destination as variable bit width and keep the source fixed. Then we
> > have u2f32, u2f64, u2f16, etc. and we have one opcode regardless of
> > destination bit width. This doesn't completely break the
> > combinatorial explosion but it does let us cut it by a factor of 3 or
> > 4.
> >
> > Another thing that Connor and I discussed was to let conversion
> > operations be the one exception to the "source and destination bit-
> > widths must match for variable bit-width operations" and add i2i and
> > f2f opcodes. That would drop the number of opcodes even further but
> > I'm not sure how bad the special-casing would end up being.
> >
> > Thoughts?
> >
>
>
> The first option doesn't really address the problem and feels more like
> a work-around, but I guess we could go with that if other people prefer
> that approach. Personally, I think i would try the second option. I
> can't tell right know the implications this would have for the special-
> casing in the validation of variable bit-width opcodes, etc but I guess
> we can just try to write that patch and see how it looks like in the
> end, we can always resort to u2f32 and cia if we don't like it.
>
> In any case, regardless the solution we agree on, any of these changes
> would require time to get them right as they touch several places in
> NIR, so considering that the feature freeze for the next Mesa release
> is around the corner (January 13th [0]) I think we also need to discuss
> whether we want to tackle this before or after we land Vulkan Fp64.
>
> We are leaning towards landing Vulkan Fp64 before we do these changes
> so we can have that in the next release and also because we need the
> vertex input attributes changes introduced with this series for our
> Haswell VA64 implementation (which we were hoping to send this week for
> review too, although maybe we are trying to squeeze too many things for
> the next release :-D)
>
> What do you think?
>
I 100% agree. This was mostly a discussion for the future direction.
> Sam
>
> [0] https://lists.freedesktop.org/archives/mesa-dev/2016-November/13701
> 5.html
>
>
> > --Jason Ekstrand
> >
> > On Fri, Dec 16, 2016 at 6:49 AM, Juan A. Suarez Romero <jasuarez at igal
> > ia.com> wrote:
> > > From: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> > >
> > > This function returns the nir_op corresponding to the conversion
> > > between
> > > the given nir_alu_type arguments.
> > >
> > > This function lacks support for integer-based types with bit_size
> > > != 32
> > > and for float16 conversion ops.
> > >
> > > Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> > > ---
> > > src/compiler/nir/nir.c | 83
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++
> > > src/compiler/nir/nir.h | 2 ++
> > > 2 files changed, 85 insertions(+)
> > >
> > > diff --git a/src/compiler/nir/nir.c b/src/compiler/nir/nir.c
> > > index 2d882f7..3e00452 100644
> > > --- a/src/compiler/nir/nir.c
> > > +++ b/src/compiler/nir/nir.c
> > > @@ -1953,3 +1953,86 @@
> > > nir_system_value_from_intrinsic(nir_intrinsic_op intrin)
> > > unreachable("intrinsic doesn't produce a system value");
> > > }
> > > }
> > > +
> > > +nir_op
> > > +nir_type_conversion_op(nir_alu_type src, nir_alu_type dst)
> > > +{
> > > + nir_alu_type src_base_type = (nir_alu_type)
> > > nir_alu_type_get_base_type(src);
> > > + nir_alu_type dst_base_type = (nir_alu_type)
> > > nir_alu_type_get_base_type(dst);
> > > + unsigned src_bitsize = nir_alu_type_get_type_size(src);
> > > + unsigned dst_bitsize = nir_alu_type_get_type_size(dst);
> > > +
> > > + if (src_base_type == dst_base_type) {
> > > + if (src_bitsize == dst_bitsize)
> > > + return (src_base_type == nir_type_float) ? nir_op_fmov :
> > > nir_op_imov;
> > > +
> > > + switch (src_base_type) {
> > > + case nir_type_bool:
> > > + case nir_type_uint:
> > > + case nir_type_int:
> > > + return nir_op_imov;
> >
> > These three cases can't happen right now. We should just assert that
> > it's float.
> >
> > > + case nir_type_float:
> > > + /* TODO: implement support for float16 */
> > > + assert(src_bitsize == 64 || dst_bitsize == 64);
> > > + return (src_bitsize == 64) ? nir_op_d2f : nir_op_f2d;
> > > + default:
> > > + assert(!"Invalid conversion");
> > > + };
> > > + }
> > > +
> > > + /* Different base type but same bit_size */
> > > + if (src_bitsize == dst_bitsize) {
> > > + /* TODO: This does not include specific conversions between
> > > + * signed or unsigned integer types of bit size different of
> > > 32 yet.
> > > + */
> > > + assert(src_bitsize == 32);
> > > + switch (src_base_type) {
> > > + case nir_type_uint:
> > > + return (dst_base_type == nir_type_float) ? nir_op_u2f :
> > > nir_op_imov;
> > > + case nir_type_int:
> > > + return (dst_base_type == nir_type_float) ? nir_op_i2f :
> > > nir_op_imov;
> > > + case nir_type_bool:
> > > + return (dst_base_type == nir_type_float) ? nir_op_b2f :
> > > nir_op_imov;
> >
> > There's also a b2i opcode we should handle...
> >
> > > + case nir_type_float:
> > > + return (dst_base_type == nir_type_uint) ? nir_op_f2u :
> > > + (dst_base_type == nir_type_bool) ? nir_op_f2b :
> > > nir_op_f2i;
> >
> > Switch?
> >
> > > + default:
> > > + assert(!"Invalid conversion");
> > > + };
> > > + }
> > > +
> > > + /* Different bit_size and different base type */
> > > + /* TODO: Implement integer support for types with bit_size !=
> > > 32 */
> > > + switch (src_base_type) {
> > > + case nir_type_uint:
> > > + assert(dst == nir_type_float64);
> > > + return nir_op_u2d;
> > > + case nir_type_int:
> > > + assert(dst == nir_type_float64);
> > > + return nir_op_i2d;
> > > + case nir_type_bool:
> > > + assert(dst == nir_type_float64);
> > > + return nir_op_u2d;
> > > + case nir_type_float:
> > > + assert(src_bitsize == 32 || src_bitsize == 64);
> > > + if (src_bitsize != 64) {
> > > + assert(dst == nir_type_float64);
> > > + return nir_op_f2d;
> > > + }
> > > + assert(dst_bitsize == 32);
> > > + switch (dst_base_type) {
> > > + case nir_type_uint:
> > > + return nir_op_d2u;
> > > + case nir_type_int:
> > > + return nir_op_d2i;
> > > + case nir_type_bool:
> > > + return nir_op_d2b;
> > > + case nir_type_float:
> > > + return nir_op_d2f;
> > > + default:
> > > + assert(!"Invalid conversion");
> > > + };
> > > + default:
> > > + assert(!"Invalid conversion");
> > > + };
> > > +}
> > > diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> > > index 9310dab..9f3abb7 100644
> > > --- a/src/compiler/nir/nir.h
> > > +++ b/src/compiler/nir/nir.h
> > > @@ -689,6 +689,8 @@ nir_get_nir_type_for_glsl_type(const struct
> > > glsl_type *type)
> > > unreachable("unknown type");
> > > }
> > >
> > > +nir_op nir_type_conversion_op(nir_alu_type src, nir_alu_type dst);
> > > +
> > > typedef enum {
> > > NIR_OP_IS_COMMUTATIVE = (1 << 0),
> > > NIR_OP_IS_ASSOCIATIVE = (1 << 1),
> > > --
> > > 2.9.3
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > 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/20170103/039530b2/attachment-0001.html>
More information about the mesa-dev
mailing list