On Mon, Jan 21, 2019 at 3:17 AM Iago Toral <ito...@igalia.com> wrote:
> 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? > Yeah, I think we're spending way too much time trying to figure out how to assert. I think the right thing to do here is just to do it when src1 and src2 are HF. Don't over-complicate it. > > 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
