Browse Source

First modifications to define access modes for data handles into starpu_codelet and no longer in starpu_task

include/starpu_task.h:
	mark (struct starpu_task).buffers as deprecated
	add (struct starpu_task).handles
	add (struct starpu_codelet).modes

src:
	new function starpu_task_check_deprecated_fields() which sets the new defined fields (see above)
	code using buffers has been updated to use handles and modes

TODO:
	todo list (after the xmas holidays!)
Nathalie Furmento 13 years ago
parent
commit
680e312601

+ 25 - 0
TODO

@@ -0,0 +1,25 @@
+
+Moving access modes for data handles from struct starpu_task to struct starpu_codelet
+=====================================================================================
+
+TODO list
+
+- Implement test for checking old functionalities still work (similar to tests/core/deprecated)
+
+- Make struct starpu_buffer_descr private (or not, as it can still be used in tests and examples)
+
+- Update all tests and examples
+
+- Depreciate cost_model field in struct starpu_per_arch_perfmodel
+                                 struct starpu_perfmodel
+
+- Add a field cost_model_new
+  double (*cost_model_new)(starpu_data_handle_t *); /* returns expected duration in µs */
+
+- Update all cost functions in examples
+
+- Set task->buffers to NULL
+
+- When cost_model is provided, but not cost_model_new, need to rebuild a struct starpu_buffer_descr
+
+- Update doc and changelog

+ 4 - 1
include/starpu_task.h

@@ -97,6 +97,8 @@ struct starpu_codelet
 
 	/* how many buffers do the codelet takes as argument ? */
 	unsigned nbuffers;
+	/* which are the access modes for these buffers */
+	enum starpu_access_mode modes[STARPU_NMAXBUFS];
 
 	/* performance model of the codelet */
 	struct starpu_perfmodel *model;
@@ -121,7 +123,8 @@ struct starpu_task
 	struct starpu_codelet *cl;
 
 	/* arguments managed by the DSM */
-	struct starpu_buffer_descr buffers[STARPU_NMAXBUFS];
+	struct starpu_buffer_descr buffers[STARPU_NMAXBUFS] STARPU_DEPRECATED;
+	starpu_data_handle_t handles[STARPU_NMAXBUFS];
 	void *interfaces[STARPU_NMAXBUFS];
 
 	/* arguments not managed by the DSM are given as a buffer */

+ 7 - 1
src/core/dependencies/data_concurrency.c

@@ -235,7 +235,13 @@ unsigned _starpu_submit_job_enforce_data_deps(struct _starpu_job *j)
 	/* Compute an ordered list of the different pieces of data so that we
 	 * grab then according to a total order, thus avoiding a deadlock
 	 * condition */
-	memcpy(j->ordered_buffers, j->task->buffers, cl->nbuffers*sizeof(struct starpu_buffer_descr));
+	unsigned i;
+	for (i=0 ; i<cl->nbuffers ; i++)
+	{
+		j->ordered_buffers[i].handle = j->task->handles[i];
+		j->ordered_buffers[i].mode = j->task->cl->modes[i];
+	}
+
 	_starpu_sort_task_handles(j->ordered_buffers, cl->nbuffers);
 
 	return _submit_job_enforce_data_deps(j, 0);

+ 2 - 2
src/core/dependencies/implicit_data_deps.c

