Hi Jakub!
On Sun, 11 Nov 2018 22:31:42 -0600, I wrote:
> On Tue, 28 Feb 2017 18:43:36 +0100, I wrote:
> > The 2.5 versions of the OpenACC standard added a new chapter "Profiling
> > Interface".
>
> I'd like to get that into trunk. It's not yet complete (that is, doesn't
> provide all the information specified), but it's very useful already, and
> the missing pieces can later be added incrementally.
>
> Jakub, would you please especially review the non-OpenACC-specific
> changes here, including the libgomp ABI changes?
Given a baseline that I've not yet posted ;-) would you please anyway
have a look at the following changes? Is it OK to add/handle the
'acc_register_library' symbol in this way? The idea behind that one is
that you dynamically (including via 'LD_PRELOAD') link your code against
a "library" providing an implementation of 'acc_register_library', or
even define it in your user code (see the test case below), and then upon
initialization, "The OpenACC runtime will invoke 'acc_register_library',
passing [...]".
As far as I can tell, it was never a concern (by us internally as well as
that nobody external ever complained) that 'acc_*' and 'GOACC_*' symbols
are visible when building with '-fopenmp' but (default) '-fno-openacc',
and vice versa, 'omp_*' and 'GOMP_*' symbols are visible when building
with '-fopenacc' but (default) '-fno-openmp'. But,
'acc_register_library' is special in that the runtime (libgomp) will
unconditionally call it, also for '-fopenmp' but (default)
'-fno-openacc'. So, when OpenMP user code happens to contain an
(unrelated) 'acc_register_library' symbol, strange things will happen.
OpenACC states that "Typically, the OpenACC runtime will include a _weak_
definition of 'acc_register_library', which does nothing and which will
be called when there is no tools library". I'm not sure if that's "weak"
specifically in the ELF linking sense, or just generally "weak"
semantics. But it seemed easy enough to just provide a regular symbol in
its own '*.o' file, to be overridden in both the dynamic and static
linking cases, so that's what I've done. Any comments to that aspect?
--- libgomp/Makefile.am
+++ libgomp/Makefile.am
@@ -66,7 +66,7 @@ libgomp_la_SOURCES = alloc.c atomic.c barrier.c
critical.c env.c error.c \
splay-tree.c libgomp-plugin.c oacc-parallel.c oacc-host.c oacc-init.c \
oacc-mem.c oacc-async.c oacc-plugin.c oacc-cuda.c priority_queue.c \
affinity-fmt.c teams.c \
- oacc-profiling.c
+ oacc-profiling.c oacc-profiling-acc_register_library.c
include $(top_srcdir)/plugin/Makefrag.am
--- libgomp/acc_prof.h
+++ libgomp/acc_prof.h
@@ -235,6 +235,9 @@ extern void acc_prof_unregister (acc_event_t,
acc_prof_callback, acc_register_t)
typedef void (*acc_query_fn) ();
typedef acc_query_fn (*acc_prof_lookup_func) (const char *);
extern acc_query_fn acc_prof_lookup (const char *) __GOACC_NOTHROW;
+/* Don't tag 'acc_register_library' as '__GOACC_NOTHROW': this function
can be
+ overridden by the application, and must be expected to do anything. */
+extern void acc_register_library (acc_prof_reg, acc_prof_reg,
acc_prof_lookup_func);
#ifdef __cplusplus
--- libgomp/libgomp.map
+++ libgomp/libgomp.map
@@ -469,6 +469,7 @@ OACC_2.5 {
acc_prof_lookup;
acc_prof_register;
acc_prof_unregister;
+ acc_register_library;
acc_update_device_async;
acc_update_device_async_32_h_;
acc_update_device_async_64_h_;
--- /dev/null
+++ libgomp/oacc-profiling-acc_register_library.c
@@ -0,0 +1,40 @@
+/* OpenACC Profiling Interface: stub 'acc_register_library' function
+[...]
+
+#include "libgomp.h"
+#include "acc_prof.h"
+
+/* This is in its own file so that this function definition can be
overridden
+ when linking statically. */
+
+void
+acc_register_library (acc_prof_reg reg, acc_prof_reg unreg,
+ acc_prof_lookup_func lookup)
+{
+ gomp_debug (0, "dummy %s\n", __FUNCTION__);
+}
--- libgomp/oacc-profiling.c
+++ libgomp/oacc-profiling.c
@@ -107,6 +107,12 @@ goacc_profiling_initialize (void)
/* ..., but profiling is still disabled. */
__atomic_store_n (&goacc_prof_enabled, false, MEMMODEL_RELAXED);
+ /* We are to invoke an external acc_register_library routine, defaulting
to
+ our stub oacc-profiling-acc_register_library.c:acc_register_library
+ implementation. */
+ gomp_debug (0, "%s: calling acc_register_library\n", __FUNCTION__);
+ acc_register_library (acc_prof_register, acc_prof_unregister,
+ acc_prof_lookup);
#ifdef PLUGIN_SUPPORT
char *acc_proflibs = secure_getenv ("ACC_PROFLIB");
while (acc_proflibs != NULL && acc_proflibs[0] != '\0')
@@ -139,16 +145,24 @@ goacc_profiling_initialize (void)
void *dl_handle = dlopen (acc_proflib, RTLD_LAZY);
if (dl_handle != NULL)
{
- extern void acc_register_library (acc_prof_reg, acc_prof_reg,
- acc_prof_lookup_func);
typeof (&acc_register_library) a_r_l
= dlsym (dl_handle, "acc_register_library");
if (a_r_l == NULL)
goto dl_fail;
- gomp_debug (0, " %s: calling %s:acc_register_library\n",
- __FUNCTION__, acc_proflib);
- a_r_l (acc_prof_register, acc_prof_unregister,
- acc_prof_lookup);
+ /* Avoid duplicate registration, for example if the same shared
+ library is specified in LD_PRELOAD and ACC_PROFLIB -- which,
+ for example, TAU 2.26 does when using 'tau_exec -openacc'. */
+ if (a_r_l == acc_register_library)
+ gomp_debug (0, " %s: skipping duplicate"
+ " %s:acc_register_library\n",
+ __FUNCTION__, acc_proflib);
+ else
+ {
+ gomp_debug (0, " %s: calling %s:acc_register_library\n",
+ __FUNCTION__, acc_proflib);
+ a_r_l (acc_prof_register, acc_prof_unregister,
+ acc_prof_lookup);
+ }
}
else
{
--- libgomp/testsuite/libgomp.oacc-c-c++-common/acc_prof-dispatch-1.c
+++ libgomp/testsuite/libgomp.oacc-c-c++-common/acc_prof-dispatch-1.c
@@ -102,7 +102,7 @@ static void cb_compute_construct_end_3 (acc_prof_info
*prof_info, acc_event_info
static acc_prof_reg reg;
static acc_prof_reg unreg;
static acc_prof_lookup_func lookup;
-static void acc_register_library (acc_prof_reg reg_, acc_prof_reg unreg_,
acc_prof_lookup_func lookup_)
+void acc_register_library (acc_prof_reg reg_, acc_prof_reg unreg_,
acc_prof_lookup_func lookup_)
{
DEBUG_printf ("%s\n", __FUNCTION__);
@@ -114,8 +114,6 @@ static void acc_register_library (acc_prof_reg reg_,
acc_prof_reg unreg_, acc_pr
int main()
{
- acc_register_library (acc_prof_register, acc_prof_unregister,
acc_prof_lookup);
-
STATE_OP (state, = 0);
reg (acc_ev_compute_construct_start, cb_compute_construct_start_1,
acc_reg);
reg (acc_ev_compute_construct_start, cb_compute_construct_start_1,
acc_reg);
[...]
Grüße
Thomas
