[Mesa-dev] [PATCH 08/14] i965/compaction: Reduce size of compacted_counts[] array.

[Matt Turner]

On Tue, Sep 23, 2014 at 12:50 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On Thu, Aug 28, 2014 at 8:10 PM, Matt Turner <mattst88 at gmail.com> wrote:
>>
>> The array was previously indexed in units of brw_compact_inst (8-bytes),
>> but before compaction all instructions are uncompacted, so every odd
>> element was unused.
>> ---
>>  src/mesa/drivers/dri/i965/brw_eu_compact.c | 59
>> +++++++++++++++++++-----------
>>  1 file changed, 37 insertions(+), 22 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_eu_compact.c
>> b/src/mesa/drivers/dri/i965/brw_eu_compact.c
>> index c291f96..acce663 100644
>> --- a/src/mesa/drivers/dri/i965/brw_eu_compact.c
>> +++ b/src/mesa/drivers/dri/i965/brw_eu_compact.c
>> @@ -1033,20 +1033,32 @@ static void
>>  update_uip_jip(struct brw_context *brw, brw_inst *insn,
>>                 int this_old_ip, int *compacted_counts)
>>  {
>> -   int scale = brw->gen >= 8 ? sizeof(brw_compact_inst) : 1;
>> -
>> -   int32_t jip = brw_inst_jip(brw, insn) / scale;
>> -   jip -= compacted_between(this_old_ip, this_old_ip + jip,
>> compacted_counts);
>> -   brw_inst_set_jip(brw, insn, jip * scale);
>> +   /* JIP and UIP are in units of:
>> +    *    - bytes on Gen8+; and
>> +    *    - compacted instructions on Gen6+.
>> +    */
>> +   int32_t jip = brw_inst_jip(brw, insn);
>> +   int32_t jip_compacted = jip / (brw->gen >= 8 ?
>> sizeof(brw_compact_inst) : 1);
>> +   int32_t jip_uncompacted = jip / (brw->gen >= 8 ? sizeof(brw_inst) :
>> 2);
>> +   jip_compacted -= compacted_between(this_old_ip,
>> +                                      this_old_ip + jip_uncompacted,
>> +                                      compacted_counts);
>
>
> Is this correct on gen >= 8?  It seems as if you would be missing a factor
> of sizeof(brw_compact_inst).  In general, this and the hunk below it seem
> very fragile.  Can we do these calculations without the gen >= 8 specific
> stuff and then do the factor of 8 at the end?

Not in a way I can see if we want to use the same array indexing on
all platforms (doing otherwise seems awful). The code is complicated
because the JIP and UIP are stored in different units on Gen8+. I
think immediately converting into common units of
compacted-instructions and uncompacted-instructions is the simplest
thing to do.

Unless I've misunderstood your feedback?

Also, the line below your comment converts the units of
compacted-instructions back into the hardware format:

>>
>> +   brw_inst_set_jip(brw, insn,
>> +                    jip_compacted * (brw->gen >= 8 ? sizeof(brw_inst) :
>> 1));