On Fri, 2019-01-18 at 06:54 -0600, Jason Ekstrand wrote:
>
>
>
>
>
> On January 18, 2019 04:47:51 Iago Toral <ito...@igalia.com> wrote:
> > On Thu, 2019-01-17 at 14:18 -0600, Jason Ekstrand wrote:
> > > On Tue, Jan 15, 2019 at 7:55 AM Iago Toral Quiroga <
> > > ito...@igalia.com> wrote:
> > > > Source0 and Destination extract the floating-point precision
> > > > automatically
> > > >
> > > > from the SrcType and DstType instruction fields respectively
> > > > when they are
> > > >
> > > > set to types :F or :HF. For Source1 and Source2 operands, we
> > > > use the new
> > > >
> > > > 1-bit fields Src1Type and Src2Type, where 0 means normal
> > > > precision and 1
> > > >
> > > > means half-precision. Since we always use the type of the
> > > > destination for
> > > >
> > > > all operands when we emit 3-source instructions, we only need
> > > > set Src1Type
> > > >
> > > > and Src2Type to 1 when we are emitting a half-precision
> > > > instruction.
> > > >
> > > >
> > > >
> > > > v2:
> > > >
> > > > - Set the bit separately for each source based on its type so
> > > > we can
> > > >
> > > > do mixed floating-point mode in the future (Topi).
> > > >
> > > >
> > > >
> > > > Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
> > > >
> > > > ---
> > > >
> > > > src/intel/compiler/brw_eu_emit.c | 16 ++++++++++++++++
> > > >
> > > > 1 file changed, 16 insertions(+)
> > > >
> > > >
> > > >
> > > > diff --git a/src/intel/compiler/brw_eu_emit.c
> > > > b/src/intel/compiler/brw_eu_emit.c
> > > >
> > > > index a785f96b650..2fa89f8a2a3 100644
> > > >
> > > > --- a/src/intel/compiler/brw_eu_emit.c
> > > >
> > > > +++ b/src/intel/compiler/brw_eu_emit.c
> > > >
> > > > @@ -801,6 +801,22 @@ brw_alu3(struct brw_codegen *p, unsigned
> > > > opcode, struct brw_reg dest,
> > > >
> > > > */
> > > >
> > > > brw_inst_set_3src_a16_src_type(devinfo, inst,
> > > > dest.type);
> > > >
> > > > brw_inst_set_3src_a16_dst_type(devinfo, inst,
> > > > dest.type);
> > > >
> > > > +
> > > >
> > > > + /* From the Bspec: Instruction types
> > > >
> > > > + *
> > > >
> > > > + * Three source instructions can use operands with
> > > > mixed-mode
> > > >
> > > > + * precision. When SrcType field is set to :f or :hf
> > > > it defines
> > > >
> > > > + * precision for source 0 only, and fields Src1Type
> > > > and Src2Type
> > > >
> > > > + * define precision for other source operands:
> > > >
> > > > + *
> > > >
> > > > + * 0b = :f. Single precision Float (32-bit).
> > > >
> > > > + * 1b = :hf. Half precision Float (16-bit).
> > > >
> > > > + */
> > > >
> > > > + if (src1.type == BRW_REGISTER_TYPE_HF)
> > > >
> > > > + brw_inst_set_3src_a16_src1_type(devinfo, inst, 1);
> > >
> > > Maybe worth throwing in an
> > >
> > > assert(src0.type == BRW_REGISTER_TYPE_F || src0.type ==
> > > BRW_REGISTER_TYPE_HF);
> > >
> > > just to be sure?
> >
> > If we are going to do this I guess we should also check the same
> > for src2.
>
> Yeah, it'd probably be good to have a general assertion that the
> three sources have the same type with the caveat that they can vary
> to mix half and full float. Maybe that would be better than
> something specific right here.
Actually, I don't think that is going to work. There is this comment
right above this hunk:
/* Set both the source and destination types based on
dest.type, * ignoring the source register types. The MAD and
LRP emitters ensure * that all four types are float. The BFE
and BFI2 emitters, however, * may send us mixed D and UD types
and want us to ignore that and use * the destination
type. */
I guess we could still formulate a readable assertion using helpers,
something like this:
assert(all_types_32bit_integer(src0.type, src1.type, src2.type)
|| all_types_mixed_float(src0.type, src1.type, src2.type)
|| all_types_64bit_float(src0.type, src1.type, src2.type));
but creating 3 helpers only for one assertion might a bit excessive...
so maybe just adding the specific asserts for src1 and src2 when they
are HF is more reasonable?
> > > Either way, this and patch 20 are
> > > Reviewed-by: Jason Ekstrand <ja...@jlekstrand.net>
> > >
> > > > +
> > > >
> > > > + if (src2.type == BRW_REGISTER_TYPE_HF)
> > > >
> > > > + brw_inst_set_3src_a16_src2_type(devinfo, inst, 1);
> > > >
> > > > }
> >
> > And the same here (for src0 and src1)
> > > > }
> > > >
> > > >
> > > >
>
>
>
>
>
>
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
