[Mesa-dev] [PATCH 3/3] i965/fs: emit DIM instruction to load 64-bit immediates in HSW

[Samuel Iglesias Gonsálvez]

On 14/07/16 03:46, Matt Turner wrote:
> On Wed, Jul 13, 2016 at 5:06 PM, Matt Turner <mattst88 at gmail.com> wrote:
>> On Tue, Jul 12, 2016 at 11:42 PM, Samuel Iglesias Gonsálvez
>> <siglesias at igalia.com> wrote:
>>> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
>>> ---
>>>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 12 ++++++++++++
>>>  1 file changed, 12 insertions(+)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>> index a65c273..bf32dfd 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>> @@ -4558,6 +4558,18 @@ setup_imm_df(const fs_builder &bld, double v)
>>>     if (devinfo->gen >= 8)
>>>        return brw_imm_df(v);
>>>
>>> +   /* gen7.5 does not support DF immediates straighforward but the DIM
>>> +    * instruction allows to set the 64-bit immediate value.
>>> +    */
>>> +   if (devinfo->is_haswell) {
>>> +      const fs_builder ubld = bld.exec_all();
>>> +      fs_reg dst = ubld.vgrf(BRW_REGISTER_TYPE_DF, 1);
>>> +      struct brw_reg imm = brw_imm_reg(BRW_REGISTER_TYPE_F);
>>> +      imm.df = v;
>>> +      ubld.DIM(dst, imm);
>>
>> I know the hardware is strange and requires that src0's type is F, but
>> I don't think we need to model that in the IR. I think that using a DF
>> type in the IR
>> would require otherwise unnecessary changes to dump_instructions().
>>
>> With the above three lines changed to just
>>
>>    ubld.DIM(dst, brw_imm_df(v));
>>
>> this patch is:
>>
>> Reviewed-by: Matt Turner <mattst88 at gmail.com>
>>
>> Patch 1 I sent comments on. With those addressed it is also
>>
>> Reviewed-by: Matt Turner <mattst88 at gmail.com>
>>
>> I believe with my comments addressed on 1/3 and 3/3 that 2/3 is unecessary.
> 
> Actually, I guess 2/3 is necessary since the type is changed before
> brw_eu_emit? Sorry, not thinking very clearly.
> 

Exactly, patch 2/3 is needed because the type has been changed before
brw_eu_emit.

Does patch 2/3 get your R-b then?

Thanks for the review!

Sam

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160714/6aaba018/attachment.sig>