Re: [PATCH 3/6] wined3d: Handle raw thread group shared memory declarations.

[Henri Verbeet]

On 28 February 2017 at 14:42, Józef Kucia  wrote:
> @@ -1047,6 +1047,31 @@ static HRESULT shader_get_registers_used(struct wined3d_shader *shader, const st
>          {
>              reg_maps->temporary_count = ins.declaration.count;
>          }
> -
>    else if (ins.handler_idx == WINED3DSIH_DCL_TGSM_RAW)
> -
>    {
> -
>        unsigned int reg_idx = ins.declaration.tgsm_raw.reg.reg.idx[0].offset;
> -
>        if (reg_idx >= MAX_TGSM_REGISTERS)
> -
>        {
> -
>            ERR("Invalid TGSM register index %u.\n", reg_idx);
> -
>            break;
> -
>        }
> -
>        if (shader_version.type == WINED3D_SHADER_TYPE_COMPUTE)
> -
>        {
> -
>            struct wined3d_shader_tgsm *tgsm;
> -
> -
>            if (!wined3d_array_reserve((void **)&reg_maps->tgsm, &reg_maps->tgsm_capacity,
> -
>                    reg_idx + 1, sizeof(*reg_maps->tgsm)))

I'm not sure about this. I guess the expectation is that these will
get allocated consecutively and starting from 0, while even starting
at MAX_TGSM_REGISTERS-1 and going down wouldn't be terrible. Still,
I'm wondering if it wouldn't be worth it to only allocate space for
the number of declarations, and use e.g. bsearch().

[PATCH 3/6] wined3d: Handle raw thread group shared memory declarations.

[Józef Kucia]

On Tue, Feb 28, 2017 at 5:11 PM, Henri Verbeet  wrote:
> On 28 February 2017 at 14:42, Józef Kucia  wrote:
> > @@ -1047,6 +1047,31 @@ static HRESULT shader_get_registers_used(struct wined3d_shader *shader, const st
> >          {
> >              reg_maps->temporary_count = ins.declaration.count;
> >          }
> > -
> >    else if (ins.handler_idx == WINED3DSIH_DCL_TGSM_RAW)
> > -
> >    {
> > -
> >        unsigned int reg_idx = ins.declaration.tgsm_raw.reg.reg.idx[0].offset;
> > -
> >        if (reg_idx >= MAX_TGSM_REGISTERS)
> > -
> >        {
> > -
> >            ERR("Invalid TGSM register index %u.\n", reg_idx);
> > -
> >            break;
> > -
> >        }
> > -
> >        if (shader_version.type == WINED3D_SHADER_TYPE_COMPUTE)
> > -
> >        {
> > -
> >            struct wined3d_shader_tgsm *tgsm;
> > -
> > -
> >            if (!wined3d_array_reserve((void **)&reg_maps->tgsm, &reg_maps->tgsm_capacity,
> > -
> >                    reg_idx + 1, sizeof(*reg_maps->tgsm)))
>
> I'm not sure about this. I guess the expectation is that these will
> get allocated consecutively and starting from 0, while even starting
> at MAX_TGSM_REGISTERS-1 and going down wouldn't be terrible. Still,
> I'm wondering if it wouldn't be worth it to only allocate space for
> the number of declarations, and use e.g. bsearch().

Yes, the shader compile seems to always allocate TGSM registers
consecutively. Additionally, the typical shader uses up to a few TGSM
registers (1-5). Anyway, I can change it to use bsearch(), if this is
preferred.