On 25/03/2025 18:55, Sandra Loosemore wrote:
On 3/25/25 09:25, Paul-Antoine Arras wrote:
On 24/03/2025 21:17, Sandra Loosemore wrote:
[snip]
Besides, I am not sure how to encode complex types like (**const *). Does that require creating new definitions in gcc/builtin-types.def and gcc/fortran/types.def?

I don't understand what the Fortran front end has to do with this, BUILT_IN_GOMP_INTEROP is only referenced from gimplify.cc and omp-low.cc.

DEF_GOMP_BUILTIN defines builtins for both C/C++ and Fortran. If a type definition is missing in gcc/fortran/types.def, you get an error, e.g.:

gcc/fortran/../omp-builtins.def: In function 'void gfc_init_builtin_functions()': gcc/fortran/../omp-builtins.def:406:19: error: 'BT_FN_VOID_INT_INT_PTRPTRCONSTPTR_CONSTPTR_PTRCONSTSTRING_INT_PTRPTR_INT_PTRPTRCONSTPTR_UINT_PTRPTR' was not declared in this scope 406 | BT_FN_VOID_INT_INT_PTRPTRCONSTPTR_CONSTPTR_PTRCONSTSTRING_INT_PTRPTR_INT_PTRPTRCONSTPTR_UINT_PTRPTR, | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ gcc/fortran/f95-lang.cc:1291:60: note: in definition of macro 'DEF_GOMP_BUILTIN'
 1291 |       gfc_define_builtin ("__builtin_" name, builtin_types[type], \
      |                                                            ^~~~

And what difference does it make to have an argument declared as BT_PTR instead of, say, BT_PTR_CONST_PTR_PTR? Is is just a matter of optimisation?
If someone could shed some light...

Well, the declaration used by GCC for code generation should match the definition in the library, right?

Mainly, I got to thinking about this because the "declare variant" interop creation/destruction code that Tobias had sketched out before handing it over to me included a bunch of explicit clobbers that confused the heck out of me.  AFAICT, the only thing that GOMP_interop needs to modify is the actual interop objects whose pointers are in the init/destroy arrays, so in my final version of the code those are the only things clobbered after the interop objects are destroyed.  If GCC knows the arrays themselves are const, that potentially also enables optimizations like hoisting the code to set up the argument arrays out a loop containing a variant call.

I updated the patch (see attachment) with that in mind. Let me know what you think.
--
PA
commit c379d33e55fb7af4d560ae912b9395e33b723b9f
Author: Paul-Antoine Arras <par...@baylibre.com>
Date:   Thu Mar 27 17:01:41 2025 +0100

    OpenMP: interop - make arrays const
    
    Treat arrays passed to GOMP_interop as const.
    
    gcc/ChangeLog:
    
            * builtin-types.def (BT_PTR_CONST_PTR_PTR): Define.
            (BT_FN_VOID_INT_INT_PTR_PTR_PTR_INT_PTR_INT_PTR_UINT_PTR): Define.
            * omp-builtins.def (BUILT_IN_GOMP_INTEROP): Update.
    
    gcc/fortran/ChangeLog:
    
            * types.def (BT_FN_VOID_INT_INT_PTR_PTR_PTR_INT_PTR_INT_PTR_UINT_PTR):
            Define.
            (BT_FN_VOID_INT_INT_PTRCONSTPTRPTR_CONSTPTR_PTRCONSTSTRING_INT_PTRPTR_INT_PTRCONSTPTRPTR_UINT_PTRPTR):
            Define.
    
    libgomp/ChangeLog:
    
            * libgomp_g.h (GOMP_interop): Make arrays const.
            * target.c (struct interop_data_t): Likewise.
            (GOMP_interop): Likewise.

diff --git gcc/builtin-types.def gcc/builtin-types.def
index 9583d30dfc0..c7b2e1619d1 100644
--- gcc/builtin-types.def
+++ gcc/builtin-types.def
@@ -221,6 +221,11 @@ DEF_PRIMITIVE_TYPE (BT_PTR_CONST_STRING,
 		     (build_qualified_type (string_type_node,
 					    TYPE_QUAL_CONST)))
 
+/* The C type `void **const *'.  */
+DEF_PRIMITIVE_TYPE (BT_PTR_CONST_PTR_PTR,
+		    build_pointer_type (build_qualified_type (
+		      build_pointer_type (ptr_type_node), TYPE_QUAL_CONST)))
+
 DEF_POINTER_TYPE (BT_PTR_UINT, BT_UINT)
 DEF_POINTER_TYPE (BT_PTR_LONG, BT_LONG)
 DEF_POINTER_TYPE (BT_PTR_ULONG, BT_ULONG)
@@ -1015,9 +1020,11 @@ DEF_FUNCTION_TYPE_11 (BT_FN_VOID_OMPFN_PTR_OMPCPYFN_LONG_LONG_UINT_LONG_INT_ULL_
 		      BT_PTR_FN_VOID_PTR_PTR, BT_LONG, BT_LONG,
 		      BT_UINT, BT_LONG, BT_INT,
 		      BT_ULONGLONG, BT_ULONGLONG, BT_ULONGLONG)
-DEF_FUNCTION_TYPE_11 (BT_FN_VOID_INT_INT_PTR_PTR_PTR_INT_PTR_INT_PTR_UINT_PTR,
-		      BT_VOID, BT_INT, BT_INT, BT_PTR, BT_PTR, BT_PTR, BT_INT,
-		      BT_PTR, BT_INT, BT_PTR, BT_UINT, BT_PTR)
+DEF_FUNCTION_TYPE_11 (
+  BT_FN_VOID_INT_INT_PTRCONSTPTRPTR_CONSTPTR_PTRCONSTSTRING_INT_PTRPTR_INT_PTRCONSTPTRPTR_UINT_PTRPTR,
+  BT_VOID, BT_INT, BT_INT, BT_PTR_CONST_PTR_PTR, BT_CONST_PTR,
+  BT_PTR_CONST_STRING, BT_INT, BT_PTR_PTR, BT_INT, BT_PTR_CONST_PTR_PTR,
+  BT_UINT, BT_PTR_PTR)
 
 DEF_FUNCTION_TYPE_VAR_0 (BT_FN_VOID_VAR, BT_VOID)
 DEF_FUNCTION_TYPE_VAR_0 (BT_FN_INT_VAR, BT_INT)
diff --git gcc/fortran/types.def gcc/fortran/types.def
index dd9b8df59be..d68d02e819d 100644
--- gcc/fortran/types.def
+++ gcc/fortran/types.def
@@ -266,7 +266,7 @@ DEF_FUNCTION_TYPE_11 (BT_FN_VOID_OMPFN_PTR_OMPCPYFN_LONG_LONG_UINT_LONG_INT_ULL_
 		      BT_PTR_FN_VOID_PTR_PTR, BT_LONG, BT_LONG,
 		      BT_UINT, BT_LONG, BT_INT,
 		      BT_ULONGLONG, BT_ULONGLONG, BT_ULONGLONG)
-DEF_FUNCTION_TYPE_11 (BT_FN_VOID_INT_INT_PTR_PTR_PTR_INT_PTR_INT_PTR_UINT_PTR,
+DEF_FUNCTION_TYPE_11 (BT_FN_VOID_INT_INT_PTRCONSTPTRPTR_CONSTPTR_PTRCONSTSTRING_INT_PTRPTR_INT_PTRCONSTPTRPTR_UINT_PTRPTR,
 		      BT_VOID, BT_INT, BT_INT, BT_PTR, BT_PTR, BT_PTR, BT_INT,
 		      BT_PTR, BT_INT, BT_PTR, BT_UINT, BT_PTR)
 
diff --git gcc/omp-builtins.def gcc/omp-builtins.def
index f73fb7b9dd8..1ca4eb068cc 100644
--- gcc/omp-builtins.def
+++ gcc/omp-builtins.def
@@ -402,9 +402,10 @@ DEF_GOMP_BUILTIN (BUILT_IN_GOMP_DOACROSS_ULL_POST, "GOMP_doacross_ull_post",
 		  BT_FN_VOID_PTR, ATTR_NOTHROW_LEAF_LIST)
 DEF_GOMP_BUILTIN (BUILT_IN_GOMP_DOACROSS_ULL_WAIT, "GOMP_doacross_ull_wait",
 		  BT_FN_VOID_ULL_VAR, ATTR_NOTHROW_LEAF_LIST)
-DEF_GOMP_BUILTIN (BUILT_IN_GOMP_INTEROP, "GOMP_interop",
-		  BT_FN_VOID_INT_INT_PTR_PTR_PTR_INT_PTR_INT_PTR_UINT_PTR,
-		  ATTR_NOTHROW_LIST)
+DEF_GOMP_BUILTIN (
+  BUILT_IN_GOMP_INTEROP, "GOMP_interop",
+  BT_FN_VOID_INT_INT_PTRCONSTPTRPTR_CONSTPTR_PTRCONSTSTRING_INT_PTRPTR_INT_PTRCONSTPTRPTR_UINT_PTRPTR,
+  ATTR_NOTHROW_LIST)
 DEF_GOMP_BUILTIN (BUILT_IN_GOMP_PARALLEL, "GOMP_parallel",
 		  BT_FN_VOID_OMPFN_PTR_UINT_UINT, ATTR_NOTHROW_LIST)
 DEF_GOMP_BUILTIN (BUILT_IN_GOMP_PARALLEL_REDUCTIONS,
diff --git libgomp/libgomp_g.h libgomp/libgomp_g.h
index 8993ec610fb..274f4937680 100644
--- libgomp/libgomp_g.h
+++ libgomp/libgomp_g.h
@@ -359,9 +359,10 @@ extern void GOMP_teams (unsigned int, unsigned int);
 extern bool GOMP_teams4 (unsigned int, unsigned int, unsigned int, bool);
 extern void *GOMP_target_map_indirect_ptr (void *);
 struct interop_obj_t;
-extern void GOMP_interop (int, int, struct interop_obj_t ***, const int *,
-			  const char **, int, struct interop_obj_t **, int,
-			  struct interop_obj_t ***, unsigned, void **);
+extern void GOMP_interop (int, int, struct interop_obj_t **const *, const int *,
+			  const char *const *, int, struct interop_obj_t **,
+			  int, struct interop_obj_t **const *, unsigned,
+			  void **);
 
 /* teams.c */
 
diff --git libgomp/target.c libgomp/target.c
index a64ee96af2a..1490fa7aa6b 100644
--- libgomp/target.c
+++ libgomp/target.c
@@ -5279,11 +5279,11 @@ ialias (omp_get_interop_rc_desc)
 struct interop_data_t
 {
   int device_num, n_init, n_use, n_destroy;
-  struct interop_obj_t ***init;
+  struct interop_obj_t **const *init;
   struct interop_obj_t **use;
-  struct interop_obj_t ***destroy;
+  struct interop_obj_t **const *destroy;
   const int *target_targetsync;
-  const char **prefer_type;
+  const char *const *prefer_type;
 };
 
 static void
@@ -5349,10 +5349,10 @@ gomp_interop_internal (void *data)
    'flags' is used for the 'nowait' clause.  */
 
 void
-GOMP_interop (int device_num, int n_init, struct interop_obj_t ***init,
-	      const int *target_targetsync, const char **prefer_type, int n_use,
-	      struct interop_obj_t **use, int n_destroy,
-	      struct interop_obj_t ***destroy, unsigned int flags,
+GOMP_interop (int device_num, int n_init, struct interop_obj_t **const *init,
+	      const int *target_targetsync, const char *const *prefer_type,
+	      int n_use, struct interop_obj_t **use, int n_destroy,
+	      struct interop_obj_t **const *destroy, unsigned int flags,
 	      void **depend)
 {
   struct interop_data_t args;

Reply via email to