On 12/05/2026 11:42, Arsen Arsenović wrote:
Andrew Stubbs <[email protected]> writes:
On 05/05/2026 14:14, Arsen Arsenović wrote:
+ /* TODO(arsen): This should be part of a mechanism that allows us to override
+ nthreads-var with OMP_NUM_THREADS. But, we currently don't have access to
+ that list on the device.
+
+ thr->task->icv.nthreads_var = ...; */
The previous code did write to this field. Why does the new thread not do at
least the same? (This does not seem like "no functional changes".)
This write was redundant with gomp_init_task, which does:
/* task = thr->task, prev_icv = &start_data->prev_icvs */
gomp_init_task (struct gomp_task *task, struct gomp_task *parent_task,
struct gomp_task_icv *prev_icv)
/* [...] */
task->icv = *prev_icv;
The generic team.c implementation does (abridged):
nthreads_var = icv->nthreads_var;
if (__builtin_expect (gomp_nthreads_var_list != NULL, 0)
&& thr->ts.level < gomp_nthreads_var_list_len)
nthreads_var = gomp_nthreads_var_list[thr->ts.level];
gomp_init_task (thr->task, task, icv);
thr->task->taskgroup = taskgroup;
team->implicit_task[0].icv.nthreads_var = nthreads_var;
... i.e. it allows replacing the value of nthreads_var with an element
of gomp_nthreads_var_list, before writing it back.
On the device, however, that list is always empty, and analogous logic
was not even present, so the value of nthreads_var is necessarily the
same as prev_icv->nthreads_var, being written into a field where
prev_icv->nthreads_var was already copied.
So, it never had an effect.
The patch is OK if you note something about this in the description.
Thanks.
Andrew
