[PATCH] libgomp: added simple functions and tests for OMPD

[Jakub Jelinek]

On Wed, Jun 24, 2020 at 09:22:51PM -0400, y2s1982 wrote:
> +#ifndef LIBGOMPD_H
> +#define LIBGOMPD_H 1
> +
> +#define ompd_str1(x) ompd_str2(x)
> +#define ompd_str2(x) #x

I'd call ompd_str1 (the one that should be actually used)
just ompd_str or ompd_stringify.

> --- /dev/null
> +++ b/libgomp/libgompd.map
> @@ -0,0 +1,49 @@
> +OMPD_5.0 {
> +  global:
> +	ompd_dll_locations_valid;

ompd_dll_locations and ompd_dll_locations_valid both need to be exported,
but not from libgompd.so.1 but from libgomp.so.1, so they need to go into
libgomp.map and be defined somewhere in libgomp.so.1, so likely env.c.
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.

> diff --git a/libgomp/libgompd.s b/libgomp/libgompd.s
> new file mode 100644
> index 00000000000..a648bedf3c7
> --- /dev/null
> +++ b/libgomp/libgompd.s
> @@ -0,0 +1 @@
> +	.file	"libgompd.c"

You don't want to check this in.

> 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;

> +ompd_rc_t
> +ompd_get_api_version (ompd_word_t *version)
> +{
> +  *version = OMPD_VERSION;
> +  return ompd_rc_ok;
> +}
> +
> +ompd_rc_t
> +ompd_get_version_string (const char **string)
> +{
> +  *string = "GNU OpenMP Runtime implementing OpenMP 5.0" ompd_str2(OMPD_VERSION);

You need a space between 5.0 and the OMPD_VERSION string, so 
5.0 " ompd_stringify (OMPD_VERSION);
and also, the line gets too long, so please write
  *string = "GNU OpenMP Runtime implementing OpenMP 5.0 "
	    ompd_stringify (OMPD_VERSION);

> +ompd_rc_t
> +ompd_initialize (ompd_word_t api_version, const ompd_callbacks_t *callbacks)
> +{
> +  /* initialized flag */

You don't need to comment everything the function is doing, I think the
comments don't say anything that isn't obvious from it.
Comment only for larger blocks of code where it might not be obvious what
the code is doing and the comment adds added value; comments should be
usually full sentences, starting with capital letter, ending with . and
two spaces after the .
Or one can add function comments (comments before function definition) that
explain what the function does, what are the arguments etc., but in the
case of functions documented in the standard it seems pointless,
the best documentation is the standard.

You don't use the callbacks argument yet, but you will need it later,
so instead of adding an unused attribute to it, just add
  (void) callbacks;
for now to shut up warnings.

Otherwise LGTM.

	Jakub