On 3/18/25 08:36, Paul-Antoine Arras wrote:
This patch partially enables use of the OpenMP interop construct by adding
middle end support, mostly in the omplower pass, and in the target-independent
part of the libgomp runtime. It follows up on previous patches for C, C++ and
Fortran front ends support. The full interop feature requires another patch to
enable foreign runtime support in libgomp plugins.

gcc/ChangeLog:

        * builtin-types.def
        (BT_FN_VOID_INT_INT_PTR_PTR_PTR_INT_PTR_INT_PTR_UINT_PTR): New.
        * gimple-low.cc (lower_stmt): Handle GIMPLE_OMP_INTEROP.
        * gimple-pretty-print.cc (dump_gimple_omp_interop): New function.
        (pp_gimple_stmt_1): Handle GIMPLE_OMP_INTEROP.
        * gimple.cc (gimple_build_omp_interop): New function.
        (gimple_copy): Handle GIMPLE_OMP_INTEROP.
        * gimple.def (GIMPLE_OMP_INTEROP): Define.
        * gimple.h (gimple_build_omp_interop): Declare.
        (gimple_omp_interop_clauses): New function.
        (gimple_omp_interop_clauses_ptr): Likewise.
        (gimple_omp_interop_set_clauses): Likewise.
        (gimple_return_set_retval): Handle GIMPLE_OMP_INTEROP.
        * gimplify.cc (gimplify_scan_omp_clauses): Handle OMP_CLAUSE_INIT,
        OMP_CLAUSE_USE and OMP_CLAUSE_DESTROY.
        (gimplify_omp_interop): New function.
        (gimplify_expr): Replace sorry with call to gimplify_omp_interop.
        * omp-builtins.def (BUILT_IN_GOMP_INTEROP): Define.
        * omp-low.cc (scan_sharing_clauses): Handle OMP_CLAUSE_INIT,
        OMP_CLAUSE_USE and OMP_CLAUSE_DESTROY.
        (scan_omp_1_stmt): Handle GIMPLE_OMP_INTEROP.
        (lower_omp_interop_action_clauses): New function.
        (lower_omp_interop): Likewise.
        (lower_omp_1): Handle GIMPLE_OMP_INTEROP.

gcc/c/ChangeLog:

        * c-parser.cc (c_parser_omp_clause_destroy): Make addressable.
        (c_parser_omp_clause_init): Make addressable.

gcc/cp/ChangeLog:

        * parser.cc (cp_parser_omp_clause_init): Make addressable.

gcc/fortran/ChangeLog:

        * trans-openmp.cc (gfc_trans_omp_clauses): Make OMP_CLAUSE_DESTROY and
        OMP_CLAUSE_INIT addressable.
        * types.def (BT_FN_VOID_INT_INT_PTR_PTR_PTR_INT_PTR_INT_PTR_UINT_PTR):
        New.

include/ChangeLog:

        * gomp-constants.h (GOMP_DEVICE_DEFAULT_OMP_61): Define.
        (GOMP_INTEROP_FLAG_TARGET): Define.
        (GOMP_INTEROP_FLAG_TARGETSYNC): Define.

libgomp/ChangeLog:

        * icv-device.c (omp_set_default_device): Check
        GOMP_DEVICE_DEFAULT_OMP_61.
        * libgomp-plugin.h (struct interop_obj_t): New.
        (enum gomp_interop_flag): New.
        (GOMP_OFFLOAD_interop): Declare.
        (GOMP_OFFLOAD_get_interop_int): Declare.
        (GOMP_OFFLOAD_get_interop_ptr): Declare.
        (GOMP_OFFLOAD_get_interop_str): Declare.
        (GOMP_OFFLOAD_get_interop_type_desc): Declare.
        * libgomp.h (_LIBGOMP_OMP_LOCK_DEFINED): Define.
        (struct gomp_device_descr): Add interop_func, get_interop_int_func,
        get_interop_ptr_func, get_interop_str_func, get_interop_type_desc_func.
        * libgomp.map: Add GOMP_interop.
        * libgomp_g.h (GOMP_interop): Declare.
        * target.c (resolve_device): Handle GOMP_DEVICE_DEFAULT_OMP_61.
        (omp_get_interop_int): Replace stub with actual implementation.
        (omp_get_interop_ptr): Likewise.
        (omp_get_interop_str): Likewise.
        (omp_get_interop_type_desc): Likewise.
        (struct interop_data_t): Define.
        (gomp_interop_internal): New function.
        (GOMP_interop): Likewise.
        (gomp_load_plugin_for_device): Load symbols for get_interop_int,
        get_interop_ptr, get_interop_str and get_interop_type_desc.
        * testsuite/libgomp.c-c++-common/interop-1.c: New test.

gcc/testsuite/ChangeLog:

        * c-c++-common/gomp/interop-1.c: Remove dg-prune-output "sorry".
        * c-c++-common/gomp/interop-2.c: Likewise.
        * c-c++-common/gomp/interop-3.c: Likewise.
        * c-c++-common/gomp/interop-4.c: Remove dg-message "not supported".
        * g++.dg/gomp/interop-5.C: Likewise.
        * gfortran.dg/gomp/interop-4.f90: Likewise.
        * c-c++-common/gomp/interop-5.c: New test.
        * gfortran.dg/gomp/interop-5.f90: New test.

I have a general complaint, which applies most particularly to this piece in libgomp/target.c:

+
+void
+GOMP_interop (int device_num, int n_init, void *init,
+             const void *target_targetsync, const void *prefer_type,
+             int n_use, void *use, int n_destroy, void *destroy,
+             unsigned int nowait, void **depend)
+{

The GNU coding standards say:

"Please put a comment on each function saying what the function does, what sorts of arguments it gets, and what the possible values of arguments mean and are used for. It is not necessary to duplicate in words the meaning of the C argument declarations, if a C type is being used in its customary fashion. If there is anything nonstandard about its use (such as an argument of type char * which is really the address of the second character of a string, not the first), or any possible values that would not work the way one would expect (such as, that strings containing newlines are not guaranteed to work), be sure to say so."

Here we certainly have nonstandard use of C types (those void * arguments, which might either be a pointer to a thing or a pointer to an array of pointers to things, at least in the case of prefer_type the thing turns out to be some sort of byte encoding represented in either a char * or char **). On top of that, this routine is an ABI between the compiler and library and is especially important to document for that reason as well.

I'm also thinking this is a pretty messy interface for an ABI... I'm not sure where the bottlenecks are but it seems like having code gen for the OpenMP constructs that call this routine create a stack variable to hold the "pointer to thing" and uniformly making the interface "thing**" would not add as much overhead as needing to handle both "thing*" and "thing **" at runtime. Plus there is a benefit to using the actual type of the argument in the function declaration in terms of making the code on both the GCC and libgomp sides simpler, more understandable, and and more maintainable.

-Sandra

Reply via email to