From f8332e52a498df480f72303de32ad0751ad899fe Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Mon, 27 Feb 2023 15:56:18 +0100
Subject: [PATCH] Use 'GOMP_MAP_VARS_TARGET' for OpenACC compute constructs
[PR90596]
Thereby considerably simplify the device plugins' 'GOMP_OFFLOAD_openacc_exec',
'GOMP_OFFLOAD_openacc_async_exec' functions: in terms of lines of code, but in
particular conceptually: no more device memory allocation, host to device data
copying, device memory deallocation -- 'GOMP_MAP_VARS_TARGET' does all that for
us.
This depends on commit 2b2340e236c0bba8aaca358ea25a5accd8249fbd
"Allow libgomp 'cbuf' buffering with OpenACC 'async' for 'ephemeral' data",
where I said that "a use will emerge later", which is this one here.
PR libgomp/90596
libgomp/
* target.c (gomp_map_vars_internal): Allow for
'param_kind == GOMP_MAP_VARS_OPENACC | GOMP_MAP_VARS_TARGET'.
* oacc-parallel.c (GOACC_parallel_keyed): Pass
'GOMP_MAP_VARS_TARGET' to 'goacc_map_vars'.
* plugin/plugin-gcn.c (alloc_by_agent, gcn_exec)
(GOMP_OFFLOAD_openacc_exec, GOMP_OFFLOAD_openacc_async_exec):
Adjust, simplify.
(gomp_offload_free): Remove.
* plugin/plugin-nvptx.c (nvptx_exec, GOMP_OFFLOAD_openacc_exec)
(GOMP_OFFLOAD_openacc_async_exec): Adjust, simplify.
(cuda_free_argmem): Remove.
* testsuite/libgomp.oacc-c-c++-common/acc_prof-parallel-1.c:
Adjust.
---
libgomp/oacc-parallel.c | 13 +-
libgomp/plugin/plugin-gcn.c | 47 +-----
libgomp/plugin/plugin-nvptx.c | 154 ++----------------
libgomp/target.c | 10 +-
.../acc_prof-parallel-1.c | 58 ++-----
5 files changed, 44 insertions(+), 238 deletions(-)
@@ -108,8 +108,6 @@ GOACC_parallel_keyed (int flags_m, void (*fn) (void *),
va_list ap;
struct goacc_thread *thr;
struct gomp_device_descr *acc_dev;
- struct target_mem_desc *tgt;
- void **devaddrs;
unsigned int i;
struct splay_tree_key_s k;
splay_tree_key tgt_fn_key;
@@ -290,8 +288,10 @@ GOACC_parallel_keyed (int flags_m, void (*fn) (void *),
goacc_aq aq = get_goacc_asyncqueue (async);
- tgt = goacc_map_vars (acc_dev, aq, mapnum, hostaddrs, NULL, sizes, kinds,
- true, 0);
+ struct target_mem_desc *tgt
+ = goacc_map_vars (acc_dev, aq, mapnum, hostaddrs, NULL, sizes, kinds, true,
+ GOMP_MAP_VARS_TARGET);
+
if (profiling_p)
{
prof_info.event_type = acc_ev_enter_data_end;
@@ -301,10 +301,7 @@ GOACC_parallel_keyed (int flags_m, void (*fn) (void *),
&api_info);
}
- devaddrs = gomp_alloca (sizeof (void *) * mapnum);
- for (i = 0; i < mapnum; i++)
- devaddrs[i] = (void *) gomp_map_val (tgt, hostaddrs, i);
-
+ void **devaddrs = (void **) tgt->tgt_start;
if (aq == NULL)
acc_dev->openacc.exec_func (tgt_fn, mapnum, hostaddrs, devaddrs, dims,
tgt);
@@ -1833,13 +1833,6 @@ alloc_by_agent (struct agent_info *agent, size_t size)
{
GCN_DEBUG ("Allocating %zu bytes on device %d\n", size, agent->device_id);
- /* Zero-size allocations are invalid, so in order to return a valid pointer
- we need to pass a valid size. One source of zero-size allocations is
- kernargs for kernels that have no inputs or outputs (the kernel may
- only use console output, for example). */
- if (size == 0)
- size = 4;
-
void *ptr;
hsa_status_t status = hsa_fns.hsa_memory_allocate_fn (agent->data_region,
size, &ptr);
@@ -2989,15 +2982,6 @@ copy_data (void *data_)
free (data);
}
-/* Free device data. This is intended for use as an async callback event. */
-
-static void
-gomp_offload_free (void *ptr)
-{
- GCN_DEBUG ("Async thread ?:?: Freeing %p\n", ptr);
- GOMP_OFFLOAD_free (0, ptr);
-}
-
/* Request an asynchronous data copy, to or from a device, on a given queue.
The event will be registered as a callback. */
@@ -3064,7 +3048,7 @@ wait_queue (struct goacc_asyncqueue *aq)
/* Execute an OpenACC kernel, synchronously or asynchronously. */
static void
-gcn_exec (struct kernel_info *kernel, size_t mapnum,
+gcn_exec (struct kernel_info *kernel,
void **devaddrs, unsigned *dims, void *targ_mem_desc, bool async,
struct goacc_asyncqueue *aq)
{
@@ -3074,11 +3058,6 @@ gcn_exec (struct kernel_info *kernel, size_t mapnum,
/* If we get here then this must be an OpenACC kernel. */
kernel->kind = KIND_OPENACC;
- /* devaddrs must be double-indirect on the target. */
- void **ind_da = alloc_by_agent (kernel->agent, sizeof (void*) * mapnum);
- for (size_t i = 0; i < mapnum; i++)
- hsa_fns.hsa_memory_copy_fn (&ind_da[i], &devaddrs[i], sizeof (void *));
-
struct hsa_kernel_description *hsa_kernel_desc = NULL;
for (unsigned i = 0; i < kernel->module->image_desc->kernel_count; i++)
{
@@ -3190,9 +3169,9 @@ gcn_exec (struct kernel_info *kernel, size_t mapnum,
}
if (!async)
- run_kernel (kernel, ind_da, &kla, NULL, false);
+ run_kernel (kernel, devaddrs, &kla, NULL, false);
else
- queue_push_launch (aq, kernel, ind_da, &kla);
+ queue_push_launch (aq, kernel, devaddrs, &kla);
if (profiling_dispatch_p)
{
@@ -3202,16 +3181,6 @@ gcn_exec (struct kernel_info *kernel, size_t mapnum,
&enqueue_launch_event_info,
api_info);
}
-
- if (!async)
- gomp_offload_free (ind_da);
- else
- {
- if (DEBUG_QUEUES)
- GCN_DEBUG ("queue_push_callback %d:%d gomp_offload_free, %p\n",
- aq->agent->device_id, aq->id, ind_da);
- queue_push_callback (aq, gomp_offload_free, ind_da);
- }
}
/* }}} */
@@ -3884,20 +3853,22 @@ GOMP_OFFLOAD_async_run (int device, void *tgt_fn, void *tgt_vars,
already-loaded KERNEL. */
void
-GOMP_OFFLOAD_openacc_exec (void (*fn_ptr) (void *), size_t mapnum,
+GOMP_OFFLOAD_openacc_exec (void (*fn_ptr) (void *),
+ size_t mapnum __attribute__((unused)),
void **hostaddrs __attribute__((unused)),
void **devaddrs, unsigned *dims,
void *targ_mem_desc)
{
struct kernel_info *kernel = (struct kernel_info *) fn_ptr;
- gcn_exec (kernel, mapnum, devaddrs, dims, targ_mem_desc, false, NULL);
+ gcn_exec (kernel, devaddrs, dims, targ_mem_desc, false, NULL);
}
/* Run an asynchronous OpenACC kernel on the specified queue. */
void
-GOMP_OFFLOAD_openacc_async_exec (void (*fn_ptr) (void *), size_t mapnum,
+GOMP_OFFLOAD_openacc_async_exec (void (*fn_ptr) (void *),
+ size_t mapnum __attribute__((unused)),
void **hostaddrs __attribute__((unused)),
void **devaddrs,
unsigned *dims, void *targ_mem_desc,
@@ -3905,7 +3876,7 @@ GOMP_OFFLOAD_openacc_async_exec (void (*fn_ptr) (void *), size_t mapnum,
{
struct kernel_info *kernel = (struct kernel_info *) fn_ptr;
- gcn_exec (kernel, mapnum, devaddrs, dims, targ_mem_desc, true, aq);
+ gcn_exec (kernel, devaddrs, dims, targ_mem_desc, true, aq);
}
/* Create a new asynchronous thread and queue for running future kernels. */
@@ -742,7 +742,7 @@ link_ptx (CUmodule *module, const struct targ_ptx_obj *ptx_objs,
}
static void
-nvptx_exec (void (*fn), size_t mapnum, unsigned *dims, void *targ_mem_desc,
+nvptx_exec (void (*fn), unsigned *dims, void *targ_mem_desc,
CUdeviceptr dp, CUstream stream)
{
struct targ_fn_descriptor *targ_fn = (struct targ_fn_descriptor *) fn;
@@ -1528,70 +1528,16 @@ GOMP_OFFLOAD_free (int ord, void *ptr)
}
void
-GOMP_OFFLOAD_openacc_exec (void (*fn) (void *), size_t mapnum,
+GOMP_OFFLOAD_openacc_exec (void (*fn) (void *),
+ size_t mapnum __attribute__((unused)),
void **hostaddrs __attribute__((unused)),
void **devaddrs,
unsigned *dims, void *targ_mem_desc)
{
- GOMP_PLUGIN_debug (0, " %s: prepare mappings\n", __FUNCTION__);
+ GOMP_PLUGIN_debug (0, "nvptx %s\n", __FUNCTION__);
- struct goacc_thread *thr = GOMP_PLUGIN_goacc_thread ();
- acc_prof_info *prof_info = thr->prof_info;
- acc_event_info data_event_info;
- acc_api_info *api_info = thr->api_info;
- bool profiling_p = __builtin_expect (prof_info != NULL, false);
-
- void **hp = NULL;
- CUdeviceptr dp = 0;
-
- if (mapnum > 0)
- {
- size_t s = mapnum * sizeof (void *);
- hp = alloca (s);
- for (int i = 0; i < mapnum; i++)
- hp[i] = devaddrs[i];
- CUDA_CALL_ASSERT (cuMemAlloc, &dp, s);
- if (profiling_p)
- goacc_profiling_acc_ev_alloc (thr, (void *) dp, s);
- }
-
- /* Copy the (device) pointers to arguments to the device (dp and hp might in
- fact have the same value on a unified-memory system). */
- if (mapnum > 0)
- {
- if (profiling_p)
- {
- prof_info->event_type = acc_ev_enqueue_upload_start;
-
- data_event_info.data_event.event_type = prof_info->event_type;
- data_event_info.data_event.valid_bytes
- = _ACC_DATA_EVENT_INFO_VALID_BYTES;
- data_event_info.data_event.parent_construct
- = acc_construct_parallel;
- data_event_info.data_event.implicit = 1; /* Always implicit. */
- data_event_info.data_event.tool_info = NULL;
- data_event_info.data_event.var_name = NULL;
- data_event_info.data_event.bytes = mapnum * sizeof (void *);
- data_event_info.data_event.host_ptr = hp;
- data_event_info.data_event.device_ptr = (const void *) dp;
-
- api_info->device_api = acc_device_api_cuda;
-
- GOMP_PLUGIN_goacc_profiling_dispatch (prof_info, &data_event_info,
- api_info);
- }
- CUDA_CALL_ASSERT (cuMemcpyHtoD, dp, (void *) hp,
- mapnum * sizeof (void *));
- if (profiling_p)
- {
- prof_info->event_type = acc_ev_enqueue_upload_end;
- data_event_info.data_event.event_type = prof_info->event_type;
- GOMP_PLUGIN_goacc_profiling_dispatch (prof_info, &data_event_info,
- api_info);
- }
- }
-
- nvptx_exec (fn, mapnum, dims, targ_mem_desc, dp, NULL);
+ CUdeviceptr dp = (CUdeviceptr) devaddrs;
+ nvptx_exec (fn, dims, targ_mem_desc, dp, NULL);
CUresult r = CUDA_CALL_NOCHECK (cuStreamSynchronize, NULL);
const char *maybe_abort_msg = "(perhaps abort was called)";
@@ -1600,98 +1546,20 @@ GOMP_OFFLOAD_openacc_exec (void (*fn) (void *), size_t mapnum,
maybe_abort_msg);
else if (r != CUDA_SUCCESS)
GOMP_PLUGIN_fatal ("cuStreamSynchronize error: %s", cuda_error (r));
-
- CUDA_CALL_ASSERT (cuMemFree, dp);
- if (profiling_p)
- goacc_profiling_acc_ev_free (thr, (void *) dp);
-}
-
-static void
-cuda_free_argmem (void *ptr)
-{
- void **block = (void **) ptr;
- nvptx_free (block[0], (struct ptx_device *) block[1]);
- free (block);
}
void
-GOMP_OFFLOAD_openacc_async_exec (void (*fn) (void *), size_t mapnum,
+GOMP_OFFLOAD_openacc_async_exec (void (*fn) (void *),
+ size_t mapnum __attribute__((unused)),
void **hostaddrs __attribute__((unused)),
void **devaddrs,
unsigned *dims, void *targ_mem_desc,
struct goacc_asyncqueue *aq)
{
- GOMP_PLUGIN_debug (0, " %s: prepare mappings\n", __FUNCTION__);
-
- struct goacc_thread *thr = GOMP_PLUGIN_goacc_thread ();
- acc_prof_info *prof_info = thr->prof_info;
- acc_event_info data_event_info;
- acc_api_info *api_info = thr->api_info;
- bool profiling_p = __builtin_expect (prof_info != NULL, false);
-
- void **hp = NULL;
- CUdeviceptr dp = 0;
- void **block = NULL;
-
- if (mapnum > 0)
- {
- size_t s = mapnum * sizeof (void *);
- block = (void **) GOMP_PLUGIN_malloc (2 * sizeof (void *) + s);
- hp = block + 2;
- for (int i = 0; i < mapnum; i++)
- hp[i] = devaddrs[i];
- CUDA_CALL_ASSERT (cuMemAlloc, &dp, s);
- if (profiling_p)
- goacc_profiling_acc_ev_alloc (thr, (void *) dp, s);
- }
-
- /* Copy the (device) pointers to arguments to the device (dp and hp might in
- fact have the same value on a unified-memory system). */
- if (mapnum > 0)
- {
- if (profiling_p)
- {
- prof_info->event_type = acc_ev_enqueue_upload_start;
-
- data_event_info.data_event.event_type = prof_info->event_type;
- data_event_info.data_event.valid_bytes
- = _ACC_DATA_EVENT_INFO_VALID_BYTES;
- data_event_info.data_event.parent_construct
- = acc_construct_parallel;
- data_event_info.data_event.implicit = 1; /* Always implicit. */
- data_event_info.data_event.tool_info = NULL;
- data_event_info.data_event.var_name = NULL;
- data_event_info.data_event.bytes = mapnum * sizeof (void *);
- data_event_info.data_event.host_ptr = hp;
- data_event_info.data_event.device_ptr = (const void *) dp;
-
- api_info->device_api = acc_device_api_cuda;
-
- GOMP_PLUGIN_goacc_profiling_dispatch (prof_info, &data_event_info,
- api_info);
- }
-
- CUDA_CALL_ASSERT (cuMemcpyHtoDAsync, dp, (void *) hp,
- mapnum * sizeof (void *), aq->cuda_stream);
- block[0] = (void *) dp;
-
- struct nvptx_thread *nvthd =
- (struct nvptx_thread *) GOMP_PLUGIN_acc_thread ();
- block[1] = (void *) nvthd->ptx_dev;
-
- if (profiling_p)
- {
- prof_info->event_type = acc_ev_enqueue_upload_end;
- data_event_info.data_event.event_type = prof_info->event_type;
- GOMP_PLUGIN_goacc_profiling_dispatch (prof_info, &data_event_info,
- api_info);
- }
- }
-
- nvptx_exec (fn, mapnum, dims, targ_mem_desc, dp, aq->cuda_stream);
+ GOMP_PLUGIN_debug (0, "nvptx %s\n", __FUNCTION__);
- if (mapnum > 0)
- GOMP_OFFLOAD_openacc_async_queue_callback (aq, cuda_free_argmem, block);
+ CUdeviceptr dp = (CUdeviceptr) devaddrs;
+ nvptx_exec (fn, dims, targ_mem_desc, dp, aq->cuda_stream);
}
void *
@@ -983,13 +983,13 @@ gomp_map_vars_internal (struct gomp_device_descr *devicep,
cbuf.chunk_cnt = -1;
cbuf.use_cnt = 0;
cbuf.buf = NULL;
- if (mapnum > 1 || pragma_kind == GOMP_MAP_VARS_TARGET)
+ if (mapnum > 1 || (pragma_kind & GOMP_MAP_VARS_TARGET))
{
size_t chunks_size = (mapnum + 1) * sizeof (struct gomp_coalesce_chunk);
cbuf.chunks = (struct gomp_coalesce_chunk *) gomp_alloca (chunks_size);
cbuf.chunk_cnt = 0;
}
- if (pragma_kind == GOMP_MAP_VARS_TARGET)
+ if (pragma_kind & GOMP_MAP_VARS_TARGET)
{
size_t align = 4 * sizeof (void *);
tgt_align = align;
@@ -1262,7 +1262,7 @@ gomp_map_vars_internal (struct gomp_device_descr *devicep,
tgt->tgt_start = (uintptr_t) tgt->to_free;
tgt->tgt_end = tgt->tgt_start + sizes[0];
}
- else if (not_found_cnt || pragma_kind == GOMP_MAP_VARS_TARGET)
+ else if (not_found_cnt || (pragma_kind & GOMP_MAP_VARS_TARGET))
{
/* Allocate tgt_align aligned tgt_size block of memory. */
/* FIXME: Perhaps change interface to allocate properly aligned
@@ -1300,7 +1300,7 @@ gomp_map_vars_internal (struct gomp_device_descr *devicep,
}
tgt_size = 0;
- if (pragma_kind == GOMP_MAP_VARS_TARGET)
+ if (pragma_kind & GOMP_MAP_VARS_TARGET)
tgt_size = mapnum * sizeof (void *);
tgt->array = NULL;
@@ -1738,7 +1738,7 @@ gomp_map_vars_internal (struct gomp_device_descr *devicep,
}
}
- if (pragma_kind == GOMP_MAP_VARS_TARGET)
+ if (pragma_kind & GOMP_MAP_VARS_TARGET)
{
for (i = 0; i < mapnum; i++)
{
@@ -203,9 +203,7 @@ static void cb_alloc (acc_prof_info *prof_info, acc_event_info *event_info, acc_
# error TODO
#else
assert (state == 4
- || state == 6
- || state == 104
- || state == 106);
+ || state == 104);
STATE_OP (state, ++);
if (state == 5
@@ -217,13 +215,6 @@ static void cb_alloc (acc_prof_info *prof_info, acc_event_info *event_info, acc_
assert (tool_info->nested->event_info.other_event.event_type == acc_ev_enter_data_start);
assert (tool_info->nested->nested == NULL);
}
- else if (state == 7
- || state == 107)
- {
- assert (tool_info != NULL);
- assert (tool_info->event_info.other_event.event_type == acc_ev_compute_construct_start);
- assert (tool_info->nested == NULL);
- }
else
abort ();
#endif
@@ -268,17 +259,10 @@ static void cb_free (acc_prof_info *prof_info, acc_event_info *event_info, acc_a
#if DEVICE_INIT_INSIDE_COMPUTE_CONSTRUCT
# error TODO
#else
- assert (state == 9
- || state == 11);
+ assert (state == 9);
STATE_OP (state, ++);
if (state == 10)
- {
- assert (tool_info != NULL);
- assert (tool_info->event_info.other_event.event_type == acc_ev_compute_construct_start);
- assert (tool_info->nested == NULL);
- }
- else if (state == 12)
{
assert (tool_info != NULL);
assert (tool_info->event_info.other_event.event_type == acc_ev_compute_construct_start);
@@ -449,19 +433,9 @@ static void cb_exit_data_start (acc_prof_info *prof_info, acc_event_info *event_
{
DEBUG_printf ("%s\n", __FUNCTION__);
+ assert (state == 8
#if ASYNC_EXIT_DATA
- if (acc_async != acc_async_sync)
- {
- /* Compensate for the deferred 'acc_ev_free'. */
- state += 1;
- }
-#else
-# error TODO
-#endif
-
- assert (state == 10
-#if ASYNC_EXIT_DATA
- || state == 110
+ || state == 108
#endif
);
STATE_OP (state, ++);
@@ -525,9 +499,9 @@ static void cb_exit_data_end (acc_prof_info *prof_info, acc_event_info *event_in
{
DEBUG_printf ("%s\n", __FUNCTION__);
- assert (state == 12
+ assert (state == 10
#if ASYNC_EXIT_DATA
- || state == 112
+ || state == 110
#endif
);
STATE_OP (state, ++);
@@ -654,13 +628,9 @@ static void cb_compute_construct_end (acc_prof_info *prof_info, acc_event_info *
{
/* Compensate for the missing 'acc_ev_enter_data_end'. */
state += 1;
- /* Compensate for the missing 'acc_ev_alloc'. */
- state += 1;
/* Compensate for the missing 'acc_ev_enqueue_launch_start' and
'acc_ev_enqueue_launch_end'. */
state += 2;
- /* Compensate for the missing 'acc_ev_free'. */
- state += 1;
/* Compensate for the missing 'acc_ev_exit_data_start'. */
state += 1;
/* Compensate for the missing 'acc_ev_free'. */
@@ -676,8 +646,8 @@ static void cb_compute_construct_end (acc_prof_info *prof_info, acc_event_info *
state += 2;
}
#endif
- assert (state == 13
- || state == 113);
+ assert (state == 11
+ || state == 111);
STATE_OP (state, ++);
assert (tool_info != NULL);
@@ -731,8 +701,8 @@ static void cb_enqueue_launch_start (acc_prof_info *prof_info, acc_event_info *e
assert (acc_device_type != acc_device_host);
- assert (state == 7
- || state == 107);
+ assert (state == 6
+ || state == 106);
STATE_OP (state, ++);
assert (tool_info != NULL);
@@ -800,8 +770,8 @@ static void cb_enqueue_launch_end (acc_prof_info *prof_info, acc_event_info *eve
assert (acc_device_type != acc_device_host);
- assert (state == 8
- || state == 108);
+ assert (state == 7
+ || state == 107);
STATE_OP (state, ++);
assert (tool_info != NULL);
@@ -891,7 +861,7 @@ int main()
}
assert (state_init == 5);
}
- assert (state == 14);
+ assert (state == 12);
STATE_OP (state, = 100);
@@ -908,7 +878,7 @@ int main()
#pragma acc wait
assert (state_init == 105);
}
- assert (state == 114);
+ assert (state == 112);
return 0;
}
--
2.25.1
Hi! Pushed to master branch commit f8332e52a498df480f72303de32ad0751ad899fe "Use 'GOMP_MAP_VARS_TARGET' for OpenACC compute constructs [PR90596]", see attached. libgomp/oacc-parallel.c | 13 +- libgomp/plugin/plugin-gcn.c | 47 ++----- libgomp/plugin/plugin-nvptx.c | 154 ++------------------- libgomp/target.c | 10 +- .../acc_prof-parallel-1.c | 58 ++------ 5 files changed, 44 insertions(+), 238 deletions(-) I like it. :-) Grüße Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955