[gomp4.1] document more things
diff mbox

Message ID 55DF24EB.5090605@redhat.com
State New
Headers show

Commit Message

Aldy Hernandez Aug. 27, 2015, 2:55 p.m. UTC
On 08/27/2015 07:14 AM, Jakub Jelinek wrote:
> On Thu, Aug 27, 2015 at 07:01:48AM -0700, Aldy Hernandez wrote:
>> commit 45fdbc84123fefde6bf99aaf87099317aaa38f24
>> Author: Aldy Hernandez <aldyh@redhat.com>
>> Date:   Wed Aug 26 07:28:39 2015 -0700
>>
>>      	* libgomp.h (enum gomp_task_kind): Comment fields.
>>      	(struct gomp_task): Comment some fields.
>>      	* task.c (GOMP_task): Add comments.
>>      	(gomp_task_run_pre): Same.
>>
>> diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
>> index 3d705ef..34f064c 100644
>> --- a/libgomp/libgomp.h
>> +++ b/libgomp/libgomp.h
>> @@ -263,9 +263,13 @@ extern char *goacc_device_type;
>>
>>   enum gomp_task_kind
>>   {
>> +  /* Thread not directly created by user.  For instance, when creating
>> +     a team of threads or an ICV.  */
>
> That is misleading, especially with the (possibly) upcoming thread task.
> "implicit task" is an OpenMP tasking term:

Ah, fixed.

> "A task generated by an implicit parallel region or generated when a parallel
> construct is encountered during execution."
> but there is really no reason to explain what it is, so you should just say
> in the comment
>    /* Implicit task.  */
>
>>     GOMP_TASK_IMPLICIT,
>>     GOMP_TASK_IFFALSE,
>
> This is for if (0) tasks.  The right OpenMP tasking term is "undeferred
> task", perhaps we should just replace GOMP_TASK_IFFALSE with
> GOMP_TASK_UNDEFERRED everywhere.

Done.

>
>> +  /* Task created by GOMP_task and waiting to be run.  */
>>     GOMP_TASK_WAITING,
>> +  /* Task currently scheduled and about to execute.  */
>
> Well, not just about to execute, that kind is used even for the task that is
> already executing.

Fixed.

>
> Otherwise LGTM.

Committed.
commit 862e81ee2d71f33eb09041061d3b1db1671495f8
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Wed Aug 26 07:28:39 2015 -0700

    	* libgomp.h (enum gomp_task_kind): Comment fields.
    	(struct gomp_task): Comment some fields.
    	* task.c (GOMP_task): Add comments.
    	Rename GOMP_TASK_IFFALSE to GOMP_TASK_UNDEFERRED.
    	(gomp_task_run_pre): Add comments.
    	(GOMP_taskgroup_start): Rename GOMP_TASK_IFFALSE to
    	GOMP_TASK_UNDEFERRED.
    	* taskloop.c (GOMP_taskloop): Same.

Patch
diff mbox

diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index 3d705ef..f855813 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -263,9 +263,13 @@  extern char *goacc_device_type;
 
 enum gomp_task_kind
 {
+  /* Implicit task.  */
   GOMP_TASK_IMPLICIT,
-  GOMP_TASK_IFFALSE,
+  /* Undeferred task.  */
+  GOMP_TASK_UNDEFERRED,
+  /* Task created by GOMP_task and waiting to be run.  */
   GOMP_TASK_WAITING,
+  /* Task currently executing or scheduled and about to execute.  */
   GOMP_TASK_TIED
 };
 
@@ -313,8 +317,11 @@  struct gomp_task
   struct gomp_task *prev_child;
   struct gomp_task *next_queue;
   struct gomp_task *prev_queue;
+  /* Next task in the current taskgroup.  */
   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;
   struct gomp_dependers_vec *dependers;
   struct htab *depend_hash;
diff --git a/libgomp/task.c b/libgomp/task.c
index 009a9f8..aa7ae4d 100644
--- a/libgomp/task.c
+++ b/libgomp/task.c
@@ -165,7 +165,7 @@  GOMP_task (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
 	gomp_task_maybe_wait_for_dependencies (depend);
 
       gomp_init_task (&task, thr->task, gomp_icv (false));
-      task.kind = GOMP_TASK_IFFALSE;
+      task.kind = GOMP_TASK_UNDEFERRED;
       task.final_task = (thr->task && thr->task->final_task)
 			|| (flags & GOMP_TASK_FLAG_FINAL);
       if (thr->task)
@@ -218,7 +218,7 @@  GOMP_task (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
       arg = (char *) (((uintptr_t) (task + 1) + depend_size + arg_align - 1)
 		      & ~(uintptr_t) (arg_align - 1));
       gomp_init_task (task, parent, gomp_icv (false));
-      task->kind = GOMP_TASK_IFFALSE;
+      task->kind = GOMP_TASK_UNDEFERRED;
       task->in_tied_task = parent->in_tied_task;
       task->taskgroup = taskgroup;
       thr->task = task;
@@ -388,6 +388,7 @@  GOMP_task (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
       parent->children = task;
       if (taskgroup)
 	{
+	  /* If applicable, place task into its taskgroup.  */
 	  if (taskgroup->children)
 	    {
 	      task->next_taskgroup = taskgroup->children;
@@ -451,7 +452,14 @@  gomp_task_run_pre (struct gomp_task *child_task, struct gomp_task *parent,
 {
   if (parent)
     {
-      /* Remove child_task from parent.  */
+      /* Adjust children such that it will point to a next child,
+	 while the current one is scheduled to be executed.  This way,
+	 GOMP_taskwait (and others) can schedule a next task while
+	 waiting.
+
+	 Do not remove it entirely from the circular list, as it is
+	 still a child, though not one we should consider first (say
+	 by GOMP_taskwait).  */
       if (parent->children == child_task)
 	parent->children = child_task->next_child;
 
@@ -465,10 +473,14 @@  gomp_task_run_pre (struct gomp_task *child_task, struct gomp_task *parent,
 	    parent->taskwait->last_parent_depends_on = NULL;
 	}
     }
-  /* Remove child_task from taskgroup.  */
+
+  /* Adjust taskgroup to point to the next taskgroup.  See note above
+     regarding adjustment of children as to why the child_task is not
+     removed entirely from the circular list.  */
   if (taskgroup && taskgroup->children == child_task)
     taskgroup->children = child_task->next_taskgroup;
 
+  /* Remove child_task from the task_queue.  */
   child_task->prev_queue->next_queue = child_task->next_queue;
   child_task->next_queue->prev_queue = child_task->prev_queue;
   if (team->task_queue == child_task)
@@ -479,6 +491,7 @@  gomp_task_run_pre (struct gomp_task *child_task, struct gomp_task *parent,
 	team->task_queue = NULL;
     }
   child_task->kind = GOMP_TASK_TIED;
+
   if (--team->task_queued_count == 0)
     gomp_team_barrier_clear_task_pending (&team->barrier);
   if ((gomp_team_barrier_cancelled (&team->barrier)
@@ -1137,7 +1150,7 @@  GOMP_taskgroup_start (void)
   struct gomp_taskgroup *taskgroup;
 
   /* If team is NULL, all tasks are executed as
-     GOMP_TASK_IFFALSE tasks and thus all children tasks of
+     GOMP_TASK_UNDEFERRED tasks and thus all children tasks of
      taskgroup and their descendant tasks will be finished
      by the time GOMP_taskgroup_end is called.  */
   if (team == NULL)
diff --git a/libgomp/taskloop.c b/libgomp/taskloop.c
index 1a2b85d..f57a5a1 100644
--- a/libgomp/taskloop.c
+++ b/libgomp/taskloop.c
@@ -175,7 +175,7 @@  GOMP_taskloop (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
 	  for (i = 0; i < num_tasks; i++)
 	    {
 	      gomp_init_task (&task[i], parent, gomp_icv (false));
-	      task[i].kind = GOMP_TASK_IFFALSE;
+	      task[i].kind = GOMP_TASK_UNDEFERRED;
 	      task[i].final_task = (thr->task && thr->task->final_task)
 				   || (flags & GOMP_TASK_FLAG_FINAL);
 	      if (thr->task)
@@ -213,7 +213,7 @@  GOMP_taskloop (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
 	    struct gomp_task task;
 
 	    gomp_init_task (&task, thr->task, gomp_icv (false));
-	    task.kind = GOMP_TASK_IFFALSE;
+	    task.kind = GOMP_TASK_UNDEFERRED;
 	    task.final_task = (thr->task && thr->task->final_task)
 			      || (flags & GOMP_TASK_FLAG_FINAL);
 	    if (thr->task)
@@ -254,7 +254,7 @@  GOMP_taskloop (void (*fn) (void *), void *data, void (*cpyfn) (void *, void *),
 	  arg = (char *) (((uintptr_t) (task + 1) + arg_align - 1)
 			  & ~(uintptr_t) (arg_align - 1));
 	  gomp_init_task (task, parent, gomp_icv (false));
-	  task->kind = GOMP_TASK_IFFALSE;
+	  task->kind = GOMP_TASK_UNDEFERRED;
 	  task->in_tied_task = parent->in_tied_task;
 	  task->taskgroup = taskgroup;
 	  thr->task = task;