From a7578a077ed8b64b94282aa55faf7037690abbc5 Mon Sep 17 00:00:00 2001 From: Chung-Lin Tang Date: Tue, 16 Apr 2024 09:03:21 +0000 Subject: [PATCH] OpenACC 2.7: Adjust acc_map_data/acc_unmap_data interaction with reference counters This patch adjusts the implementation of acc_map_data/acc_unmap_data API library routines to more fit the description in the OpenACC 2.7 specification. Instead of using REFCOUNT_INFINITY, we now define a REFCOUNT_ACC_MAP_DATA special value to mark acc_map_data-created mappings. Adjustment around mapping related code to respect OpenACC semantics are also added. libgomp/ChangeLog: * libgomp.h (REFCOUNT_ACC_MAP_DATA): Define as (REFCOUNT_SPECIAL | 2). * oacc-mem.c (acc_map_data): Adjust to use REFCOUNT_ACC_MAP_DATA, initialize dynamic_refcount as 1. (acc_unmap_data): Adjust to use REFCOUNT_ACC_MAP_DATA, (goacc_map_var_existing): Add REFCOUNT_ACC_MAP_DATA case. (goacc_exit_datum_1): Add REFCOUNT_ACC_MAP_DATA case, respect REFCOUNT_ACC_MAP_DATA when decrementing/finalizing. Force lowest dynamic_refcount to be 1 for REFCOUNT_ACC_MAP_DATA. (goacc_enter_data_internal): Add REFCOUNT_ACC_MAP_DATA case. * target.c (gomp_increment_refcount): Return early for REFCOUNT_ACC_MAP_DATA case. (gomp_decrement_refcount): Likewise. * testsuite/libgomp.oacc-c-c++-common/lib-96.c: New testcase. * testsuite/libgomp.oacc-c-c++-common/unmap-infinity-1.c: Adjust testcase error output scan test. --- libgomp/libgomp.h | 2 + libgomp/oacc-mem.c | 49 ++++++++++++------- libgomp/target.c | 8 ++- .../libgomp.oacc-c-c++-common/lib-96.c | 36 ++++++++++++++ .../unmap-infinity-1.c | 2 +- 5 files changed, 77 insertions(+), 20 deletions(-) create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/lib-96.c diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h index f98cccd8b66..089393846d1 100644 --- a/libgomp/libgomp.h +++ b/libgomp/libgomp.h @@ -1163,6 +1163,8 @@ struct target_mem_desc; /* Special value for refcount - tgt_offset contains target address of the artificial pointer to "omp declare target link" object. */ #define REFCOUNT_LINK (REFCOUNT_SPECIAL | 1) +/* Special value for refcount - created through acc_map_data. */ +#define REFCOUNT_ACC_MAP_DATA (REFCOUNT_SPECIAL | 2) /* Special value for refcount - structure element sibling list items. All such key refounts have REFCOUNT_STRUCTELEM bits set, with _FLAG_FIRST diff --git a/libgomp/oacc-mem.c b/libgomp/oacc-mem.c index ba48a1ccbb7..d590806b5cd 100644 --- a/libgomp/oacc-mem.c +++ b/libgomp/oacc-mem.c @@ -411,7 +411,8 @@ acc_map_data (void *h, void *d, size_t s) assert (n->refcount == 1); assert (n->dynamic_refcount == 0); /* Special reference counting behavior. */ - n->refcount = REFCOUNT_INFINITY; + n->refcount = REFCOUNT_ACC_MAP_DATA; + n->dynamic_refcount = 1; if (profiling_p) { @@ -455,12 +456,7 @@ acc_unmap_data (void *h) gomp_fatal ("[%p,%d] surrounds %p", (void *) n->host_start, (int) host_size, (void *) h); } - /* TODO This currently doesn't catch 'REFCOUNT_INFINITY' usage different from - 'acc_map_data'. Maybe 'dynamic_refcount' can be used for disambiguating - the different 'REFCOUNT_INFINITY' cases, or simply separate - 'REFCOUNT_INFINITY' values per different usage ('REFCOUNT_ACC_MAP_DATA' - etc.)? */ - else if (n->refcount != REFCOUNT_INFINITY) + else if (n->refcount != REFCOUNT_ACC_MAP_DATA) { gomp_mutex_unlock (&acc_dev->lock); gomp_fatal ("refusing to unmap block [%p,+%d] that has not been mapped" @@ -468,13 +464,12 @@ acc_unmap_data (void *h) (void *) h, (int) host_size); } - struct target_mem_desc *tgt = n->tgt; + /* This should hold for all mappings created by acc_map_data. We are however + removing this mapping in a "finalize" manner, so dynamic_refcount > 1 does + not matter. */ + assert (n->dynamic_refcount >= 1); - if (tgt->refcount == REFCOUNT_INFINITY) - { - gomp_mutex_unlock (&acc_dev->lock); - gomp_fatal ("cannot unmap target block"); - } + struct target_mem_desc *tgt = n->tgt; /* Above, we've verified that the mapping must have been set up by 'acc_map_data'. */ @@ -519,7 +514,8 @@ goacc_map_var_existing (struct gomp_device_descr *acc_dev, void *hostaddr, } assert (n->refcount != REFCOUNT_LINK); - if (n->refcount != REFCOUNT_INFINITY) + if (n->refcount != REFCOUNT_INFINITY + && n->refcount != REFCOUNT_ACC_MAP_DATA) n->refcount++; n->dynamic_refcount++; @@ -683,13 +679,30 @@ goacc_exit_datum_1 (struct gomp_device_descr *acc_dev, void *h, size_t s, assert (n->refcount != REFCOUNT_LINK); if (n->refcount != REFCOUNT_INFINITY + && n->refcount != REFCOUNT_ACC_MAP_DATA && n->refcount < n->dynamic_refcount) { gomp_mutex_unlock (&acc_dev->lock); gomp_fatal ("Dynamic reference counting assert fail\n"); } - if (finalize) + if (n->refcount == REFCOUNT_ACC_MAP_DATA) + { + if (finalize) + { + /* Mappings created by acc_map_data are returned to initial + dynamic_refcount of 1. Can only be deleted by acc_unmap_data. */ + n->dynamic_refcount = 1; + } + else if (n->dynamic_refcount) + { + /* When mapping is created by acc_map_data, dynamic_refcount must be + maintained at >= 1. */ + if (n->dynamic_refcount > 1) + n->dynamic_refcount--; + } + } + else if (finalize) { if (n->refcount != REFCOUNT_INFINITY) n->refcount -= n->dynamic_refcount; @@ -1131,7 +1144,8 @@ goacc_enter_data_internal (struct gomp_device_descr *acc_dev, size_t mapnum, } /* This is a special case because we must increment the refcount by the number of mapped struct elements, rather than by one. */ - if (n->refcount != REFCOUNT_INFINITY) + if (n->refcount != REFCOUNT_INFINITY + && n->refcount != REFCOUNT_ACC_MAP_DATA) n->refcount += groupnum - 1; n->dynamic_refcount += groupnum - 1; } @@ -1203,7 +1217,8 @@ goacc_enter_data_internal (struct gomp_device_descr *acc_dev, size_t mapnum, processed = true; } else - assert (n->refcount != REFCOUNT_INFINITY); + assert (n->refcount != REFCOUNT_INFINITY + && n->refcount != REFCOUNT_ACC_MAP_DATA); for (size_t j = 0; j < tgt->list_count; j++) if (tgt->list[j].key == n) diff --git a/libgomp/target.c b/libgomp/target.c index bcc86051601..5ec19ae489e 100644 --- a/libgomp/target.c +++ b/libgomp/target.c @@ -476,7 +476,9 @@ gomp_free_device_memory (struct gomp_device_descr *devicep, void *devptr) static inline void gomp_increment_refcount (splay_tree_key k, htab_t *refcount_set) { - if (k == NULL || k->refcount == REFCOUNT_INFINITY) + if (k == NULL + || k->refcount == REFCOUNT_INFINITY + || k->refcount == REFCOUNT_ACC_MAP_DATA) return; uintptr_t *refcount_ptr = &k->refcount; @@ -520,7 +522,9 @@ static inline void gomp_decrement_refcount (splay_tree_key k, htab_t *refcount_set, bool delete_p, bool *do_copy, bool *do_remove) { - if (k == NULL || k->refcount == REFCOUNT_INFINITY) + if (k == NULL + || k->refcount == REFCOUNT_INFINITY + || k->refcount == REFCOUNT_ACC_MAP_DATA) { *do_copy = *do_remove = false; return; diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-96.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-96.c new file mode 100644 index 00000000000..7bc55b94f33 --- /dev/null +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/lib-96.c @@ -0,0 +1,36 @@ +/* { dg-do run } */ +/* { dg-skip-if "" { *-*-* } { "-DACC_MEM_SHARED=1" } } */ + +/* Test if acc_unmap_data has implicit finalize semantics. */ + +#include +#include + +int +main (int argc, char **argv) +{ + const int N = 256; + unsigned char *h; + void *d; + + h = (unsigned char *) malloc (N); + + d = acc_malloc (N); + + acc_map_data (h, d, N); + + acc_copyin (h, N); + acc_copyin (h, N); + acc_copyin (h, N); + + acc_unmap_data (h); + + if (acc_is_present (h, N)) + abort (); + + acc_free (d); + + free (h); + + return 0; +} diff --git a/libgomp/testsuite/libgomp.oacc-c-c++-common/unmap-infinity-1.c b/libgomp/testsuite/libgomp.oacc-c-c++-common/unmap-infinity-1.c index 872f0c1de5c..9ed9fa7e413 100644 --- a/libgomp/testsuite/libgomp.oacc-c-c++-common/unmap-infinity-1.c +++ b/libgomp/testsuite/libgomp.oacc-c-c++-common/unmap-infinity-1.c @@ -10,7 +10,7 @@ main (int argc, char *argv[]) { acc_init (acc_device_default); acc_unmap_data ((void *) foo); -/* { dg-output "libgomp: cannot unmap target block" } */ +/* { dg-output "libgomp: refusing to unmap block \\\[\[0-9a-fA-FxX\]+,\\\+64\\\] that has not been mapped by 'acc_map_data'" } */ return 0; }