[Mesa-dev] [PATCH v3 21/42] intel/compiler: set correct precision fields for 3-source float instructions

[Jason Ekstrand]

On January 18, 2019 04:47:51 Iago Toral <itoral at 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 <itoral at 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.pohjolainen at 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.

>
>
>> Either way, this and patch 20 are
>>
>> Reviewed-by: Jason Ekstrand <jason at 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)
>
>>>    }

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190118/b60e20e3/attachment-0001.html>