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

[Matteo Bruni]

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.

Also this patch probably should have been split in two, with the first
patch introducing get_offset_reg() with no functional changes and the
second hardcoding the table check / dropping the table lookup. Not
worth a rework at this point I guess.

> @@ -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?

> -        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.