On Sat, Jul 25, 2020 at 11:03:27AM -0400, y2s1982 via Gcc-patches wrote:
> This patch adds few helper functions aims to improve readability of
> fetching addresses, sizes, and values. It also proposes a syntax for
> querying these information through the callback functions, similar to
> that of LLVM implementation. The syntax format is <query type>_<varaible
> name>, or <query type>_<variable type>_<member type>. '_' is the
> delimiter between fields. '<query type>', as currently defined in the
> enum, is either gompd_query_address or gompd_query_size: the first
> handles address or offset queries while the second handles the size of
> the variable/member. '<variable type>' refers to the variable type, and
> '<member type>' refers to the data type of the member of the variable.
> This code is incomplete: in particular, it currently lacks CUDA support,
> as well as segment handling, and inlining of the functions.
That assumes on the libgomp.so.1 side you want to add all the magic symbols
to the dynamic! symbol table. We do not want that, it is a big waste of
system resources that way.
Rather than that, as I said several times, there should be a single
variable, perhaps with generated content, with a compact format of data on
which the two libraries agree on and libgompd should parse and use
information from that single variable.
So I'm afraid pretty much nothing from this patch is really usable.
> +#define FOREACH_QUERYTYPE(TYPE)\
> + TYPE (gompd_query_address)\
> + TYPE (gompd_query_size)\
> +
Why the final \ ? There should be space before the \ too.
> +
> extern ompd_callbacks_t gompd_callbacks;
>
> typedef struct _ompd_aspace_handle {
> @@ -47,4 +53,51 @@ typedef struct _ompd_aspace_handle {
> ompd_size_t ref_count;
> } ompd_address_space_handle_t;
>
> +typedef enum gompd_query_type {
> +#define GENERATE_ENUM(ENUM) ENUM,
> + FOREACH_QUERYTYPE (GENERATE_ENUM)
> +#undef GENERATE_ENUM
> +} query_type;
It is unclear what you want to achieve through this, right now it is
a very fancy way to say
typedef enum gompd_query_type {
gompd_query_address,
gompd_query_size,
} query_type;
> +
> +ompd_rc_t gompd_getQueryStringSize (size_t *, query_type, const char*,
> + const char *);
GCC is not a CamelCaseShop, so please don't use such names.
Appropriate names would be gompd_get_query_string_size etc.
Jakub
