From c20a14aa9800eafcdbe9775cae0e47c2eb4a9769 Mon Sep 17 00:00:00 2001 From: tschwinge Date: Mon, 22 Dec 2014 22:53:55 +0000 Subject: [PATCH] libgomp: Fix locking in OpenMP GOMP_target* functions. libgomp/ * libgomp.c (struct gomp_device_descr): Add lock member. * oacc-host.c (goacc_host_init): Initialize it. * target.c (gomp_target_init): Likewise. (gomp_init_dev_tables): Remove function. (GOMP_target, GOMP_target_data, GOMP_target_update): Instead of calling gomp_init_dev_tables, separate device and memory mapping initilization, guarded by appropriate locking. Check (immutable) device capabilities early. With Intel MIC offloading (emulation), this fixes: FAIL: libgomp.fortran/target7.f90 -O0 execution test FAIL: libgomp.fortran/target7.f90 -O1 execution test FAIL: libgomp.fortran/target7.f90 -O2 execution test FAIL: libgomp.fortran/target7.f90 -O3 -fomit-frame-pointer execution test FAIL: libgomp.fortran/target7.f90 -O3 -fomit-frame-pointer -funroll-loops execution test FAIL: libgomp.fortran/target7.f90 -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions execution test FAIL: libgomp.fortran/target7.f90 -O3 -g execution test FAIL: libgomp.fortran/target7.f90 -Os execution test FAIL: libgomp.fortran/target8.f90 -O0 execution test FAIL: libgomp.fortran/target8.f90 -O1 execution test FAIL: libgomp.fortran/target8.f90 -O2 execution test FAIL: libgomp.fortran/target8.f90 -O3 -fomit-frame-pointer execution test FAIL: libgomp.fortran/target8.f90 -O3 -fomit-frame-pointer -funroll-loops execution test FAIL: libgomp.fortran/target8.f90 -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions execution test FAIL: libgomp.fortran/target8.f90 -O3 -g execution test FAIL: libgomp.fortran/target8.f90 -Os execution test git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@219036 138bc75d-0d04-0410-961f-82ee72b054a4 --- libgomp/ChangeLog.gomp | 12 ++++++++ libgomp/libgomp.h | 11 ++++++++ libgomp/oacc-host.c | 1 + libgomp/oacc-init.c | 12 ++++++-- libgomp/target.c | 77 +++++++++++++++++++++++++++++--------------------- 5 files changed, 78 insertions(+), 35 deletions(-) diff --git a/libgomp/ChangeLog.gomp b/libgomp/ChangeLog.gomp index b9bd024e4cb..2f5b4ae37ce 100644 --- a/libgomp/ChangeLog.gomp +++ b/libgomp/ChangeLog.gomp @@ -1,5 +1,17 @@ 2014-12-22 Thomas Schwinge + * libgomp.c (struct gomp_device_descr): Add lock member. + * oacc-host.c (goacc_host_init): Initialize it. + * target.c (gomp_target_init): Likewise. + (gomp_init_dev_tables): Remove function. + (GOMP_target, GOMP_target_data, GOMP_target_update): Instead of + calling gomp_init_dev_tables, separate device and memory mapping + initilization, guarded by appropriate locking. Check (immutable) + device capabilities early. + + * oacc-init.c (lazy_open, acc_shutdown_1): Lock mem_map when in + use. + * target.c (num_devices_openmp): New variable. (gomp_get_num_devices): Use it. (gomp_target_init): Initialize it, and sort the devices array diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h index b6d216b425c..edcd7bcc520 100644 --- a/libgomp/libgomp.h +++ b/libgomp/libgomp.h @@ -682,9 +682,11 @@ typedef struct acc_dispatch_t acc_map_data/acc_unmap_data or "acc enter data"/"acc exit data" pragmas (TODO). Unlike mapped_data in the goacc_thread struct, unmapping can happen out-of-order with respect to mapping. */ + /* This is guarded by the lock in the "outer" struct gomp_device_descr. */ struct target_mem_desc *data_environ; /* Extra information required for a device instance by a given target. */ + /* This is guarded by the lock in the "outer" struct gomp_device_descr. */ void *target_data; /* Open or close a device instance. */ @@ -730,6 +732,9 @@ typedef struct acc_dispatch_t mapped memory. */ struct gomp_device_descr { + /* Immutable data, which is only set during initialization, and which is not + guarded by the lock. */ + /* The name of the device. */ const char *name; @@ -764,10 +769,16 @@ struct gomp_device_descr void (*run_func) (int, void *, void *); /* OpenACC-specific functions. */ + /* This is mutable because of its mutable data_environ and target_data + members. */ acc_dispatch_t openacc; /* Memory-mapping info for this device instance. */ + /* Uses a separate lock. */ struct gomp_memory_mapping mem_map; + + /* Mutex for the mutable data. */ + gomp_mutex_t lock; }; extern void gomp_acc_insert_pointer (size_t, void **, size_t *, void *); diff --git a/libgomp/oacc-host.c b/libgomp/oacc-host.c index 2a825174dee..c37546389aa 100644 --- a/libgomp/oacc-host.c +++ b/libgomp/oacc-host.c @@ -96,5 +96,6 @@ static __attribute__ ((constructor)) void goacc_host_init (void) { gomp_mutex_init (&host_dispatch.mem_map.lock); + gomp_mutex_init (&host_dispatch.lock); goacc_register (&host_dispatch); } diff --git a/libgomp/oacc-init.c b/libgomp/oacc-init.c index d10b974be38..6acf61bc383 100644 --- a/libgomp/oacc-init.c +++ b/libgomp/oacc-init.c @@ -287,8 +287,11 @@ lazy_open (int ord) acc_dev->openacc.async_set_async_func (acc_async_sync); - if (!acc_dev->mem_map.is_initialized) - gomp_init_tables (acc_dev, &acc_dev->mem_map); + struct gomp_memory_mapping *mem_map = &acc_dev->mem_map; + gomp_mutex_lock (&mem_map->lock); + if (!mem_map->is_initialized) + gomp_init_tables (acc_dev, mem_map); + gomp_mutex_unlock (&mem_map->lock); } /* OpenACC 2.0a (3.2.12, 3.2.13) doesn't specify whether the serialization of @@ -350,7 +353,10 @@ acc_shutdown_1 (acc_device_t d) walk->dev->openacc.target_data = target_data = NULL; - gomp_free_memmap (&walk->dev->mem_map); + struct gomp_memory_mapping *mem_map = &walk->dev->mem_map; + gomp_mutex_lock (&mem_map->lock); + gomp_free_memmap (mem_map); + gomp_mutex_unlock (&mem_map->lock); walk->dev = NULL; } diff --git a/libgomp/target.c b/libgomp/target.c index bf6edd2eada..53e0c7f46f7 100644 --- a/libgomp/target.c +++ b/libgomp/target.c @@ -44,6 +44,7 @@ static void gomp_target_init (void); +/* The whole initialization code for offloading plugins is only run one. */ static pthread_once_t gomp_is_initialized = PTHREAD_ONCE_INIT; /* This structure describes an offload image. @@ -169,6 +170,7 @@ gomp_map_vars (struct gomp_device_descr *devicep, size_t mapnum, tgt_size = mapnum * sizeof (void *); } gomp_mutex_lock (&mm->lock); + for (i = 0; i < mapnum; i++) { int kind = get_kind (is_openacc, kinds, i); @@ -552,8 +554,9 @@ gomp_unmap_vars (struct target_mem_desc *tgt, bool do_copyfrom) return; } - size_t i; gomp_mutex_lock (&mm->lock); + + size_t i; for (i = 0; i < tgt->list_count; i++) if (tgt->list[i] == NULL) ; @@ -580,6 +583,7 @@ gomp_unmap_vars (struct target_mem_desc *tgt, bool do_copyfrom) tgt->refcount--; else gomp_unmap_tgt (tgt); + gomp_mutex_unlock (&mm->lock); } @@ -669,17 +673,19 @@ GOMP_offload_register (void *host_table, enum offload_target_type target_type, num_offload_images++; } -/* This function initializes the target device, specified by DEVICEP. */ +/* This function initializes the target device, specified by DEVICEP. DEVICEP + must be locked on entry, and remains locked on return. */ attribute_hidden void gomp_init_device (struct gomp_device_descr *devicep) { - /* Initialize the target device. */ devicep->init_device_func (devicep->target_id); - devicep->is_initialized = true; } +/* Initialize address mapping tables. MM must be locked on entry, and remains + locked on return. */ + attribute_hidden void gomp_init_tables (struct gomp_device_descr *devicep, struct gomp_memory_mapping *mm) @@ -716,13 +722,8 @@ gomp_init_tables (struct gomp_device_descr *devicep, mm->is_initialized = true; } -static void -gomp_init_dev_tables (struct gomp_device_descr *devicep) -{ - gomp_init_device (devicep); - gomp_init_tables (devicep, &devicep->mem_map); -} - +/* Free address mapping tables. MM must be locked on entry, and remains locked + on return. */ attribute_hidden void gomp_free_memmap (struct gomp_memory_mapping *mm) @@ -739,6 +740,9 @@ gomp_free_memmap (struct gomp_memory_mapping *mm) mm->is_initialized = false; } +/* This function de-initializes the target device, specified by DEVICEP. + DEVICEP must be locked on entry, and remains locked on return. */ + attribute_hidden void gomp_fini_device (struct gomp_device_descr *devicep) { @@ -766,9 +770,6 @@ GOMP_target (int device, void (*fn) (void *), const void *offload_table, { struct gomp_device_descr *devicep = resolve_device (device); - if (devicep != NULL && !devicep->is_initialized) - gomp_init_dev_tables (devicep); - if (devicep == NULL || !(devicep->capabilities & GOMP_OFFLOAD_CAP_OPENMP_400)) { @@ -787,6 +788,11 @@ GOMP_target (int device, void (*fn) (void *), const void *offload_table, return; } + gomp_mutex_lock (&devicep->lock); + if (!devicep->is_initialized) + gomp_init_device (devicep); + gomp_mutex_unlock (&devicep->lock); + void *fn_addr; if (devicep->capabilities & GOMP_OFFLOAD_CAP_NATIVE_EXEC) @@ -795,15 +801,17 @@ GOMP_target (int device, void (*fn) (void *), const void *offload_table, { struct gomp_memory_mapping *mm = &devicep->mem_map; gomp_mutex_lock (&mm->lock); - if (!devicep->is_initialized) - gomp_init_dev_tables (devicep); + + if (!mm->is_initialized) + gomp_init_tables (devicep, mm); + struct splay_tree_key_s k; k.host_start = (uintptr_t) fn; k.host_end = k.host_start + 1; - splay_tree_key tgt_fn = splay_tree_lookup (&devicep->mem_map.splay_tree, - &k); + splay_tree_key tgt_fn = splay_tree_lookup (&mm->splay_tree, &k); if (tgt_fn == NULL) gomp_fatal ("Target function wasn't mapped"); + gomp_mutex_unlock (&mm->lock); fn_addr = (void *) tgt_fn->tgt->tgt_start; @@ -832,9 +840,6 @@ GOMP_target_data (int device, const void *offload_table, size_t mapnum, { struct gomp_device_descr *devicep = resolve_device (device); - if (devicep != NULL && !devicep->is_initialized) - gomp_init_dev_tables (devicep); - if (devicep == NULL || !(devicep->capabilities & GOMP_OFFLOAD_CAP_OPENMP_400)) { @@ -854,10 +859,15 @@ GOMP_target_data (int device, const void *offload_table, size_t mapnum, return; } + gomp_mutex_lock (&devicep->lock); + if (!devicep->is_initialized) + gomp_init_device (devicep); + gomp_mutex_unlock (&devicep->lock); + struct gomp_memory_mapping *mm = &devicep->mem_map; gomp_mutex_lock (&mm->lock); - if (!devicep->is_initialized) - gomp_init_dev_tables (devicep); + if (!mm->is_initialized) + gomp_init_tables (devicep, mm); gomp_mutex_unlock (&mm->lock); struct target_mem_desc *tgt @@ -886,20 +896,22 @@ GOMP_target_update (int device, const void *offload_table, size_t mapnum, { struct gomp_device_descr *devicep = resolve_device (device); - if (devicep == NULL) + if (devicep == NULL + || !(devicep->capabilities & GOMP_OFFLOAD_CAP_OPENMP_400)) return; + gomp_mutex_lock (&devicep->lock); + if (!devicep->is_initialized) + gomp_init_device (devicep); + gomp_mutex_unlock (&devicep->lock); + struct gomp_memory_mapping *mm = &devicep->mem_map; gomp_mutex_lock (&mm->lock); - if (!devicep->is_initialized) - gomp_init_dev_tables (devicep); + if (!mm->is_initialized) + gomp_init_tables (devicep, mm); gomp_mutex_unlock (&mm->lock); - if (!(devicep->capabilities & GOMP_OFFLOAD_CAP_OPENMP_400)) - return; - - gomp_update (devicep, &devicep->mem_map, mapnum, hostaddrs, sizes, kinds, - false); + gomp_update (devicep, mm, mapnum, hostaddrs, sizes, kinds, false); } void @@ -1035,7 +1047,7 @@ gomp_load_plugin_for_device (struct gomp_device_descr *device, } /* This function adds a compatible offload image IMAGE to an accelerator device - DEVICE. */ + DEVICE. DEVICE must be locked on entry, and remains locked on return. */ static void gomp_register_image_for_device (struct gomp_device_descr *device, @@ -1118,6 +1130,7 @@ gomp_target_init (void) current_device.target_id = i; devices[num_devices] = current_device; gomp_mutex_init (&devices[num_devices].mem_map.lock); + gomp_mutex_init (&devices[num_devices].lock); num_devices++; } } -- 2.11.4.GIT