On Mon, Feb 22, 2021 at 02:49:44PM +0100, Jakub Jelinek wrote:
> So, I think for the team != gomp_thread ()->ts.team
> && !do_wake
> && gomp_team_barrier_waiting_for_tasks (&team->barrier)
> && team->task_detach_count == 0
> case, we need to wake up 1 thread anyway and arrange for it to do:
> gomp_team_barrier_done (&team->barrier, state);
> gomp_mutex_unlock (&team->task_lock);
> gomp_team_barrier_wake (&team->barrier, 0);
> Possibly in
> if (!priority_queue_empty_p (&team->task_queue, MEMMODEL_RELAXED))
> add
> else if (team->task_count == 0
> && gomp_team_barrier_waiting_for_tasks (&team->barrier))
> {
> gomp_team_barrier_done (&team->barrier, state);
> gomp_mutex_unlock (&team->task_lock);
> gomp_team_barrier_wake (&team->barrier, 0);
> if (to_free)
> {
> gomp_finish_task (to_free);
> free (to_free);
> }
> return;
> }
> but the:
> if (--team->task_count == 0
> && gomp_team_barrier_waiting_for_tasks (&team->barrier))
> {
> gomp_team_barrier_done (&team->barrier, state);
> gomp_mutex_unlock (&team->task_lock);
> gomp_team_barrier_wake (&team->barrier, 0);
> gomp_mutex_lock (&team->task_lock);
> }
> in that case would then be incorrect, we don't want to do that twice.
> So, either that second if would need to do the to_free handling
> and return instead of taking the lock again and looping, or
> perhaps we can just do
> --team->task_count;
> there instead and let the above added else if handle that?
FYI, I've just tested that part of change alone whether it doesn't break
anything else and it worked fine:
diff --git a/libgomp/task.c b/libgomp/task.c
index b242e7c8d20..9c27c3b5148 100644
--- a/libgomp/task.c
+++ b/libgomp/task.c
@@ -1405,6 +1405,19 @@ gomp_barrier_handle_tasks (gomp_barrier_state_t state)
team->task_running_count++;
child_task->in_tied_task = true;
}
+ else if (team->task_count == 0
+ && gomp_team_barrier_waiting_for_tasks (&team->barrier))
+ {
+ gomp_team_barrier_done (&team->barrier, state);
+ gomp_mutex_unlock (&team->task_lock);
+ gomp_team_barrier_wake (&team->barrier, 0);
+ if (to_free)
+ {
+ gomp_finish_task (to_free);
+ free (to_free);
+ }
+ return;
+ }
gomp_mutex_unlock (&team->task_lock);
if (do_wake)
{
@@ -1479,14 +1492,7 @@ gomp_barrier_handle_tasks (gomp_barrier_state_t state)
if (do_wake > new_tasks)
do_wake = new_tasks;
}
- if (--team->task_count == 0
- && gomp_team_barrier_waiting_for_tasks (&team->barrier))
- {
- gomp_team_barrier_done (&team->barrier, state);
- gomp_mutex_unlock (&team->task_lock);
- gomp_team_barrier_wake (&team->barrier, 0);
- gomp_mutex_lock (&team->task_lock);
- }
+ --team->task_count;
}
}
}
Jakub
