From patchwork Fri Aug 28 15:53:44 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aldy Hernandez X-Patchwork-Id: 511939 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 80ABB14018C for ; Sat, 29 Aug 2015 01:54:03 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=sDwcnWF5; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=U0nrXgRYuqejZJZDX Y5RUX0Lecv+HmDejiT430zBQHg4q6Vlz0u+jybbbGxxL1R2Ch/52FZ0fOj7LuUjP cu8NtH94NK76PHjdc8ELmQCaWna+wZTq9yEP2VxeSGTaUCjRZJKUiqLN0M+xjvsf PTPgFhJQc86Mi3jaofm1DRtmNk= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=nfJhTGrBr5sZv5Ms3tyY4O3 BlZo=; b=sDwcnWF5UU3VTAh2b/6d61UaYZzkNxIMMdiQKy7ea7lSTdStGm5dhvO NiAHF/jUVcA7OWqHjxTfIc+YKvJNi2jXT1J2wZ6tyzLtjWDSiZZGLi1skdqWCdzi VH07tOfKyBBN/tsgz2MTzE6/f9cKtnT8bKPFKRwA/mM6/QbOcjAI= Received: (qmail 97729 invoked by alias); 28 Aug 2015 15:53:53 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 97715 invoked by uid 89); 28 Aug 2015 15:53:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-6.2 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 28 Aug 2015 15:53:47 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 40DE48E74C for ; Fri, 28 Aug 2015 15:53:46 +0000 (UTC) Received: from reynosa.quesejoda.com (vpn-56-53.rdu2.redhat.com [10.10.56.53]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t7SFrjeA029185; Fri, 28 Aug 2015 11:53:45 -0400 Subject: Re: [gomp4.1] document more structures in libgomp.h To: Jakub Jelinek References: <55E06E7A.5000002@redhat.com> <20150828143153.GN9425@tucnak.redhat.com> Cc: gcc-patches From: Aldy Hernandez Message-ID: <55E08408.9050208@redhat.com> Date: Fri, 28 Aug 2015 08:53:44 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <20150828143153.GN9425@tucnak.redhat.com> On 08/28/2015 07:31 AM, Jakub Jelinek wrote: > On Fri, Aug 28, 2015 at 07:21:46AM -0700, Aldy Hernandez wrote: >> * libgomp.h: Document gomp_task_depend_entry, gomp_task, >> gomp_taskgroup. >> *task.c (gomp_task_run_pre): Add comments. > > Missing space before task.c. > >> --- a/libgomp/libgomp.h >> +++ b/libgomp/libgomp.h >> @@ -279,9 +279,11 @@ struct htab; >> >> struct gomp_task_depend_entry >> { >> + /* Address of dependency. */ >> void *addr; >> struct gomp_task_depend_entry *next; >> struct gomp_task_depend_entry *prev; >> + /* Task that provides the depdency in ADDR. */ > > Typo. > >> struct gomp_task *task; >> /* Depend entry is of type "IN". */ >> bool is_in; >> @@ -312,10 +314,14 @@ struct gomp_taskwait >> struct gomp_task >> { >> struct gomp_task *parent; >> + /* Children of this task. Siblings are chained by >> + NEXT/PREV_CHILD fields below. */ > > I think it would be better to say here that it is a circular list, > and how the siblings are ordered in the circular list. > >> struct gomp_taskgroup >> { >> struct gomp_taskgroup *prev; >> + /* List of tasks that belong in this taskgroup. Tasks are chained >> + by next/prev_taskgroup within the gomp_task. */ > > Again, perhaps mention also that it is a circular list and how the items of > the circular list are sorted. > >> + /* Scheduled tasks. Chain fields are next/prev_queue within a >> + gomp_task. */ > > Similarly. >> struct gomp_task *task_queue; > > > Jakub > How about this? Aldy commit 9647978b5a05dfb851e2b61704187dce71ffe379 Author: Aldy Hernandez Date: Fri Aug 28 07:19:51 2015 -0700 * libgomp.h: Document gomp_task_depend_entry, gomp_task, gomp_taskgroup. *task.c (gomp_task_run_pre): Add comments. (gomp_task_run_post_handle_dependers): Same. (GOMP_taskwait): Same. diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h index f855813..7babf04 100644 --- a/libgomp/libgomp.h +++ b/libgomp/libgomp.h @@ -279,9 +279,11 @@ struct htab; struct gomp_task_depend_entry { + /* Address of dependency. */ void *addr; struct gomp_task_depend_entry *next; struct gomp_task_depend_entry *prev; + /* Task that provides the dependency in ADDR. */ struct gomp_task *task; /* Depend entry is of type "IN". */ bool is_in; @@ -311,22 +313,35 @@ struct gomp_taskwait struct gomp_task { + /* Parent circular list. See children description below. */ struct gomp_task *parent; + /* Circular list representing the children of this task. + + In this list we first have parent_depends_on ready to run tasks, + then !parent_depends_on ready to run tasks, then already running + tasks, and finally the rest of the tasks. */ struct gomp_task *children; struct gomp_task *next_child; struct gomp_task *prev_child; + /* Circular task_queue in `struct gomp_team'. + + GOMP_TASK_WAITING tasks come before GOMP_TASK_TIED tasks. */ struct gomp_task *next_queue; struct gomp_task *prev_queue; - /* Next task in the current taskgroup. */ + /* Circular queue in gomp_taskgroup->children. + + GOMP_TASK_WAITING tasks come before GOMP_TASK_TIED tasks. */ struct gomp_task *next_taskgroup; - /* Previous task in the current taskgroup. */ struct gomp_task *prev_taskgroup; /* Taskgroup this task belongs in. */ struct gomp_taskgroup *taskgroup; + /* Tasks that depend on this task. */ struct gomp_dependers_vec *dependers; struct htab *depend_hash; struct gomp_taskwait *taskwait; + /* Number of items in DEPEND. */ size_t depend_count; + /* Number of tasks in the DEPENDERS field above. */ size_t num_dependees; struct gomp_task_icv icv; void (*fn) (void *); @@ -335,13 +350,23 @@ struct gomp_task bool in_tied_task; bool final_task; bool copy_ctors_done; + /* Set for undeferred tasks with unsatisfied dependencies which + block further execution of their parent until the dependencies + are satisfied. */ bool parent_depends_on; + /* Dependencies provided and/or needed for this task. DEPEND_COUNT + is the number of items available. */ struct gomp_task_depend_entry depend[]; }; struct gomp_taskgroup { struct gomp_taskgroup *prev; + /* Circular list of tasks that belong in this taskgroup. + + Tasks are chained by next/prev_taskgroup within gomp_task, and + are sorted by GOMP_TASK_WAITING tasks, and then GOMP_TASK_TIED + tasks. */ struct gomp_task *children; bool in_taskgroup_wait; bool cancelled; @@ -411,6 +436,8 @@ struct gomp_team struct gomp_work_share work_shares[8]; gomp_mutex_t task_lock; + /* Scheduled tasks. Chain fields are next/prev_queue within a + gomp_task. */ struct gomp_task *task_queue; /* Number of all GOMP_TASK_{WAITING,TIED} tasks in the team. */ unsigned int task_count; diff --git a/libgomp/task.c b/libgomp/task.c index aa7ae4d..179e0fa 100644 --- a/libgomp/task.c +++ b/libgomp/task.c @@ -463,14 +463,26 @@ gomp_task_run_pre (struct gomp_task *child_task, struct gomp_task *parent, if (parent->children == child_task) parent->children = child_task->next_child; + /* If the current task (child_task) is at the top of the + parent's last_parent_depends_on, it's about to be removed + from it. Adjust last_parent_depends_on appropriately. */ if (__builtin_expect (child_task->parent_depends_on, 0) && parent->taskwait->last_parent_depends_on == child_task) { + /* The last_parent_depends_on list was built with all + parent_depends_on entries linked to the prev_child. Grab + the next last_parent_depends_on head from this prev_child if + available... */ if (child_task->prev_child->kind == GOMP_TASK_WAITING && child_task->prev_child->parent_depends_on) parent->taskwait->last_parent_depends_on = child_task->prev_child; else - parent->taskwait->last_parent_depends_on = NULL; + { + /* ...otherwise, there are no more parent_depends_on + entries waiting to run. In which case, clear the + list. */ + parent->taskwait->last_parent_depends_on = NULL; + } } } @@ -529,6 +541,11 @@ gomp_task_run_post_handle_depend_hash (struct gomp_task *child_task) } } +/* After CHILD_TASK has been run, adjust the various task queues to + give higher priority to the tasks that depend on CHILD_TASK. + + TEAM is the team to which CHILD_TASK belongs to. */ + static size_t gomp_task_run_post_handle_dependers (struct gomp_task *child_task, struct gomp_team *team) @@ -552,7 +569,7 @@ gomp_task_run_post_handle_dependers (struct gomp_task *child_task, if (parent->taskwait && parent->taskwait->last_parent_depends_on && !task->parent_depends_on) { - /* Put task in last_parent_depends_on. */ + /* Put depender in last_parent_depends_on. */ struct gomp_task *last_parent_depends_on = parent->taskwait->last_parent_depends_on; task->next_child = last_parent_depends_on->next_child; @@ -560,7 +577,8 @@ gomp_task_run_post_handle_dependers (struct gomp_task *child_task, } else { - /* Put task at the top of the sibling list. */ + /* Make depender a sibling of child_task, and place + it at the top of said sibling list. */ task->next_child = parent->children; task->prev_child = parent->children->prev_child; parent->children = task; @@ -570,7 +588,7 @@ gomp_task_run_post_handle_dependers (struct gomp_task *child_task, } else { - /* Put task in the sibling list. */ + /* Make depender a sibling of child_task. */ task->next_child = task; task->prev_child = task; parent->children = task; @@ -592,6 +610,8 @@ gomp_task_run_post_handle_dependers (struct gomp_task *child_task, parent->taskwait->last_parent_depends_on = task; } } + /* If depender is in a taskgroup, put it at the TOP of its + taskgroup. */ if (taskgroup) { if (taskgroup->children) @@ -613,6 +633,8 @@ gomp_task_run_post_handle_dependers (struct gomp_task *child_task, gomp_sem_post (&taskgroup->taskgroup_sem); } } + /* Put depender of child_task at the END of the team's + task_queue. */ if (team->task_queue) { task->next_queue = team->task_queue; @@ -829,7 +851,9 @@ gomp_barrier_handle_tasks (gomp_barrier_state_t state) } } -/* Called when encountering a taskwait directive. */ +/* Called when encountering a taskwait directive. + + Wait for all children of the current task. */ void GOMP_taskwait (void) @@ -1024,6 +1048,10 @@ gomp_task_maybe_wait_for_dependencies (void **depend) tsk->prev_child->next_child = tsk; tsk->next_child->prev_child = tsk; } + else + { + /* It's already in task->children. Nothing to do. */; + } last_parent_depends_on = tsk; } }