في الجمعة، ١٣ مايو، ٢٠٢٢ ٣:١٤ م Jakub Jelinek <ja...@redhat.com> كتب:
> On Tue, May 10, 2022 at 07:40:41AM +0200, Mohamed Atef wrote:
> > --- a/libgomp/env.c
> > +++ b/libgomp/env.c
> > @@ -33,6 +33,7 @@
> > #ifndef LIBGOMP_OFFLOADED_ONLY
> > #include "libgomp_f.h"
> > #include "oacc-int.h"
> > +#include "ompd-support.h"
> > #include <ctype.h>
> > #include <stdlib.h>
> > #include <stdio.h>
> > @@ -89,6 +90,7 @@ void **gomp_places_list;
> > unsigned long gomp_places_list_len;
> > uintptr_t gomp_def_allocator = omp_default_mem_alloc;
> > int gomp_debug_var;
> > +bool gompd_enabled;
> > unsigned int gomp_num_teams_var;
> > int gomp_nteams_var;
> > int gomp_teams_thread_limit_var;
> > @@ -418,6 +420,33 @@ parse_target_offload (const char *name, enum
> gomp_target_offload_t *offload)
> > gomp_error ("Invalid value for environment variable
> OMP_TARGET_OFFLOAD");
> > }
> >
> > +/* Parse OMPD_DEBUG environment variable. */
>
> s/OMPD_/OMP_/
>
> > +
> > +static void
> > +parse_debug (const char *name, bool *debug_value)
> > +{
> > + const char *env;
> > + bool ret = false;
> > + env = getenv (name);
> > + if (env == NULL)
> > + return;
> > + while (isspace ((unsigned char) *env))
> > + ++env;
> > + if (strncasecmp (env, "enabled", 7) == 0)
> > + {
> > + env += 7;
> > + ret = true;
> > + }
>
> disabled is a valid OMP_DEBUG value, so it needs to be supported too.
>
> > + while (isspace ((unsigned char) *env))
> > + ++env;
> > + if (ret && *env == '\0')
> > + {
> > + *debug_value = ret;
> > + return;
> > + }
> > + gomp_error ("Invalid value for environment variable OMP_DEBUG");
> > +}
> > +
> > /* Parse environment variable set to a boolean or list of
> omp_proc_bind_t
> > enum values. Return true if one was present and it was successfully
> > parsed. */
> > @@ -1483,6 +1512,9 @@ initialize_env (void)
> > = thread_limit_var > INT_MAX ? UINT_MAX : thread_limit_var;
> > }
> > parse_int_secure ("GOMP_DEBUG", &gomp_debug_var, true);
> > + parse_debug ("OMP_DEBUG", &gompd_enabled);
> > + if (gompd_enabled)
> > + gompd_load ();
> > #ifndef HAVE_SYNC_BUILTINS
> > gomp_mutex_init (&gomp_managed_threads_lock);
> > #endif
> > diff --git a/libgomp/libgomp.map b/libgomp/libgomp.map
> > index 2ac58094169..46feba394f5 100644
> > --- a/libgomp/libgomp.map
> > +++ b/libgomp/libgomp.map
> > @@ -13,7 +13,7 @@ OMP_1.0 {
> > #ifdef HAVE_SYMVER_SYMBOL_RENAMING_RUNTIME_SUPPORT
> > # If the assembler used lacks the .symver directive or the
> linker
> > # doesn't support GNU symbol versioning, we have the same symbol in
> > - # two versions, which Sun ld chokes on.
> > + # two versions, which Sun ld chokes on.
> > omp_init_lock;
> > omp_init_nest_lock;
> > omp_destroy_lock;
> > @@ -226,6 +226,14 @@ OMP_5.1 {
> > omp_get_teams_thread_limit_;
> > } OMP_5.0.2;
> >
> > +OMP_5.0.3 {
> > + global:
> > + ompd_dll_locations;
> > + ompd_dll_locations_valid;
> > + ompd_bp_parallel_begin;
> > + ompd_bp_parallel_end;
> > +} OMP_5.0.2;
> > +
>
> This should use OMP_5.1 rather than OMP_5.0.2.
>
> > GOMP_1.0 {
> > global:
> > GOMP_atomic_end;
> > diff --git a/libgomp/libgompd.map b/libgomp/libgompd.map
> > new file mode 100644
> > index 00000000000..fc1b219cdde
> > --- /dev/null
> > +++ b/libgomp/libgompd.map
> > @@ -0,0 +1,21 @@
> > +OMPD_5.1 {
> > + global:
> > + ompd_initialize;
> > + ompd_get_api_version;
> > + ompd_get_version_string;
> > + ompd_process_initialize;
> > + ompd_device_initialize;
> > + ompd_rel_address_space_handle;
> > + ompd_finalize;
> > + ompd_enumerate_icvs;
> > + ompd_get_icv_from_scope;
> > + ompd_get_icv_string_from_scope;
> > + ompd_get_thread_handle;
> > + ompd_get_thread_in_parallel;
> > + ompd_rel_thread_handle;
> > + ompd_thread_handle_compare;
> > + ompd_get_thread_id;
> > + ompd_get_device_from_thread;
> > + ompd_bp_thread_begin;
> > + ompd_bp_thread_end;
>
> local:
> *;
> is missing here.
>
> > +};
> > +
> > +#ifdef __ELF__
> > +#define ompd_dll_locations_valid() \
> > + __asm__ __volatile__ (".globl ompd_dll_locations_valid\n\t" \
> > + "ompd_dll_locations_valid:")
> > +#else
> > +extern void ompd_dll_locations_valid (void) __GOMPD_NOTHROW;
> > +#endif /* __ELF__ */
>
> As I've tried to explain, this #ifdef __ELF__ doesn't belong
> to the public header, which should contain just
> extern void ompd_dll_locations_valid (void) __GOMPD_NOTHROW;
> The #define should be in some internal header that is included
> after the public one.
>
I will put the #define in ompd-support.h is that okay?
And i will put ompd_bp_* labels there too.
>
> > +#ifdef __ELF__
> > +#define ompd_bp_parallel_begin() \
> > + __asm__ __volatile__ (".globl ompd_bp_parallel_begin\n\t" \
> > + "ompd_bp_parallel_begin:")
> > +#define ompd_bp_parallel_end() \
> > + __asm__ __volatile__ (".globl ompd_bp_parallel_end\n\t" \
> > + "ompd_bp_parallel_end:")
> > +#define ompd_bp_task_begin() \
> > + __asm__ __volatile__ (".globl ompd_bp_task_begin\n\t" \
> > + "ompd_bp_task_begin:")
> > +#define ompd_bp_task_end() \
> > + __asm__ __volatile__ (".globl ompd_bp_task_end\n\t" \
> > + "ompd_bp_task_end:")
> > +#define ompd_bp_thread_begin() \
> > + __asm__ __volatile__ (".globl ompd_bp_thread_begin\n\t" \
> > + "ompd_bp_thread_begin:")
> > +#define ompd_bp_thread_end() \
> > + __asm__ __volatile__ (".globl ompd_bp_thread_end\n\t" \
> > + "ompd_bp_thread_end:")
> > +#define ompd_bp_device_begin() \
> > + __asm__ __volatile__ (".globl ompd_bp_device_begin\n\t" \
> > + "ompd_bp_device_end:")
> > +#define ompd_bp_device_end() \
> > + __asm__ __volatile__ (".globl ompd_bp_device_end\n\t" \
> > + "ompd_bp_device_end:")
> > +#else
>
> Similarly here.
>
> > +extern void ompd_bp_parallel_begin (void) __GOMPD_NOTHROW;
> > +extern void ompd_bp_parallel_end (void) __GOMPD_NOTHROW;
> > +
> > +extern void ompd_bp_task_begin (void) __GOMPD_NOTHROW;
> > +extern void ompd_bp_task_end (void) __GOMPD_NOTHROW;
> > +
> > +extern void ompd_bp_thread_begin (void) __GOMPD_NOTHROW;
> > +extern void ompd_bp_thread_end (void) __GOMPD_NOTHROW;
> > +
> > +extern void ompd_bp_device_begin (void) __GOMPD_NOTHROW;
> > +extern void ompd_bp_device_end (void) __GOMPD_NOTHROW;
> > +#endif /* __ELF__ */
>
> > @@ -74,7 +75,7 @@ gomp_thread_start (void *xdata)
> > struct gomp_thread_pool *pool;
> > void (*local_fn) (void *);
> > void *local_data;
> > -
> > + ompd_bp_thread_begin ();
>
> Please keep the vertical space after the var declarations.
>
> > #if defined HAVE_TLS || defined USE_EMUTLS
> > thr = &gomp_tls_data;
> > #else
> > @@ -321,6 +322,7 @@ gomp_team_start (void (*fn) (void *), void *data,
> unsigned nthreads,
> > unsigned flags, struct gomp_team *team,
> > struct gomp_taskgroup *taskgroup)
> > {
> > + ompd_bp_parallel_begin ();
> > struct gomp_thread_start_data *start_data = NULL;
> > struct gomp_thread *thr, *nthr;
> > struct gomp_task *task;
>
> Please put ompd_bp_* after the var declarations + vertical space,
> unless there are already function calls (very cheap inline
> calls like gomp_thread () don't count).
>
> > @@ -1011,6 +1013,7 @@ gomp_team_end (void)
> > pool->last_team = team;
> > gomp_release_thread_pool (pool);
> > }
> > + ompd_bp_parallel_end ();
> > }
> >
> > #ifdef LIBGOMP_USE_PTHREADS
>
>
> Jakub
>
>
