On Mon, May 16, 2022 at 07:35:17PM +0200, Mohamed Atef via Gcc-patches wrote: > libgomp/ChangeLog > > 2022-05-15 Mohamed Atef <mohamedatef1...@gmail.com> > > *config/darwin/plugin-suffix.h (SONAME_SUFFIX): Remove ()s. > *config/hpux/plugin-suffix.h (SONAME_SUFFIX): Remove ()s. > *config/posix/plugin-suffix.h (SONAME_SUFFIX): Remove ()s. > *configure: Regenerate. > *Makefile.am (toolexeclib_LTLIBRARIES): Add libgompd.la. > (libgompd_la_LDFLAGS, libgompd_la_DEPENDENCIES, > libgompd_la_LINK,libgompd_la_SOURCES, libgompd_version_dep, > libgompd_version_script, libgompd.ver-sun, libgompd.ver, > libgompd_version_info): New. > *Makefile.in: Regenerate. > *aclocal.m4: Regenerate. > *env.c: Include ompd-support.h. > (parse_debug): New function. > (gompd_enabled): New Variable. > (initialize_env): Call gompd_load. > (initialize_env): Call parse_debug. > *team.c: Include ompd-support.h. > (gomp_team_start): Call ompd_bp_parallel_begin. > (gomp_team_end): Call ompd_bp_parallel_end. > (gomp_thread_start): Call ompd_bp_thread_start. > *libgomp.map: ADD OMP_5.0.3 symbol versions.
Add rather than ADD > *libgompd.map: New. > *omp-tools.h.in: New. > *ompd-types.h.in: New. > *ompd-support.h: New. > *ompd-support.c: New. > *ompd-helper.h: New. > *ompd-helper.c: New. > *ompd-init.c: New. > *ompd-icv.c: New. > *configure.ac (AC_CONFIG_FILES): Add omp-tools.h and ompd-types.h. Almost ok, minor comments below. As I said earlier, as this is just partial implementation of the OMPD, I think it would be better to commit it to a git branch but already in the upstream repository, say devel/omp/ompd branch. Do you have a FSF Copyright assignment on file or do you want to submit this under DCO (https://gcc.gnu.org/dco.html)? If the latter, we need Signed-off-by line in your final patch mail and also in the commit message. The commit message should include a one line summary, then empty line, then explanation what the patch is, followed by properly formatted ChangeLog entry (the above is badly mangled by your mailer), there should be 2 spaces around the name etc. and after the ChangeLog entry the Signed-of-by line if you use DCO. More details in https://gcc.gnu.org/gitwrite.html In Authenticated access there is a link to a form to request write access to the repository, please file it and write me as the sponsor. Then follow the rest of gitwrite to e.g. do contrib/gcc-git-customization.sh and then you can git gcc-verify to verify whether the ChangeLog entry in your commit log is ok before you push it. > --- /dev/null > +++ b/libgomp/libgompd.map > @@ -0,0 +1,23 @@ > +OMPD_5.1 { Shouldn't this be OMPD_5.0 ? The functions were already in OpenMP 5.0, weren't they? > + 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; Why is ompd_bp_thread_{begin,end} here? I thought they were in libgomp.so.*, not in libgompd.so.* > + local: > + *; > +}; > + /* NO cuda for now. */ We support other kinds of offloading, so better just say /* No offloading support for now. */ or so. > + if (device == OMPD_DEVICE_KIND_HOST) > + { > + switch (icv_id) > + { > + case gompd_icv_cancellation_var: > + return > + gompd_get_cancellation ((ompd_address_space_handle_t *) handle, > + icv_value); > + case gompd_icv_max_task_priority_var: > + return gompd_get_max_task_priority ((ompd_address_space_handle_t *) > + handle, icv_value); > + case gompd_icv_stacksize_var: > + return gompd_get_stacksize ((ompd_address_space_handle_t *) handle, > + icv_value); You use that overly long (ompd_address_space_handle_t *) handle in all the spots and it causes various formatting issues. Wouldn't it be better to add some temporary variable above the switch ompd_address_space_handle_t *whatever = (ompd_address_space_handle_t *) handle; (for some good choice of name) and then just use it instead of (ompd_address_space_handle_t *) handle which would allow always writing return gompd_whatever (whatever, icv_value); and not the uglier return with call on the next line, or even worse ( on a different line from the function name? > +ompd_rc_t > +ompd_finalize () Please use (void) instead of () > +void > +gompd_load () Likewise. > +void __attribute__ ((noipa)) > +ompd_dll_locations_valid () Likewise (and several times more). Jakub
