On Fri, Jun 12, 2020 at 01:56:05PM -0400, y2s1982 wrote:
> This patch adds partial omp-tools.h continaing OMPD-specific functions and
> data types.
>
> 2020-06-12 Tony Sim <y2s1...@gmail.com>
You'll need two spaces before < for the script to recognize it as ChangeLog
snippet start (though, it will recognize just that libgomp/ChangeLog: as the
start).
>
> libgomp/ChangeLog:
>
> * omp-tools.h: New file
Dot at the end.
> +/* This file contains prototypes of functions and data types defined
> + in ompt and ompd standard */
in OMPT and OMPD standard. */
(capitals on the standard in the comments, and end of comment is usually
full stop followed by 2 spaces).
> +typedef uint64_t ompt_wait_id_t;
The header doesn't include <stdint.h>, so I think either we need to include
that header, or IMHO better replace all uint{N}_t uses with __UINT{N}_TYPE__
macros.
Also, ompt_wait_id_t is an OMPT type, I believe this is just a bug in the
standard and filed an issue for it. So, for the OMPD part you should
typedef __UINT64_TYPE__ ompd_wait_id_t;
instead and:
+ompd_rc_t ompd_get_state (ompd_thread_handle_t *thread_handle,
+ ompd_word_t *state, ompt_wait_id_t *wait_id)
__GOMPD_NOTHROW;
use it in this prototype.
> +typedef ompd_rc_t (*ompd_callback_memory_alloc_fn_t) (ompd_size_t nbytes,
> + void **ptr)
> __GOMPD_NOTHROW;
For brevity, I'd leave out all parameter names, omp.h doesn't have them
either.
> +
> +typedef ompd_rc_t (*ompd_callback_memory_free_fn_t) (void *ptr)
> __GOMPD_NOTHROW;
> +
> +typedef ompd_rc_t (*ompd_callback_get_thread_context_for_thread_id_fn_t) (
> + ompd_address_space_context_t *address_space_context, ompd_thread_id_t
> kind,
> + ompd_size_t sizeof_thread_id, const void *thread_id,
> + ompd_thread_context_t **thread_context) __GOMPD_NOTHROW;
I'd prefer the ( not at the end of line, but indented by two spaces on the
next line, i.e.
typedef ompd_rc_t (*ompd_callback_get_thread_context_for_thread_id_fn_t)
(ompd_address_space_context_t *, ompd_thread_id_t, ompd_size_t, const void *,
ompd_thread_context_t **) __GOMPD_NOTHROW;
(similarly for others).
> +#define ompt_id_none 0
> +#define ompt_data_none { 0 }
> +#define ompt_time_none 0
> +#define ompt_hwid_none 0
> +#define ompt_addr_none ~0
> +#define ompt_mutex_impl_none 0
> +#define ompt_wait_id_none 0
These are ompt_ defines, are they used in the OMPD part at all? If not,
they should be removed.
> +
> +#define ompd_segment_none 0
> +
> +#ifdef __cplusplus
> +} // extern "C"
> +#endif
> +
> +#endif
The header itself isn't much useful, so IMHO in the same commit you want to
also add omp-tools.h to:
nodist_libsubinclude_HEADERS (right after omp.h) and regenerate Makefile.in
with automake 1.15.1 and autoconf 2.69.
Thanks.
Jakub