@@ -288,8 +288,8 @@ void _starpu_detect_implicit_data_deps(struct starpu_task *task)
 	unsigned buffer;
 	for (buffer = 0; buffer < nbuffers; buffer++)
 	{
-		starpu_data_handle_t handle = task->buffers[buffer].handle;
-		enum starpu_access_mode mode = task->buffers[buffer].mode;
+		starpu_data_handle_t handle = task->handles[buffer];
+		enum starpu_access_mode mode = task->cl->modes[buffer];
 
 		/* Scratch memory does not introduce any deps */
 		if (mode & STARPU_SCRATCH)

+ 1 - 1
src/core/jobs.c

@@ -38,7 +38,7 @@ size_t _starpu_job_get_data_size(struct _starpu_job *j)
 	unsigned buffer;
 	for (buffer = 0; buffer < nbuffers; buffer++)
 	{
-		starpu_data_handle_t handle = task->buffers[buffer].handle;
+		starpu_data_handle_t handle = task->handles[buffer];
 		size += _starpu_data_get_size(handle);
 	}
 

+ 3 - 3
src/core/perfmodel/perfmodel.c

@@ -251,7 +251,7 @@ double starpu_task_expected_conversion_time(struct starpu_task *task,
 		starpu_data_handle_t handle;
 		struct starpu_task *conversion_task;
 
-		handle = task->buffers[i].handle;
+		handle = task->handles[i];
 		id = starpu_get_handle_interface_id(handle);
 		if (id != STARPU_MULTIFORMAT_INTERFACE_ID)
 			continue;
@@ -312,8 +312,8 @@ double starpu_task_expected_data_transfer_time(uint32_t memory_node, struct star
 
 	for (buffer = 0; buffer < nbuffers; buffer++)
 	{
-		starpu_data_handle_t handle = task->buffers[buffer].handle;
-		enum starpu_access_mode mode = task->buffers[buffer].mode;
+		starpu_data_handle_t handle = task->handles[buffer];
+		enum starpu_access_mode mode = task->cl->modes[buffer];
 
 		penalty += starpu_data_expected_transfer_time(handle, memory_node, mode);
 	}

+ 5 - 5
src/core/sched_policy.c

@@ -245,13 +245,13 @@ static int _starpu_push_task_on_specific_worker(struct starpu_task *task, int wo
 		unsigned node = starpu_worker_get_memory_node(workerid);
 		if (_starpu_task_uses_multiformat_handles(task))
 		{
-			int i;
+			unsigned i;
 			for (i = 0; i < task->cl->nbuffers; i++)
 			{
 				struct starpu_task *conversion_task;
 				starpu_data_handle_t handle;
 
-				handle = task->buffers[i].handle;
+				handle = task->handles[i];
 				if (!_starpu_handle_needs_conversion_task(handle, node))
 					continue;
 
@@ -264,7 +264,7 @@ static int _starpu_push_task_on_specific_worker(struct starpu_task *task, int wo
 			}
 
 			for (i = 0; i < task->cl->nbuffers; i++)
-				task->buffers[i].handle->mf_node = node;
+				task->handles[i]->mf_node = node;
 		}
 		return _starpu_push_local_task(worker, task, 0);
 	}
@@ -438,13 +438,13 @@ pick:
 	 * We do have a task that uses multiformat handles. Let's create the 
 	 * required conversion tasks.
 	 */
-	int i;
+	unsigned i;
 	for (i = 0; i < task->cl->nbuffers; i++)
 	{
 		struct starpu_task *conversion_task;
 		starpu_data_handle_t handle;
 
-		handle = task->buffers[i].handle;
+		handle = task->handles[i];
 		if (!_starpu_handle_needs_conversion_task(handle, node))
 			continue;
 		conversion_task = _starpu_create_conversion_task(handle, node);

+ 48 - 6
src/core/task.c

@@ -280,6 +280,30 @@ void _starpu_codelet_check_deprecated_fields(struct starpu_codelet *cl)
 	}
 }
 
+void _starpu_task_check_deprecated_fields(struct starpu_task *task)
+{
+	if (task->cl)
+	{
+		unsigned i;
+		for(i=0; i<task->cl->nbuffers ; i++)
+		{
+			if (task->buffers[i].handle && task->handles[i])
+			{
+				fprintf(stderr, "[warning][struct starpu_task] task->buffers[%d] and task->handles[%d] both set. Ignoring task->buffers[%d] ?\n", i, i, i);
+				//task->buffers[i].handle = NULL;
+				STARPU_ASSERT(task->buffers[i].mode == task->cl->modes[i]);
+				STARPU_ABORT();
+			}
+			if (task->buffers[i].handle)
+			{
+				task->handles[i] = task->buffers[i].handle;
+				task->cl->modes[i] = task->buffers[i].mode;
+				//task->buffers[i].handle = NULL;
+			}
+		}
+	}
+}
+
 /* application should submit new tasks to StarPU through this function */
 int starpu_task_submit(struct starpu_task *task)
 {
@@ -302,13 +326,13 @@ int starpu_task_submit(struct starpu_task *task)
 		task->detach = 0;
 	}
 
+	_starpu_task_check_deprecated_fields(task);
+	_starpu_codelet_check_deprecated_fields(task->cl);
 
 	if (task->cl)
 	{
 		unsigned i;
 
-		_starpu_codelet_check_deprecated_fields(task->cl);
-
 		/* Check the type of worker(s) required by the task exist */
 		if (!_starpu_worker_exists(task))
 		{
@@ -321,7 +345,7 @@ int starpu_task_submit(struct starpu_task *task)
 		for (i = 0; i < task->cl->nbuffers; i++)
 		{
 			/* Make sure handles are not partitioned */
-			STARPU_ASSERT(task->buffers[i].handle->nchildren == 0);
+			STARPU_ASSERT(task->handles[i]->nchildren == 0);
 		}
 
 		/* In case we require that a task should be explicitely
@@ -377,6 +401,9 @@ int _starpu_task_submit_nodeps(struct starpu_task *task)
 {
 	int ret;
 
+	_starpu_task_check_deprecated_fields(task);
+	_starpu_codelet_check_deprecated_fields(task->cl);
+
 	if (task->cl)
 	{
 		if (task->cl->model)
@@ -396,8 +423,15 @@ int _starpu_task_submit_nodeps(struct starpu_task *task)
 	_starpu_increment_nready_tasks();
 
 	if (task->cl)
+	{
 		/* This would be done by data dependencies checking */
-		memcpy(j->ordered_buffers, j->task->buffers, task->cl->nbuffers*sizeof(struct starpu_buffer_descr));
+		unsigned i;
+		for (i=0 ; i<task->cl->nbuffers ; i++)
+		{
+			j->ordered_buffers[i].handle = j->task->handles[i];
+			j->ordered_buffers[i].mode = j->task->cl->modes[i];
+		}
+	}
 
 	ret = _starpu_push_task(j, 1);
 
@@ -415,6 +449,9 @@ int _starpu_task_submit_conversion_task(struct starpu_task *task,
 	STARPU_ASSERT(task->cl);
 	STARPU_ASSERT(task->execute_on_a_specific_worker);
 
+	_starpu_task_check_deprecated_fields(task);
+	_starpu_codelet_check_deprecated_fields(task->cl);
+
 	/* We should factorize that */
 	if (task->cl->model)
 		_starpu_load_perfmodel(task->cl->model);
@@ -428,7 +465,12 @@ int _starpu_task_submit_conversion_task(struct starpu_task *task,
 	j->submitted = 1;
 	_starpu_increment_nready_tasks();
 
-	memcpy(j->ordered_buffers, j->task->buffers, task->cl->nbuffers*sizeof(struct starpu_buffer_descr));
+	unsigned i;
+	for (i=0 ; i<task->cl->nbuffers ; i++)
+	{
+		j->ordered_buffers[i].handle = j->task->handles[i];
+		j->ordered_buffers[i].mode = j->task->cl->modes[i];
+	}
 
         _STARPU_LOG_IN();
 
@@ -590,7 +632,7 @@ _starpu_task_uses_multiformat_handles(struct starpu_task *task)
 	for (i = 0; i < task->cl->nbuffers; i++)
 	{
 		unsigned int id;
-		id = starpu_get_handle_interface_id(task->buffers[i].handle);
+		id = starpu_get_handle_interface_id(task->handles[i]);
 		if (id == STARPU_MULTIFORMAT_INTERFACE_ID)
 			return 1;
 	}

+ 2 - 0
src/core/task.h

@@ -56,7 +56,9 @@ int _starpu_task_uses_multiformat_handles(struct starpu_task *task);
 int _starpu_task_submit_conversion_task(struct starpu_task *task,
 					unsigned int workerid);
 
+void _starpu_task_check_deprecated_fields(struct starpu_task *task);
 void _starpu_codelet_check_deprecated_fields(struct starpu_codelet *cl);
+
 starpu_cpu_func_t _starpu_task_get_cpu_nth_implementation(struct starpu_codelet *cl, unsigned nimpl);
 starpu_cuda_func_t _starpu_task_get_cuda_nth_implementation(struct starpu_codelet *cl, unsigned nimpl);
 starpu_opencl_func_t _starpu_task_get_opencl_nth_implementation(struct starpu_codelet *cl, unsigned nimpl);

+ 2 - 2
src/core/task_bundle.c

@@ -307,8 +307,8 @@ double starpu_task_bundle_expected_data_transfer_time(struct starpu_task_bundle
 			unsigned b;
 			for (b = 0; b < task->cl->nbuffers; b++)
 			{
-				starpu_data_handle_t handle = task->buffers[b].handle;
-				enum starpu_access_mode mode = task->buffers[b].mode;
+				starpu_data_handle_t handle = task->handles[b];
+				enum starpu_access_mode mode = task->cl->modes[b];
 
 				if (!(mode & STARPU_R))
 					continue;

+ 11 - 15
src/datawizard/coherency.c

@@ -575,14 +575,13 @@ static void _starpu_set_data_requested_flag_if_needed(struct _starpu_data_replic
 
 int starpu_prefetch_task_input_on_node(struct starpu_task *task, uint32_t node)
 {
-	struct starpu_buffer_descr *descrs = task->buffers;
 	unsigned nbuffers = task->cl->nbuffers;
-
 	unsigned index;
+
 	for (index = 0; index < nbuffers; index++)
 	{
-		starpu_data_handle_t handle = descrs[index].handle;
-		enum starpu_access_mode mode = descrs[index].mode;
+		starpu_data_handle_t handle = task->handles[index];
+		enum starpu_access_mode mode = task->cl->modes[index];
 
 		if (mode & (STARPU_SCRATCH|STARPU_REDUX))
 			continue;
@@ -596,10 +595,8 @@ int starpu_prefetch_task_input_on_node(struct starpu_task *task, uint32_t node)
 	return 0;
 }
 
-static struct _starpu_data_replicate *get_replicate(struct starpu_buffer_descr *descr, int workerid, unsigned local_memory_node) {
-	starpu_data_handle_t handle = descr->handle;
-	enum starpu_access_mode mode = descr->mode;
-
+static struct _starpu_data_replicate *get_replicate(starpu_data_handle_t handle, enum starpu_access_mode mode, int workerid, unsigned local_memory_node)
+{
 	if (mode & (STARPU_SCRATCH|STARPU_REDUX))
 		return &handle->per_worker[workerid];
 	else
@@ -638,7 +635,7 @@ int _starpu_fetch_task_input(struct _starpu_job *j, uint32_t mask)
 			 * _starpu_compar_handles */
 			continue;
 
-		local_replicate = get_replicate(&descrs[index], workerid, local_memory_node);
+		local_replicate = get_replicate(handle, mode, workerid, local_memory_node);
 
 		ret = fetch_data(handle, local_replicate, mode);
 		if (STARPU_UNLIKELY(ret))
@@ -646,16 +643,14 @@ int _starpu_fetch_task_input(struct _starpu_job *j, uint32_t mask)
 	}
 
 	/* Now that we have taken the data locks in locking order, fill the codelet interfaces in function order.  */
-	descrs = task->buffers;
-
 	for (index = 0; index < nbuffers; index++)
 	{
-		starpu_data_handle_t handle = descrs[index].handle;
-		enum starpu_access_mode mode = descrs[index].mode;
+		starpu_data_handle_t handle = task->handles[index];
+		enum starpu_access_mode mode = task->cl->modes[index];
 
 		struct _starpu_data_replicate *local_replicate;
 
-		local_replicate = get_replicate(&descrs[index], workerid, local_memory_node);
+		local_replicate = get_replicate(handle, mode, workerid, local_memory_node);
 
 		task->interfaces[index] = local_replicate->data_interface;
 
@@ -702,6 +697,7 @@ void _starpu_push_task_output(struct _starpu_job *j, uint32_t mask)
 	for (index = 0; index < nbuffers; index++)
 	{
 		starpu_data_handle_t handle = descrs[index].handle;
+		enum starpu_access_mode mode = descrs[index].mode;
 
 		struct _starpu_data_replicate *local_replicate;
 
@@ -711,7 +707,7 @@ void _starpu_push_task_output(struct _starpu_job *j, uint32_t mask)
 			 * _starpu_compar_handles */
 			continue;
 
-		local_replicate = get_replicate(&descrs[index], workerid, local_memory_node);
+		local_replicate = get_replicate(handle, mode, workerid, local_memory_node);
 
 		/* In case there was a temporary handle (eg. used for
 		 * reduction), this handle may have requested to be destroyed

+ 1 - 1
src/datawizard/footprint.c

@@ -30,7 +30,7 @@ uint32_t _starpu_compute_buffers_footprint(struct _starpu_job *j)
 
 	for (buffer = 0; buffer < task->cl->nbuffers; buffer++)
 	{
-		starpu_data_handle_t handle = task->buffers[buffer].handle;
+		starpu_data_handle_t handle = task->handles[buffer];
 
 		uint32_t handle_footprint = _starpu_data_get_footprint(handle);
 

+ 2 - 6
src/sched_policies/deque_modeling_policy_data_aware.c

@@ -44,18 +44,14 @@ static long int ready_task_cnt = 0;
 static int count_non_ready_buffers(struct starpu_task *task, uint32_t node)
 {
 	int cnt = 0;
-
-	struct starpu_buffer_descr *descrs = task->buffers;
 	unsigned nbuffers = task->cl->nbuffers;
-
 	unsigned index;
+
 	for (index = 0; index < nbuffers; index++)
 	{
-		struct starpu_buffer_descr *descr;
 		starpu_data_handle_t handle;
 
-		descr = &descrs[index];
-		handle = descr->handle;
+		handle = task->handles[index];
 
 		int is_valid;
 		starpu_data_query_status(handle, node, NULL, &is_valid, NULL);

+ 4 - 3
src/sched_policies/heft.c

@@ -332,7 +332,8 @@ static void compute_all_performance_predictions(struct starpu_task *task,
 
 static int push_conversion_tasks(struct starpu_task *task, unsigned int workerid)
 {
-	int i, ret;
+	unsigned i;
+	int ret;
 	unsigned int node = starpu_worker_get_memory_node(workerid);
 
 	_STARPU_PTHREAD_MUTEX_LOCK(&sched_mutex[workerid]);
@@ -341,7 +342,7 @@ static int push_conversion_tasks(struct starpu_task *task, unsigned int workerid
 		struct starpu_task *conversion_task;
 		starpu_data_handle_t handle;
 
-		handle = task->buffers[i].handle;
+		handle = task->handles[i];
 		if (!_starpu_handle_needs_conversion_task(handle, node))
 			continue;
 
@@ -354,7 +355,7 @@ static int push_conversion_tasks(struct starpu_task *task, unsigned int workerid
 	}
 
 	for (i = 0; i < task->cl->nbuffers; i++)
-		task->buffers[i].handle->mf_node = node;
+		task->handles[i]->mf_node = node;
 
 	task->execute_on_a_specific_worker = 1;
 	task->workerid = workerid;