[gomp4.1] document more structures in libgomp.h
diff mbox

Message ID 55E06E7A.5000002@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez Aug. 28, 2015, 2:21 p.m. UTC
More boring patches in an effort to make sense of it all.

Does this match your understanding?  If it does, OK for branch?
commit 3b7ffc815a8e163391c913196160354348348945
Author: Aldy Hernandez <aldyh@redhat.com>
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.

Comments

Jakub Jelinek Aug. 28, 2015, 2:31 p.m. UTC | #1
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

Patch
diff mbox

diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index f855813..2357357 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 depdency in ADDR.  */
   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.  */
   struct gomp_task *children;
   struct gomp_task *next_child;
   struct gomp_task *prev_child;
+  /* Next task in TASK_QUEUE list in `struct gomp_team'.  */
   struct gomp_task *next_queue;
+  /* Prev task in TASK_QUEUE list in `struct gomp_team'.  */
   struct gomp_task *prev_queue;
   /* Next task in the current taskgroup.  */
   struct gomp_task *next_taskgroup;
@@ -323,10 +329,13 @@  struct gomp_task
   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 +344,20 @@  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;
+  /* List of tasks that belong in this taskgroup.  Tasks are chained
+     by next/prev_taskgroup within the gomp_task.  */
   struct gomp_task *children;
   bool in_taskgroup_wait;
   bool cancelled;
@@ -411,6 +427,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;
 		  }
 	      }