Hello Jakub,
Thank you again for detailed feedback. I had few questions.
On Wed, Jul 8, 2020 at 4:42 PM Jakub Jelinek <ja...@redhat.com> wrote:
> On Wed, Jul 08, 2020 at 03:30:35PM -0400, y2s1982 wrote:
> > +ompd_rc_t
> > +ompd_get_omp_version (ompd_address_space_handle_t *address_space,
> > + ompd_word_t *omp_version)
> > +{
> > + if (omp_version == NULL)
> > + return ompd_rc_bad_input;
> > + if (address_space == NULL)
> > + return ompd_rc_stale_handle;
> > +
> > + /* _OPENMP macro is defined to have yyyymm integer. */
> > + ompd_size_t macro_length = sizeof (int);
> > +
> > + ompd_rc_t ret = ompd_rc_ok;
> > +
> > + struct ompd_address_t addr;
> > + ret = gompd_callbacks.symbol_addr_lookup (address_space->context,
> NULL,
> > + "openmp_version", &addr, NULL);
>
> This can't be right. There is no openmp_version variable in libgomp.so.1
> (and I don't think we should add it).
> As I said multiple times before, you should add one (read-only) data
> variable to libgomp.so.1 that will encode a lot of information that OMPD
> needs and the version should be in there.
>
I do remember, though I obviously understood wrongly. Sorry about that.
I had assumed it might have something to do with ICV but didn't realize it
would also
apply to other variables. In all honesty, I was looking for _OPENMP macro;
I assumed
such information would be stored somewhere already and thought
symbol_addr_lookup() would find it somehow. I saw mentions of it on
ChangeLog,
testsuits, and in one string, but I couldn't find the actual macro. As for
openmp_version,
I (wrongly) made the assumption looking at the omp_lib.h.in. I should learn
more
about .in file's syntax and what they do.
To place a variable in libgomp.so.1, should I define a related struct and
declare a global
extern variable of the struct in omp.h and define it in some related .c
file?
Can I then simply use the name of the declared variable as the name (where
"openmp_version" currently is) to find the struct? As for the value for
_OPENMP version,
where can I find it, or should OMPD maintain its own values for it?
> > +ompd_rc_t
> > +ompd_get_omp_version_string (ompd_address_space_handle_t *address_space,
> > + const char **string)
> > +{
> > + if (string == NULL)
> > + return ompd_rc_bad_input;
> > +
> > + if (address_space == NULL)
> > + return ompd_rc_stale_handle;
> > +
> > + ompd_word_t omp_version;
> > + ompd_rc_t ret = ompd_get_omp_version (address_space, &omp_version);
> > + if (ret != ompd_rc_ok)
> > + return ret;
> > +
> > + char *tmp = "GNU OpenMP Runtime implementing OpenMP 5.0 "
> > + ompd_stringify (omp_version);
>
> This will append "omp_version" to the string literal, won't it?
> That is not what you want in there, you instead want the value.
>
Oh I see. Thanks for pointing that out. I am still learning how macro
expansion works.
Cheers,
Tony
>
> Jakub
>
>
