Re: [PATCH v2 1/3] vcomp: Implement _vcomp_for_dynamic_init_i8()

[Zebediah Figura]

Hello David, thanks for the patch!

I have a few comments and questions inlined.

On 2/16/22 19:54, David Koller wrote:
> diff --git a/dlls/vcomp/main.c b/dlls/vcomp/main.c
> index c4f2572c86a..91c16714ea6 100644
> --- a/dlls/vcomp/main.c
> +++ b/dlls/vcomp/main.c
> @@ -81,8 +81,14 @@ struct vcomp_thread_data
>       /* dynamic */
>       unsigned int            dynamic;
>       unsigned int            dynamic_type;
> - unsigned int            dynamic_begin;
> - unsigned int            dynamic_end;
> -
>
> union {
> -
>    unsigned int        dynamic_begin;
> -
>    ULONG64             dynamic_begin_i8;
> -
>
> };
> -
>
> union {
> -
>    unsigned int        dynamic_end;
> -
>    ULONG64             dynamic_end_i8;
> -
>
> };
> };
>
> struct vcomp_team_data
>
> @@ -113,11 +119,22 @@ struct vcomp_task_data
>   /* dynamic */
>   unsigned int            dynamic;
> - unsigned int            dynamic_first;
> - unsigned int            dynamic_last;
> - unsigned int            dynamic_iterations;
> - int                     dynamic_step;
> - unsigned int            dynamic_chunksize;
> -
>
> union {
> -
>    struct {
> -
>        unsigned int    dynamic_first;
> -
>        unsigned int    dynamic_last;
> -
>        unsigned int    dynamic_iterations;
> -
>        int             dynamic_step;
> -
>        unsigned int    dynamic_chunksize;
> -
>    };
> -
>    struct {
> -
>        ULONG64         dynamic_first_i8;
> -
>        ULONG64         dynamic_last_i8;
> -
>        ULONG64         dynamic_iterations_i8;
> -
>        LONG64          dynamic_step_i8;
> -
>        ULONG64         dynamic_chunksize_i8;
> -
>    };
> -
>
> };
> };
>
> static void **ptr_from_va_list(va_list valist)

Does it even make sense to make these unions? Can we just make the 
relevant members 64-bit?

Alternatively, should these be stored separately from the 32-bit 
versions? Tests showing what happens if you mix and match 32-bit and 
64-bit calls might be helpful.
> @@ -1483,6 +1500,78 @@ void CDECL _vcomp_for_dynamic_init(unsigned int flags, unsigned int first, unsig
>       }
>   }
>
> +void CDECL _vcomp_for_dynamic_init_i8(unsigned int flags, ULONG64 first, ULONG64 last,
> -
>                                  LONG64 step, ULONG64 chunksize)
>
> +{
> - ULONG64 iterations, per_thread, remaining;
> - struct vcomp_thread_data *thread_data = vcomp_init_thread_data();
> - struct vcomp_team_data *team_data = thread_data->team;
> - struct vcomp_task_data *task_data = thread_data->task;
> - int num_threads = team_data ? team_data->num_threads : 1;
> - int thread_num = thread_data->thread_num;
> - unsigned int type = flags & ~VCOMP_DYNAMIC_FLAGS_INCREMENT;
> -
> - TRACE("(%u, %s, %s, %s, %s)\n", flags, wine_dbgstr_longlong(first), wine_dbgstr_longlong(last),
> -
>        wine_dbgstr_longlong(step), wine_dbgstr_longlong(chunksize));
> -
> - if (step <= 0)
> - {
> -
>    thread_data->dynamic_type = 0;
> -
>    return;
> - }
> -
> - if (flags & VCOMP_DYNAMIC_FLAGS_INCREMENT)
> -
>    iterations = 1 + (last - first) / step;
> - else
> - {
> -
>    iterations = 1 + (first - last) / step;
> -
>    step *= -1;
> - }
> -
> - if (type == VCOMP_DYNAMIC_FLAGS_STATIC)
> - {
> -
>    per_thread = iterations / num_threads;
> -
>    remaining  = iterations - per_thread * num_threads;
> -
> -
>    if (thread_num < remaining)
> -
>        per_thread++;
> -
>    else if (per_thread)
> -
>        first += remaining * step;
> -
>    else
> -
>    {
> -
>        thread_data->dynamic_type = 0;
> -
>        return;
> -
>    }
> -
> -
>    thread_data->dynamic_type      = VCOMP_DYNAMIC_FLAGS_STATIC;
> -
>    thread_data->dynamic_begin_i8  = first + per_thread * thread_num * step;
> -
>    thread_data->dynamic_end_i8    = thread_data->dynamic_begin_i8 + (per_thread - 1) * step;
> - }
> - else
> - {
> -
>    if (type != VCOMP_DYNAMIC_FLAGS_CHUNKED &&
> -
>        type != VCOMP_DYNAMIC_FLAGS_GUIDED)
> -
>    {
> -
>        FIXME("unsupported flags %u\n", flags);
> -
>        type = VCOMP_DYNAMIC_FLAGS_GUIDED;
> -
>    }
> -
> -
>    EnterCriticalSection(&vcomp_section);
> -
>    thread_data->dynamic++;
> -
>    thread_data->dynamic_type = type;
> -
>    if ((int)(thread_data->dynamic - task_data->dynamic) > 0)
> -
>    {
> -
>        task_data->dynamic                = thread_data->dynamic;
> -
>        task_data->dynamic_first_i8       = first;
> -
>        task_data->dynamic_last_i8        = last;
> -
>        task_data->dynamic_iterations_i8  = iterations;
> -
>        task_data->dynamic_step_i8        = step;
> -
>        task_data->dynamic_chunksize_i8   = chunksize;
> -
>    }
> -
>    LeaveCriticalSection(&vcomp_section);
> - }
>
> +}
> -

Could we share any (or all) of this code with the 32-bit version?