Hello Jakub,
Thank you for the detailed information. I will make those changes right
away.
Cheers,
Tony Sim
On Tue, Jul 14, 2020 at 5:57 AM Jakub Jelinek <ja...@redhat.com> wrote:
> On Thu, Jul 09, 2020 at 07:01:00PM -0400, y2s1982 via Gcc-patches wrote:
> > --- a/libgomp/libgompd.h
> > +++ b/libgomp/libgompd.h
> > @@ -47,4 +47,19 @@ typedef struct _ompd_aspace_handle {
> > ompd_size_t ref_count;
> > } ompd_address_space_handle_t;
> >
> > +struct gompd_env
> > +{
> > + /* TODO: when the struct is better defined, turn it into a compact
> form.
> > + LINK:
> https://gcc.gnu.org/pipermail/gcc-patches/2020-July/549698.html
> > + For now, keep it as a struct. */
> > +
> > + /* Environment set version number. */
> > + ompd_word_t gompd_env_version;
> > + /* Represents _OPENMP that is in yyyymm format. */
> > + ompd_word_t openmp_version;
> > +};
> > +
> > +/* TODO: when gompd_env is better defined, turn it into a compact
> form. */
> > +extern struct gompd_env gompd_env_info;
>
> I don't think you want this to be a global var.
>
> > + ompd_size_t macro_length = 6; /* _OPENMP format: yyyymm. */
>
> So use omp_version_len = strlen ("yyyymm");
> and no comment is needed (the compiler will optimize it into constant)
> or omp_version_len = sizeof ("yyymm") - 1;
>
> > + char *tmp = "GNU OpenMP Runtime implementing OpenMP 5.0 ";
> > + ompd_size_t tmp_length = strlen (tmp);
>
> Use name instead of tmp in both vars?
>
> > --- a/libgomp/ompd-lib.c
> > +++ b/libgomp/ompd-lib.c
> > @@ -31,6 +31,17 @@
> >
> > ompd_callbacks_t gompd_callbacks;
> > static int ompd_initialized = 0;
> > +struct gompd_env gompd_env_info;
>
> Again.
>
> > +
> > +ompd_rc_t
> > +gompd_set_environment ()
> > +{
> > + /* TODO: Turn this placeholder function to handle OMPD environment
> variables
> > + when it becomes compact. */
> > + struct gompd_env temp_env = { 202007, 201811 };
> > + gompd_env_info = temp_env;
> > + return ompd_rc_ok;
> > +}
> >
> > ompd_rc_t
> > ompd_get_api_version (ompd_word_t *version)
> > @@ -57,6 +68,7 @@ ompd_initialize (ompd_word_t api_version, const
> ompd_callbacks_t *callbacks)
> > return ompd_rc_error;
> >
> > gompd_callbacks = *callbacks;
> > + gompd_set_environment ();
>
> And you shouldn't call it here, but instead in ompd_process_initialize
> and put the struct into the handle.
>
> The thing is, the same OMPD library can e.g. handle a 64-bit and 32-bit
> process, or one with older and one with newer libgomp.so.1.
>
> Jakub
>
>
