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

[Matteo Bruni]

2017-05-30 9:51 GMT+02:00 Paul Gofman <gofmanp at gmail.com>:
> On 05/30/2017 01:59 AM, Matteo Bruni wrote:
>>
>>
>>>>> @@ -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.
>>
>> Right, that makes sense, including the likely bug you mention.
>>
>> In theory it should be possible to do the FIXME check at init time.
>> Actually, the whole constant upload info could be precomputed. Just
>> putting it out there...
>>
> My plan was actually to get rid of this FIXME at all (it is actually more of
> a debug assertion rather than true FIXME which seems redundant now), and
> besides introduce more code paths for matrix settings, like e. g. the cases
> when output stride is the same as input. I strictly want to avoid any 'if'
> in the inner loops at least. Ideally I would maybe like to end up with
> strides values specialized loops for possible matrix sizes, but not sure yet
> if it worth it. Regarding precomputing matrix strides / counts, I tried to
> store them all in const_set structure (after the optimizations I mentioned),
> but did not see any improvement. It takes quite a lot of fields and probably
> getting them on the fly takes not more time than adding much more data to
> const_set.

Makes sense.