<html dir="ltr"><head></head><body style="text-align:left; direction:ltr;"><div>On Fri, 2019-01-18 at 06:54 -0600, Jason Ekstrand wrote:</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"> <div dir="auto"> <div dir="auto"><br></div> <div id="aqm-original" style="color: black;">  <div style="text-align:left; direction:ltr;" class="aqm-original-body"><div style="color: black;"> <p style="color: black; font-size: 10pt; font-family: sans-serif; margin: 8pt 0;">On January 18, 2019 04:47:51 Iago Toral <itoral@igalia.com> wrote:</p> <blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"> <div>On Thu, 2019-01-17 at 14:18 -0600, Jason Ekstrand wrote:</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Tue, Jan 15, 2019 at 7:55 AM Iago Toral Quiroga <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> wrote:<br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex">Source0 and Destination extract the floating-point precision automatically<br> from the SrcType and DstType instruction fields respectively when they are<br> set to types :F or :HF. For Source1 and Source2 operands, we use the new<br> 1-bit fields Src1Type and Src2Type, where 0 means normal precision and 1<br> means half-precision. Since we always use the type of the destination for<br> all operands when we emit 3-source instructions, we only need set Src1Type<br> and Src2Type to 1 when we are emitting a half-precision instruction.<br> <br> v2:<br> - Set the bit separately for each source based on its type so we can<br> do mixed floating-point mode in the future (Topi).<br> <br> Reviewed-by: Topi Pohjolainen <<a href="mailto:topi.pohjolainen@intel.com" target="_blank">topi.pohjolainen@intel.com</a>><br> ---<br> src/intel/compiler/brw_eu_emit.c | 16 ++++++++++++++++<br> 1 file changed, 16 insertions(+)<br> <br> diff --git a/src/intel/compiler/brw_eu_emit.c b/src/intel/compiler/brw_eu_emit.c<br> index a785f96b650..2fa89f8a2a3 100644<br> --- a/src/intel/compiler/brw_eu_emit.c<br> +++ b/src/intel/compiler/brw_eu_emit.c<br> @@ -801,6 +801,22 @@ brw_alu3(struct brw_codegen *p, unsigned opcode, struct brw_reg dest,<br> */<br> brw_inst_set_3src_a16_src_type(devinfo, inst, dest.type);<br> brw_inst_set_3src_a16_dst_type(devinfo, inst, dest.type);<br> +<br> + /* From the Bspec: Instruction types<br> + *<br> + * Three source instructions can use operands with mixed-mode<br> + * precision. When SrcType field is set to :f or :hf it defines<br> + * precision for source 0 only, and fields Src1Type and Src2Type<br> + * define precision for other source operands:<br> + *<br> + * 0b = :f. Single precision Float (32-bit).<br> + * 1b = :hf. Half precision Float (16-bit).<br> + */<br> + if (src1.type == BRW_REGISTER_TYPE_HF)<br> + brw_inst_set_3src_a16_src1_type(devinfo, inst, 1);<br></blockquote><div><br></div><div>Maybe worth throwing in an</div><div><br></div><div>assert(src0.type == BRW_REGISTER_TYPE_F || src0.type == BRW_REGISTER_TYPE_HF);</div><div><br></div><div>just to be sure? </div></div></div></blockquote><div><br></div><div>If we are going to do this I guess we should also check the same for src2.</div></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">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.</div></div></blockquote><div><br></div><div>Actually, I don't think that is going to work. There is this comment right above this hunk:</div><div><br></div><div> /* Set both the source and destination types based on dest.type,</div><div> * ignoring the source register types. The MAD and LRP emitters ensure</div><div> * that all four types are float. The BFE and BFI2 emitters, however,</div><div> * may send us mixed D and UD types and want us to ignore that and use</div><div> * the destination type.</div><div> */</div><div><br></div><div>I guess we could still formulate a readable assertion using helpers, something like this:</div><div><br></div><div>assert(all_types_32bit_integer(src0.type, src1.type, src2.type) ||</div><div> all_types_mixed_float(src0.type, src1.type, src2.type) ||</div><div> all_types_64bit_float(src0.type, src1.type, src2.type));</div><div><br></div><div>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?</div><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="auto"><div id="aqm-original" style="color: black;" dir="auto"><div style="text-align:left; direction:ltr;" class="aqm-original-body"><div style="color: black;"><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> Either way, this and patch 20 are</div><div><br></div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br></div><div> </div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"> +<br> + if (src2.type == BRW_REGISTER_TYPE_HF)<br> + brw_inst_set_3src_a16_src2_type(devinfo, inst, 1);<br> }<br></blockquote></div></div></blockquote><div><br></div><div>And the same here (for src0 and src1)</div><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"> }<br> <br> </blockquote></div></div></blockquote><br></blockquote> </div> </div>  </div><div dir="auto"><br></div> </div> </blockquote></body></html>