Matt Turner <matts...@gmail.com> writes:
> On Mon, Feb 9, 2015 at 6:08 AM, Francisco Jerez <curroje...@riseup.net> wrote:
>> Some instruction bits don't have a mapping defined to any compacted
>> instruction field. If they're ever set and we end up compacting the
>> instruction they will be forced to zero. Avoid using compaction in such
>> cases.
>>
>> v2: Align multiple lines of an expression to the same column. Change
>> conditional compaction of 3-source instructions to an
>> assertion. (Matt)
>> ---
>> src/mesa/drivers/dri/i965/brw_eu_compact.c | 46
>> ++++++++++++++++++++++++++++++
>> 1 file changed, 46 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_eu_compact.c
>> b/src/mesa/drivers/dri/i965/brw_eu_compact.c
>> index 8e33bcb..f80bcc1 100644
>> --- a/src/mesa/drivers/dri/i965/brw_eu_compact.c
>> +++ b/src/mesa/drivers/dri/i965/brw_eu_compact.c
>> @@ -843,10 +843,53 @@ set_3src_source_index(struct brw_context *brw,
>> brw_compact_inst *dst, brw_inst *
>> }
>>
>> static bool
>> +has_unmapped_bits(struct brw_context *brw, brw_inst *src)
>> +{
>> + /* Check for instruction bits that don't map to any of the fields of the
>> + * compacted instruction. The instruction cannot be compacted if any of
>> + * them are set. They overlap with:
>> + * - NibCtrl (bit 47 on Gen7, bit 11 on Gen8)
>> + * - Dst.AddrImm[9] (bit 47 on Gen8)
>> + * - Src0.AddrImm[9] (bit 95 on Gen8)
>> + */
>> + if (brw->gen >= 8)
>> + return brw_inst_bits(src, 95, 95) ||
>> + brw_inst_bits(src, 47, 47) ||
>> + brw_inst_bits(src, 11, 11) ||
>> + brw_inst_bits(src, 7, 7);
>
> 11 (NibCtrl) is the only one that isn't reserved. I'd rather assert
> that reserved bits are not set.
>
No, bits 47 and 95 are valid too and part of the AddrImm fields.
>> + else
>> + return brw_inst_bits(src, 95, 91) ||
>> + brw_inst_bits(src, 47, 47) ||
>> + brw_inst_bits(src, 7, 7) ||
>> + (brw->gen < 7 && brw_inst_bits(src, 90, 90));
>
> Same thing, 47 (NibCtrl) is the only non-reserved bit. I'd rather
> assert about the others.
>
>> +}
>> +
>> +static bool
>> +has_3src_unmapped_bits(struct brw_context *brw, brw_inst *src)
>> +{
>> + /* Check for three-source instruction bits that don't map to any of the
>> + * fields of the compacted instruction. All of them seem to be reserved
>> + * bits currently.
>> + */
>> + assert(brw->gen >= 8);
>> + if (brw->gen >= 9 || brw->is_cherryview)
>> + return brw_inst_bits(src, 127, 127) ||
>> + brw_inst_bits(src, 105, 105) ||
>
> 105 is Src1's SubRegNum[word] on CHV and is included in SourceIndex.
>
Ahh, that's true, good catch.
>> + brw_inst_bits(src, 7, 7);
>> + else
>> + return 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);
>
> Since bit 7 is reserved in all cases, we might just
> assert(brw_inst_bits(src, 7, 7) == 0) in
> brw_try_compact_instruction(). I don't have a preference.
>
> So, I think it'd be nice to differentiate between bits that aren't
> included in compact instructions and reserved bits.
>
> (Sorry, I could have given you this feedback in the first time around)
pgp3QKojZ224p.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
