On Thu, Jun 25, 2020 at 12:17:05PM -0400, y2s1982 . wrote:
> Ah, so code in env.c gets executed before OMPD gets started?

Not all, but initialize_env is the library constructor (has constructor
attribute) and is invoked during the library initialization.
> 
> Include omp-tools.h and plugin-suffix.h in there and move ompd_dll_locations
> > definition in there (into the #ifndef LIBGOMP_OFFLOADED_ONLY section)
> > and for ompd_dll_locations_valid, e.g. make it an alias to
> > initialize_env or for the time being just an empty function with
> > __attribute__((noipa)) that initialize_env
> > calls and I'll help with making it an alias afterwards.
> 
> I got curious about making a function alias and started looking around.
> Can it be as simple as #define ompd_dll_location_valid initialize_env?

No.

> Or should I use something like ialias_call (fn) fn, defined in libgomp.h?
> The latter seems like a wrapper that expands differently depending on the
> condition,
> and since I wasn't sure what gomp_ialias does, I wasn't sure if that was
> the way to go.

You need something like following, because not all targets that support
libgomp do support aliases.  The noipa attribute tells the compiler not to
optimize it away in any way, which will be what is in there for the arches
without aliases.  And for targets that do have them (e.g. Linux), it will
just be an alias to the initialize_env function.

diff --git a/libgomp/env.c b/libgomp/env.c
index c0c4730d47c..4c3b0131629 100644
--- a/libgomp/env.c
+++ b/libgomp/env.c
@@ -1350,6 +1350,12 @@ handle_omp_display_env (unsigned long stacksize, int 
wait_policy)
   fputs ("OPENMP DISPLAY ENVIRONMENT END\n", stderr);
 }
 
+#ifndef HAVE_ATTRIBUTE_ALIAS
+void __attribute__((noipa))
+ompd_dll_locations_valid (void)
+{
+}
+#endif
 
 static void __attribute__((constructor))
 initialize_env (void)
@@ -1357,6 +1363,10 @@ initialize_env (void)
   unsigned long thread_limit_var, stacksize = GOMP_DEFAULT_STACKSIZE;
   int wait_policy;
 
+#ifndef HAVE_ATTRIBUTE_ALIAS
+  ompd_dll_locations_valid ();
+#endif
+
   /* Do a compile time check that mkomp_h.pl did good job.  */
   omp_check_defines ();
 
@@ -1486,4 +1496,8 @@ initialize_env (void)
 
   goacc_profiling_initialize ();
 }
+#ifdef HAVE_ATTRIBUTE_ALIAS
+strong_alias (initialize_env, ompd_dll_locations_valid)
+#endif
+
 #endif /* LIBGOMP_OFFLOADED_ONLY */

> > > diff --git a/libgomp/omp-tools.h b/libgomp/omp-tools.h
> > > index 394c33e40dd..b6b8c5295a5 100644
> > > --- a/libgomp/omp-tools.h
> > > +++ b/libgomp/omp-tools.h
> > > @@ -101,7 +101,7 @@ typedef struct ompd_device_type_sizes_t {
> > >  } ompd_device_type_sizes_t;
> > >
> > >
> > > -const char **ompd_dll_locations;
> > > +//const char **ompd_dll_locations;
> >
> > Extern declaration would be
> > extern const char **ompd_dll_locations;
> >
> Okay.  I do have an issue:
> extern const char **ompd_dll_locations; and const char*
> ompd_dll_locations[2]; seems to clash with each other.

The former is the type that is mandated by the standard,
so the variable needs to be defined as
const char **ompd_dll_locations.  But, you can just initialize it
to a local variable,
i.e.
static const char *gompd_dll_locations[2] = { ..., NULL };
const char **ompd_dll_locations = gompd_dll_locations;

> Okay. I will do the same for the api_version argument, too.
> For the next patch, I had some questions about this function. It seems I
> need to declare an address space
> for callbacks. Where does this address space get stored?

Look what the OMPToolsInterface ompd-test implementation does.
E.g. when talking about ompd_process_initialize,
ompd_address_space_context_t * is some magic value that the debugger
makes up for talking with the OMPD library, while
ompd_address_space_handle_t * is a structure that is allocated
by OMPD library using the alloc_memory callback and stores whatever
you need to handle that (so at least should remember the
context handle that will need to be passed to the callbacks,
and whatever other info you need (e.g. remember the sizes of the
basic types, etc.).  It will be a private structure for the implementation
(so something to be defined in libgompd.h).

> Should I try making unit tests for the API version functions, focusing on
> the formatting of the string?

No.  You don't have that much time to complete the project, there is a lot
to be still implemented, and ideally we don't want hundreds of OMPD tests,
instead we want fewer of them that will test more functions at once.

        Jakub

Reply via email to