- add a test for two nested parallel sections

- various fixes in parallel sections handling

include/starpu_openmp.h

@@ -51,7 +51,7 @@ extern "C"
 
 extern int starpu_omp_init(void) __STARPU_OMP_NOTHROW;
 extern void starpu_omp_shutdown(void) __STARPU_OMP_NOTHROW;
-void starpu_parallel_region(struct starpu_codelet *parallel_region_cl, void *parallel_region_cl_arg) __STARPU_OMP_NOTHROW;
+void starpu_parallel_region(const struct starpu_codelet * const parallel_region_cl, void * const parallel_region_cl_arg) __STARPU_OMP_NOTHROW;
 
 extern void starpu_omp_set_num_threads(int threads) __STARPU_OMP_NOTHROW;
 extern int starpu_omp_get_num_threads() __STARPU_OMP_NOTHROW;

src/util/openmp_runtime_support.c

@@ -72,7 +72,6 @@ static struct starpu_omp_region *create_omp_region_struct(struct starpu_omp_regi
 		_STARPU_ERROR("memory allocation failed");
 
 	region->parent_region = parent_region;
-	region->initial_nested_region = NULL;
 	region->owner_device = owner_device;
 	region->thread_list = starpu_omp_thread_list_new();
 	region->implicit_task_list = starpu_omp_task_list_new();
@@ -89,7 +88,6 @@ static void destroy_omp_region_struct(struct starpu_omp_region *region)
 	STARPU_ASSERT(starpu_omp_thread_list_empty(region->thread_list));
 	STARPU_ASSERT(starpu_omp_task_list_empty(region->implicit_task_list));
 	STARPU_ASSERT(region->continuation_starpu_task == NULL);
-	STARPU_ASSERT(region->initial_nested_region == NULL);
 	starpu_omp_thread_list_delete(region->thread_list);
 	starpu_omp_task_list_delete(region->implicit_task_list);
 	memset(region, 0, sizeof(*region));
@@ -98,10 +96,9 @@ static void destroy_omp_region_struct(struct starpu_omp_region *region)
 
 static void omp_initial_thread_func(void)
 {
-	struct starpu_omp_region *init_region = _global_state.initial_region;
 	struct starpu_omp_thread *init_thread = _global_state.initial_thread;
 	struct starpu_omp_task *init_task = _global_state.initial_task;
-	struct starpu_task *continuation_task = init_region->initial_nested_region->continuation_starpu_task;
+	struct starpu_task *continuation_starpu_task = init_task->nested_region->continuation_starpu_task;
 	while (1)
 	{
 		starpu_driver_run_once(&init_thread->starpu_driver);
@@ -110,9 +107,9 @@ static void omp_initial_thread_func(void)
 		 * if we are leaving the first nested region we give control back to initial task
 		 * otherwise, we should continue to execute work
 		 */
-		if (_starpu_task_test_termination(continuation_task))
+		if (_starpu_task_test_termination(continuation_starpu_task))
 		{
-			init_region->initial_nested_region->continuation_starpu_task = NULL;
+			init_task->nested_region->continuation_starpu_task = NULL;
 			swapcontext(&init_thread->ctx, &init_task->ctx);
 		}
 	}
@@ -134,6 +131,7 @@ static struct starpu_omp_thread *create_omp_thread_struct(struct starpu_omp_regi
 	/* .ctx */
 	memset(&thread->ctx, 0, sizeof(thread->ctx));
 	/* .starpu_driver will be initialized later on */
+	/* .starpu_worker_id will be initialized later on */
 	return thread;
 }
 
@@ -261,6 +259,7 @@ static struct starpu_omp_task *create_omp_task_struct(struct starpu_omp_task *pa
 	task->parent_task = parent_task;
 	task->owner_thread = owner_thread;
 	task->owner_region = owner_region;
+	task->nested_region = NULL;
 	task->is_implicit = is_implicit;
 	/* TODO: initialize task->data_env_icvs with proper values */ 
 	memset(&task->data_env_icvs, 0, sizeof(task->data_env_icvs));
@@ -304,6 +303,7 @@ static struct starpu_omp_task *create_omp_task_struct(struct starpu_omp_task *pa
 static void destroy_omp_task_struct(struct starpu_omp_task *task)
 {
 	STARPU_ASSERT(task->state == starpu_omp_task_state_terminated);
+	STARPU_ASSERT(task->nested_region == NULL);
 	STARPU_ASSERT(task->starpu_task == NULL);
 	if (task->stack != NULL)
 	{
@@ -341,6 +341,7 @@ static void omp_initial_thread_setup(void)
 	 * we configure starpu to not launch CPU worker 0
 	 * because we will use the main thread to play the role of worker 0
 	 */
+	initial_thread->starpu_worker_id = 0;
 	struct starpu_conf conf;
 	int ret = starpu_conf_init(&conf);
 	STARPU_CHECK_RETURN_VALUE(ret, "starpu_conf_init");
@@ -431,8 +432,10 @@ void starpu_omp_shutdown(void)
 	STARPU_PTHREAD_KEY_DELETE(omp_thread_key);
 }
 
-void starpu_parallel_region(struct starpu_codelet *parallel_region_cl, void *parallel_region_cl_arg)
+void starpu_parallel_region(const struct starpu_codelet * const _parallel_region_cl,
+		void * const parallel_region_cl_arg)
 {
+	struct starpu_codelet parallel_region_cl = *_parallel_region_cl;
 	struct starpu_omp_thread *master_thread = STARPU_PTHREAD_GETSPECIFIC(omp_thread_key);
 	struct starpu_omp_task *parent_task = STARPU_PTHREAD_GETSPECIFIC(omp_task_key);
 	struct starpu_omp_region *parent_region = parent_task->owner_region;
@@ -460,6 +463,16 @@ void starpu_parallel_region(struct starpu_codelet *parallel_region_cl, void *par
 		{
 			/* TODO: specify actual starpu worker */
 			new_thread = create_omp_thread_struct(new_region);
+
+			/* TODO: use a less arbitrary thread/worker mapping scheme */
+			if (parent_region->level == 0)
+			{
+				new_thread->starpu_worker_id = i;
+			}
+			else
+			{
+				new_thread->starpu_worker_id = master_thread->starpu_worker_id;
+			}
 			starpu_omp_thread_list_push_back(new_region->thread_list, new_thread);
 		}
 
@@ -481,9 +494,10 @@ void starpu_parallel_region(struct starpu_codelet *parallel_region_cl, void *par
 
 		/* in that case, the continuation starpu task is only used for synchronisation */
 		new_region->continuation_starpu_task->cl = NULL;
+		new_region->continuation_starpu_task->workerid = master_thread->starpu_worker_id;
+		new_region->continuation_starpu_task->execute_on_a_specific_worker = 1;
 		/* this sync task will be tested for completion in omp_initial_thread_func() */
 		new_region->continuation_starpu_task->detach = 0;
-		parent_region->initial_nested_region = new_region;
 
 	}
 	else
@@ -492,18 +506,19 @@ void starpu_parallel_region(struct starpu_codelet *parallel_region_cl, void *par
 		_starpu_task_prepare_for_continuation();
 		new_region->continuation_starpu_task = parent_task->starpu_task;
 	}
+	parent_task->nested_region = new_region;
 
 	/*
 	 * save pointer to the regions user function from the parallel region codelet
 	 *
 	 * TODO: add support for multiple/heterogeneous implementations
 	 */
-	void (*parallel_region_f)(void **starpu_buffers, void *starpu_cl_arg) = parallel_region_cl->cpu_funcs[0];
+	void (*parallel_region_f)(void **starpu_buffers, void *starpu_cl_arg) = parallel_region_cl.cpu_funcs[0];
 
 	/*
 	 * plug the task wrapper into the parallel region codelet instead, to support task preemption
 	 */
-	parallel_region_cl->cpu_funcs[0] = starpu_omp_task_exec;
+	parallel_region_cl.cpu_funcs[0] = starpu_omp_task_exec;
 
 	/*
 	 * create the starpu tasks for the implicit omp tasks,
@@ -517,9 +532,11 @@ void starpu_parallel_region(struct starpu_codelet *parallel_region_cl, void *par
 		implicit_task->f = parallel_region_f;
 
 		implicit_task->starpu_task = starpu_task_create();
-		implicit_task->starpu_task->cl = parallel_region_cl;
+		implicit_task->starpu_task->cl = &parallel_region_cl;
 		implicit_task->starpu_task->cl_arg = parallel_region_cl_arg;
 		implicit_task->starpu_task->omp_task = implicit_task;
+		implicit_task->starpu_task->workerid = implicit_task->owner_thread->starpu_worker_id;
+		implicit_task->starpu_task->execute_on_a_specific_worker = 1;
 		starpu_task_declare_deps_array(new_region->continuation_starpu_task, 1, &implicit_task->starpu_task);
 	}
 
@@ -547,7 +564,15 @@ void starpu_parallel_region(struct starpu_codelet *parallel_region_cl, void *par
 	 * preempt for completion of the region
 	 */
 	starpu_omp_task_preempt();
-
+	if (parent_task == _global_state.initial_task)
+	{
+		STARPU_ASSERT(new_region->continuation_starpu_task == NULL);
+	}
+	else
+	{
+		STARPU_ASSERT(new_region->continuation_starpu_task != NULL);
+		new_region->continuation_starpu_task = NULL;
+	}
 	/*
 	 * TODO: free region resources
 	 */
@@ -567,11 +592,7 @@ void starpu_parallel_region(struct starpu_codelet *parallel_region_cl, void *par
 		destroy_omp_task_struct(implicit_task);
 	}
 	STARPU_ASSERT(new_region->nb_threads == 0);
-
-	if (parent_task == _global_state.initial_task)
-	{
-		parent_region->initial_nested_region = NULL;
-	}
+	parent_task->nested_region = NULL;
 	destroy_omp_region_struct(new_region);
 }
 

src/util/openmp_runtime_support.h

@@ -173,6 +173,7 @@ LIST_TYPE(starpu_omp_task,
 	struct starpu_omp_task *parent_task;
 	struct starpu_omp_thread *owner_thread;
 	struct starpu_omp_region *owner_region;
+	struct starpu_omp_region *nested_region;
 	int is_implicit;
 	struct starpu_omp_data_environment_icvs data_env_icvs;
 	struct starpu_omp_implicit_task_icvs implicit_task_icvs;
@@ -221,13 +222,12 @@ LIST_TYPE(starpu_omp_thread,
 	ucontext_t ctx;
 
 	struct starpu_driver starpu_driver;
+	unsigned starpu_worker_id;
 )
 
 struct starpu_omp_region
 {
 	struct starpu_omp_region *parent_region;
-	/* the first nested region of the initial region */
-	struct starpu_omp_region *initial_nested_region;
 	struct starpu_omp_device *owner_device;
 	struct starpu_omp_thread *master_thread;
 	/* note: the list of threads does not include the master_thread */

tests/Makefile.am

@@ -223,6 +223,7 @@ noinst_PROGRAMS =				\
 	openmp/init_exit_02			\
 	openmp/environment			\
 	openmp/parallel_01			\
+	openmp/parallel_02			\
 	overlap/overlap				\
 	overlap/gpu_concurrency			\
 	parallel_tasks/explicit_combined_worker	\
@@ -445,6 +446,9 @@ openmp_environment_SOURCES = 	\
 openmp_parallel_01_SOURCES = 	\
 	openmp/parallel_01.c
 
+openmp_parallel_02_SOURCES = 	\
+	openmp/parallel_02.c
+
 ###################
 # Block interface #
 ###################

tests/openmp/parallel_02.c

@@ -0,0 +1,86 @@
+/* StarPU --- Runtime system for heterogeneous multicore architectures.
+ *
+ * Copyright (C) 2014  Inria
+ *
+ * StarPU is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or (at
+ * your option) any later version.
+ *
+ * StarPU is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ * See the GNU Lesser General Public License in COPYING.LGPL for more details.
+ */
+
+#include <pthread.h>
+#include <starpu.h>
+#include "../helper.h"
+#include <stdio.h>
+
+#if !defined(STARPU_OPENMP)
+int main(int argc, char **argv)
+{
+	return STARPU_TEST_SKIPPED;
+}
+#else
+__attribute__((constructor))
+static void omp_constructor(void)
+{
+	int ret = starpu_omp_init();
+	STARPU_CHECK_RETURN_VALUE(ret, "starpu_omp_init");
+}
+
+__attribute__((destructor))
+static void omp_destructor(void)
+{
+	starpu_omp_shutdown();
+}
+
+void parallel_region_2_f(void *buffers[], void *args)
+{
+	(void) buffers;
+	(void) args;
+	int worker_id;
+	pthread_t tid;
+	tid = pthread_self();
+	worker_id = starpu_worker_get_id();
+	printf("[tid %p] parallel region 2: task thread = %d\n", (void *)tid, worker_id);
+}
+
+static struct starpu_codelet parallel_region_2_cl =
+{
+	.cpu_funcs    = { parallel_region_2_f, NULL },
+	.where        = STARPU_CPU,
+	.nbuffers     = 0
+
+};
+
+void parallel_region_1_f(void *buffers[], void *args)
+{
+	(void) buffers;
+	(void) args;
+	int worker_id;
+	pthread_t tid;
+	tid = pthread_self();
+	worker_id = starpu_worker_get_id();
+	printf("[tid %p] parallel region 1: task thread = %d\n", (void *)tid, worker_id);
+
+	starpu_parallel_region(&parallel_region_2_cl, NULL);
+}
+
+static struct starpu_codelet parallel_region_1_cl =
+{
+	.cpu_funcs    = { parallel_region_1_f, NULL },
+	.where        = STARPU_CPU,
+	.nbuffers     = 0
+
+};
+
+int
+main (int argc, char *argv[]) {
+	starpu_parallel_region(&parallel_region_1_cl, NULL);
+	return 0;
+}
+#endif