[PATCH 1/6] d3dx9: Get rid of table lookup for converting between register indexes and offsets.

[Paul Gofman]

On 05/29/2017 09:19 PM, Matteo Bruni wrote:
> 2017-05-24 11:46 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:
>
>>   static unsigned int get_reg_offset(unsigned int table, unsigned int offset)
>>   {
>> -    return offset / table_info[table].reg_component_count;
>> +    return table == PRES_REGTAB_OBCONST ? offset : offset >> 2;
>> +}
>> +
>> +static unsigned int get_offset_reg(unsigned int table, unsigned int reg_idx)
>> +{
>> +    return table == PRES_REGTAB_OBCONST ? reg_idx : reg_idx << 2;
>>   }
> I suppose using an explicit division and multiplication here wouldn't
> change the generated code, in which case I'd slightly prefer that. It
> doesn't matter right now, just mentioning it for potentially similar
> cases in the future.
Yes, sure, if it would be constant literal for multiplication / division 
here.
>> @@ -1033,9 +1035,9 @@ static void set_constants(struct d3dx_regstore *rs, struct d3dx_const_tab *const
>>                   unsigned int offset;
>>
>>                   offset = start_offset + i * major_stride + j;
>> -                if (offset / table_info[table].reg_component_count >= rs->table_sizes[table])
>> +                if (get_reg_offset(table, offset) >= rs->table_sizes[table])
>>                   {
>> -                    if (table_info[table].reg_component_count != 1)
>> +                    if (table != PRES_REGTAB_OBCONST)
>>                           FIXME("Output offset exceeds table size, name %s, component %u.\n",
>>                                   debugstr_a(param->name), i);
>>                       break;
> Not new and probably I just forgot the details, but, why would the
> register component count matter for the FIXME?
The length of the data to be copied to registers can be limited by 
RegisterCount in (pre)shader constant description. But when register is 
4-value, the major count for output loop should already mind that (since 
we always have row count and column count <= 4). The only legitimate 
case we may need to break from the inner loop is boolean constant matrix 
setting. So actually fixme condition should never happen, though break 
from the inner loop on boolean constant setting may be required. I am 
changing this place in the next patches (I didn't send yet) when 
optimizing this place. I was actually going to add a bit more tests for 
this case along with the forthcoming changes, and I suspect a bug here 
(I actually need to effectively compare with constant register count and 
not with the table size). We have a test for full update with boolean 
matrices (which will not trigger the error here) but commit of 
individual matrices test probably will.

>
>> -        offset = reg_index * table_info[table].reg_component_count
>> -                + offset % table_info[table].reg_component_count;
>> +        offset = get_offset_reg(table, reg_index) + offset % get_offset_reg(table, 1);
> That "get_offset_reg(table, 1)" comes up a number of times. Maybe
> worth making an extra get_reg_components(table) (or similar) helper?
> Simply for even greater clarity.
>
>
Yes, sure.