Re: GNU OMPD implementation

[Martin Jambor <mjambor@suse.cz>]

Hi,

On Tue, Nov 30 2021, Mohamed Atef wrote:
> Hello,
>      for the gdb part it's already understood. gdb documentation explains
> how to extend gdb functionality using python, and we looked at clang code
> and now it's very clear how to provide OMPD functions with parameters.

great to hear that.

>
> From OpenMP API specification 5.1 section 5.6
> "The OpenMP implementation must define several entry point symbols through
> which execution
> must pass when particular events occur and data collection for OMPD is
> enabled. A tool can enable
> notification of an event by setting a breakpoint at the address of the
> entry point symbol."
>
> We need to add OMPD support to libgomp, and the debugger should be notified
> that it has entered a new parallel region so we make dummy functions (or
> labels) at every  start and end of (parallel, task, thread) regions (note :
> they should not be optimized).

For a proof of concept phase you can make them functions with attributes
used and noipa and we can figure out how to come up with a less terrible
solution later.

>
> for the structures that we need to access in the runtime
> for example here:
>
> https://github.com/gcc-mirror/gcc/blob/master/libgomp/icv.c#L38
>
> if OMPD needs to get the max threads of the target program it literally
> repeat the call so we use callbacks to get the address of struct
> gomp_task_icv then we read its value the we get the value of nthreads_var
> variable
> for example : callbacks->lookup(icv)->access(nthreads_var)
>
> for now:
> We finished the initialization phase and we're now working on how to test
> the initialization of both the library and the target process.
> We finished 5.5.1, 5.5.2, but for 5.5.4 i need to know the OpenMP version.

When you are ready to share some git repo or something, please do so.

> Where is the variable of the OpenMP version defined?

I do not know if there even is one.  The OpenMP implementation often has
only partial implementation of features in the newer versions of the
standard so coming up with one definitive version may be tricky.

>
> -----------
>
> Current issues,
>
> to get the variable names from the target needs to move from the debugger
> space to the target process space to lookup the values so we can use
> something like (std::map) to save the values of the symbols that we got
> before but we don't use C++.
> Can we use C++ ? . If not, Can we implement it ourselves?

Please don't use C++ for the runtime.  libgomp has a header file for
hash tables, but as you say...

> Now we're thinking of leaving it as it's (access the target every time you
> need to read or lookup.) and after finishing the project we can think of
> something like caching the variables or any other way.

...this can be dealt with later.  Unless it is not viable even for
simple testcases, starting with linear searches and improving later is
probably a better approach.

>
> OMPD will support CPUs for now. Is that okay?

Yes, absolutely.

>
> ----------------
>
> for the macroziation part:
> the lookup and read_memory callbacks should be provided with the sizes of
> the variable they need to look for or read?
> OMPD doesn't know the size of runtime data structures and can not access
> the dwarf at the runtime.

Well, there is the ompd_callback_sizeof_fn_t thing... but I admit I am
not sure how exactly they envision it should be used, especially given
things like padding in between structure fields.

> so we need to do that manually:
> #define OMPD_FOREACH_SIZEOF(OMPD_SIZEOF)\
> OMPD_SIZEOF(gomp_task_icv)\
> OMPD_SIZEOF(gomp_task)\
> OMPD_SIZEOF(gomp_team)
>
> #define OMPD_DECLARE_SIZEOF(t) __UINT64_TYPE__ ompd_sizeof__##t;
> OMPD_FOREACH_SIZEOF(OMPD_DECLARE_SIZEOF)
> #undef OMPD_DECLARE_SIZEOF
>
> we will generate these symbols as needed, so it's okay.

Again, Jakub will eventually have to accept the approach but I do not
see anything particularly wrong with it.  I just wonder whether you'll
need offsets of individual interesting fields as well.

Anyway, thanks for the encouraging news and good luck,

Martin