[v2 1/2] d3dx9: Support relative addressing in preshader.

[Matteo Bruni]

2017-03-24 23:12 GMT+01:00 Paul Gofman <gofmanp at gmail.com>:

> @@ -697,13 +739,13 @@ static HRESULT parse_preshader(struct d3dx_preshader *pres, unsigned int *ptr, u
>      for (i = 0; i < pres->ins_count; ++i)
>      {
>          for (j = 0; j < pres_op_info[pres->ins[i].op].input_count; ++j)
> -            update_table_size(pres->regs.table_sizes, pres->ins[i].inputs[j].table,
> -                    get_reg_offset(pres->ins[i].inputs[j].table,
> -                    pres->ins[i].inputs[j].offset + pres->ins[i].component_count - 1));
> +            update_table_size(pres->regs.table_sizes, pres->ins[i].inputs[j].reg.table,
> +                    get_reg_offset(pres->ins[i].inputs[j].reg.table,
> +                    pres->ins[i].inputs[j].reg.offset + pres->ins[i].component_count - 1));
>
> -        update_table_size(pres->regs.table_sizes, pres->ins[i].output.table,
> -                get_reg_offset(pres->ins[i].output.table,
> -                pres->ins[i].output.offset + pres->ins[i].component_count - 1));
> +        update_table_size(pres->regs.table_sizes, pres->ins[i].output.reg.table,
> +                get_reg_offset(pres->ins[i].output.reg.table,
> +                pres->ins[i].output.reg.offset + pres->ins[i].component_count - 1));

Do these update_table_size() always do the right thing with relative addressing?
If I'm not mistaken, they should be fine because we only accept
relative addressing for CONST and IMMED and the former is taken care
by update_table_sizes_consts() just below while the latter's size is
set statically just above this chunk. I think that means though that
we don't need to call update_table_size() on the input arguments.

If that sounds right to you, you could drop that "for(j)" altogether
or, probably better, replace the update_table_size() with a check that
the argument is not out of bounds (probably returning failure if the
check doesn't pass). That should be a separate patch.

> @@ -1088,18 +1130,65 @@ static HRESULT init_set_constants(struct d3dx_const_tab *const_tab, ID3DXConstan
>      return ret;
>  }
>
> +static double exec_get_reg_value(struct d3dx_regstore *rs, enum pres_reg_tables table, unsigned int offset)
> +{
> +    if (!regstore_is_val_set_reg(rs, table, offset / table_info[table].reg_component_count))
> +        WARN("Using uninitialized input, table %u, offset %u.\n", table, offset);
> +
> +    return regstore_get_double(rs, table, offset);
> +}
> +
>  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 offset, base_index, reg_index, table;
> +
> +    table = opr->reg.table;
> +
> +    if (opr->index_reg.table == PRES_REGTAB_COUNT)
> +        base_index = 0;
> +    else
> +        base_index = lrint(exec_get_reg_value(rs, opr->index_reg.table, opr->index_reg.offset));
> +
> +    /* '4' is used instead of reg_component_count, as immediate constants (which have
> +     *  reg_component_count of 1) are still indexed as 4 values according to the tests.
> +     */
> +    offset = base_index * 4 + opr->reg.offset + comp;

Actually, is there any reason why reg_component_count for
PRES_REGTAB_IMMED isn't 4? Maybe change it to 4 and in
parse_preshader() check that const_count is a multiple of 4 (which is
something we're depending on anyway). That would be a separate patch
too.

Clue me if I'm missing anything.

> +    reg_index = offset / table_info[table].reg_component_count;
> +
> +    if (reg_index >= rs->table_sizes[table])
> +    {
> +        unsigned int wrap_size;
> +
> +        if (table == PRES_REGTAB_CONST)
> +        {
> +            /* As it can be guessed from tests (by iterating base register value and
> +             * observing the period of result values repeat), offset into floating
> +             * constant table are wrapped to the nearest power of 2 and not to the
> +             * actual table size.
> +             */
> +            for (wrap_size = 1; wrap_size < rs->table_sizes[table]; wrap_size <<= 1)
> +                ;

Please close the comment on the last text line (i.e. "*/" shouldn't be
on its own line) and maybe remove the part in parentheses which is
pretty obvious. I like the comment otherwise.

Another thing on this, maybe add a WARN() on wrapped accesses to show
what offset (or value) is returned in that case? Ignore it if you
don't think that's useful.