[PATCH 2/3] d3dx9: Support relative addressing in preshader.

[Paul Gofman]

On 03/24/2017 03:31 AM, Matteo Bruni wrote:
> 2017-03-24 0:39 GMT+01:00 Paul Gofman <gofmanp at gmail.com>:
>> On 03/24/2017 01:57 AM, Matteo Bruni wrote:
>>> 2017-03-23 16:06 GMT+01:00 Paul Gofman <gofmanp at gmail.com>:
>>>> Signed-off-by: Paul Gofman <gofmanp at gmail.com>
>>>> ---
>>>>    dlls/d3dx9_36/preshader.c    | 103
>>>> ++++++++++++++++++++++++++++++++++++-------
>>>>    dlls/d3dx9_36/tests/effect.c |  38 ++++++++--------
>>>>    2 files changed, 106 insertions(+), 35 deletions(-)
>>>>
>>>> diff --git a/dlls/d3dx9_36/preshader.c b/dlls/d3dx9_36/preshader.c
>>>> index 9aedc99..2974ffd 100644
>>>> --- a/dlls/d3dx9_36/preshader.c
>>>> +++ b/dlls/d3dx9_36/preshader.c
>>>> @@ -191,6 +191,8 @@ struct d3dx_pres_operand
>>>>        /* offset is component index, not register index, e. g.
>>>>           offset for component c3.y is 13 (3 * 4 + 1) */
>>>>        unsigned int offset;
>>>> +    enum pres_reg_tables base_reg_table;
>>>> +    unsigned int base_reg_offset;
>>>>    };
>>> It would be nicer to create a struct for storing together table and
>>> offset and to pass it around to the various functions taking both
>>> arguments, like parse_pres_reg().
>>> It should probably be a separate patch before this one.
>> Actually I can't guess the case when base_reg_table would be not a floating
>> constant preshader input. Maybe I just remove this variable and check with
>> FIXME during parsing?
> Sure, that's fine. Still the struct seems useful for parameter
> passing, if not right there in struct d3dx_pres_operand.
>
Sorry, I messed up things actually, base register actually can come from 
temporary (register) table also, which happens in the test. So I will 
introduce extra structure for registers.

>
>>>>    static double exec_get_arg(struct d3dx_regstore *rs, const struct
>>>> d3dx_pres_operand *opr, unsigned int comp)
>>>>    {
>>>> -    if (!regstore_is_val_set_reg(rs, opr->table, (opr->offset + comp) /
>>>> table_info[opr->table].reg_component_count))
>>>> -        WARN("Using uninitialized input, table %u, offset %u.\n",
>>>> opr->table, opr->offset + comp);
>>>> +    unsigned int base_offset, index;
>>>> +
>>>> +    if (opr->base_reg_table == PRES_REGTAB_COUNT)
>>>> +        index = 0;
>>>> +    else
>>>> +        index = (int)exec_get_reg_value(rs, opr->base_reg_table,
>>>> opr->base_reg_offset);
>>> I think it's better with lrint() in place of the cast.
>> I just want to note that whenever the input is from floating values FX
>> compiler generates preshader instructions which do the rounding, and skips
>> that if the base register comes from integers. Do you think we need to do
>> extra rounding in this case?
> Ah, you're right, I somehow overlooked that. Actually I think
> doublechecking (e.g. with a FIXME) that the source is really integer
> would be useful. If it isn't too much of a mess it would be nice to
> avoid the roundtrip to doubles for the index entirely.
     Source of the base register value seems not so straightforward to 
check. The input table always has float format. To do such check, we 
need to find which parameter the register is initialized from, we don't 
have such a link now. Besides, using float register as index seems 
totally ok in effect, compiler generates rounding code for this case. 
Moreover, base register used in instruction may come from temporary 
("r") registers and be a result of some operations, like rounding or 
multiplication when indexing anything with stride other than 4xfloat.
     Or could it be you had in mind not checking the source of register 
value, but rather that the value is actually an integer (fractional part 
is zero)?