[Mesa-dev] [PATCH v3 19/42] intel/compiler: don't compact 3-src instructions with Src1Type or Src2Type bits

[Jason Ekstrand]

On January 18, 2019 04:12:44 Iago Toral <itoral at igalia.com> wrote:
> On Thu, 2019-01-17 at 14:14 -0600, Jason Ekstrand wrote:
>> On Tue, Jan 15, 2019 at 7:55 AM Iago Toral Quiroga <itoral at igalia.com> wrote:
>>> We are now using these bits, so don't assert that they are not set, just
>>> avoid compaction in that case.
>>>
>>> Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
>>> ---
>>> src/intel/compiler/brw_eu_compact.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/intel/compiler/brw_eu_compact.c 
>>> b/src/intel/compiler/brw_eu_compact.c
>>> index ae14ef10ec0..20fed254331 100644
>>> --- a/src/intel/compiler/brw_eu_compact.c
>>> +++ b/src/intel/compiler/brw_eu_compact.c
>>> @@ -928,8 +928,11 @@ has_3src_unmapped_bits(const struct gen_device_info 
>>> *devinfo,
>>>       assert(!brw_inst_bits(src, 127, 126) &&
>>>              !brw_inst_bits(src, 105, 105) &&
>>>              !brw_inst_bits(src, 84, 84) &&
>>> -             !brw_inst_bits(src, 36, 35) &&
>>>              !brw_inst_bits(src, 7,  7));
>>> +
>>> +      /* Src1Type and Src2Type, used for mixed-precision floating point */
>>> +      if (brw_inst_bits(src, 36, 35))
>>> +         return true;
>>
>> You're only doing this in the broadwell case.  What about SKL+ and CHV?  
>> Can we compact mixed-precision stuff there?  Looks like maybe we can but 
>> there should be at least something in the commit message about that.
>
> In these platforms compaction is possible in some cases and 
> set_3src_control_index() takes
> care of this by including these bits in a tablre lookup for accepted 
> combinations. I can add this
> to the commit message:
>
> "We are now using these bits, so don't assert that they are not set. In 
> gen8, if these bits are set
> compaction is not possible. On gen9 and CHV platforms 
> set_3src_control_index() checks these
> bits (and others) against a table to validate if the particular bit 
> combination is eligible for
> compaction or not."
>
> With that said, if I am reading this correctly, it looks like all entries 
> in gen8_3src_control_index_table that allow compaction require these bits 
> to be zero at present, so I guess that right now we could also just extend 
> the check I am adding here for BDW to other platforms, however, I guess 
> that relying on the array with accepted bit combinations for compaction is 
> more reliable should any future platforms allow for more combinations.

Sounds good. With that commit message update, r-b me.

>
>>>    }
>>>
>>>    return false;

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