Re: [PATCH 2/2] d3d10/effect: Handle vector arguments in expression instructions.

[Matteo Bruni]

On Tue, Nov 16, 2021 at 6:15 PM Nikolay Sivov  wrote:
> Signed-off-by: Nikolay Sivov
>
> dlls/d3d10/effect.c | 39 +++++++++++++++++++--------------------
>  1 file changed, 19 insertions(+), 20 deletions(-)
>
> diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c
> index 9c2012768dd..74ee4bf468c 100644
> --- a/dlls/d3d10/effect.c
> +++ b/dlls/d3d10/effect.c
> @@ -191,17 +191,21 @@ struct preshader_instr
>      unsigned int scalar     :  1;
>  };
>
> -typedef float (*pres_op_func)(float **args, unsigned int n);
> +typedef void (*pres_op_func)(float **args, unsigned int n, const struct preshader_instr *instr);
>
> -static float pres_ftou(float **args, unsigned int n)
> +static void pres_ftou(float **args, unsigned int n, const struct preshader_instr *instr)
>  {
>      unsigned int u = *args[0];
> - return *(float *)&u;
> - *args[1] = *(float *)&u;
>
> }

Is ftou always scalar? Not that I'm quite sure how to test it i.e.
what would be a case where a vector ftou could be in effect?

What I'm getting at, I think it would be safer to handle vector ftou,
just in case that's a thing.
> -static float pres_add(float **args, unsigned int n)
> +static void pres_add(float **args, unsigned int n, const struct preshader_instr *instr)
>  {
> - return *args[0] + *args[1];
> - float *retval = args[2];
> - unsigned int i;
> -
> - for (i = 0; i < instr->comp_count; ++i)
> -
>    retval[i] = args[0][instr->scalar ? 0 : i] + args[1][i];
>
> }
>
> struct preshader_op_info
> @@ -333,10 +337,10 @@ static float * d3d10_effect_preshader_get_reg_ptr(const struct d3d10_effect_pres
>
> static HRESULT d3d10_effect_preshader_eval(struct d3d10_effect_preshader *p)
>  {
> - unsigned int i, j, regt, offset, instr_count, input_count;
> - unsigned int i, j, regt, offset, instr_count, arg_count;
> const DWORD *ip = ID3D10Blob_GetBufferPointer(p->code);
> - float *dst, *args[4], *retval;
> struct preshader_instr ins;
> -
>
> float *dst, *args[4];
>
> dst = d3d10_effect_preshader_get_reg_ptr(p, D3D10_REG_TABLE_RESULT, 0);
> memset(dst, 0, sizeof(float) * p->reg_tables[D3D10_REG_TABLE_RESULT].count);
>
> @@ -355,16 +359,15 @@ static HRESULT d3d10_effect_preshader_eval(struct d3d10_effect_preshader *p)
>      for (i = 0; i < instr_count; ++i)
>      {
>          *(DWORD *)&ins = *ip++;
> -
>    input_count = *ip++;
> -
>    arg_count = 1 + *ip++;
>
> -
>    if (input_count > ARRAY_SIZE(args))
> -
>    if (arg_count > ARRAY_SIZE(args))
>    {
> -
>        FIXME("Unexpected argument count %u.\n", input_count);
> -
>        FIXME("Unexpected argument count %u.\n", arg_count);
>        return E_FAIL;
>    }
>
> -
>    /* Arguments */
> -
>    for (j = 0; j < input_count; ++j)
> -
>    for (j = 0; j < arg_count; ++j)
>    {
>        ip++; /* TODO: argument register flags are currently ignored */
>        regt = *ip++;
>
> @@ -373,12 +376,7 @@ static HRESULT d3d10_effect_preshader_eval(struct d3d10_effect_preshader *p)
>              args[j] = d3d10_effect_preshader_get_reg_ptr(p, regt, offset);
>          }
> -
>    ip++; /* TODO: result register flags are currently ignored */
> -
>    regt = *ip++;
> -
>    offset = *ip++;
> -
>    retval = d3d10_effect_preshader_get_reg_ptr(p, regt, offset);
> -
> -
>    *retval = d3d10_effect_get_op_info(ins.opcode)->func(args, input_count);
> -
>    d3d10_effect_get_op_info(ins.opcode)->func(args, arg_count, &ins);

I guess a small comment mentioning that the last argument is
(usually?) the return register could be useful.
>  }
>
>  return S_OK;
>
> @@ -2100,7 +2098,7 @@ static HRESULT parse_fx10_preshader_instr(struct d3d10_preshader_parse_context *
>          unsigned int *offset, const char **ptr, unsigned int data_size)
>  {
>      const struct preshader_op_info *op_info;
> - unsigned int i, param_count;
> - unsigned int i, param_count, size;
> struct preshader_instr ins;
> uint32_t input_count;
>
> @@ -2152,7 +2150,8 @@ static HRESULT parse_fx10_preshader_instr(struct d3d10_preshader_parse_context *
>              case D3D10_REG_TABLE_CB:
>              case D3D10_REG_TABLE_RESULT:
>              case D3D10_REG_TABLE_TEMP:
> -
>            context->table_sizes[regt] = max(context->table_sizes[regt], param_offset + 1);
> -
>            size = param_offset + (ins.scalar && i == 0 ? 1 : ins.comp_count);
> -
>            context->table_sizes[regt] = max(context->table_sizes[regt], size);
>            break;
>        default:
>            FIXME("Unexpected register table index %u.\n", regt);