On 21/03/2025 20:17, Sandra Loosemore wrote:
On 3/21/25 11:35, Paul-Antoine Arras wrote:
Thanks Sandra and Jakub for your comments.
Here is attached an updated version of the patch:
* Removed special case for n==1, now use an array even when only one
interop object is passed.
* Updated scan dumps; added C/C++ disjunction where needed.
* Updated the signature of GOMP_interop to actual rather than generic
types.
* Renamed 'nowait' argument into 'flags' to allow for future extension.
* Added comments.
* Fixed style and formatting.
I have one more nit about this interface:
+/* Process the OpenMP interop directive. 'init' and 'destroy' take an
array
+ of 'omp_interop_t *', 'use' an array of 'omp_interop_t', where
+ 'omp_interop_t' is internally 'struct interop_obj_t *';
+ 'flag' 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,
+ void **depend)
Is it possible to make the init and destroy arrays const? I think the
right C syntax for that would be "struct interop_obj_t ** const *
init" (I admit I always find this syntax confusing, though, so don't
take my word for it without checking).
Basically, I'm thinking that if you have #pragma omp dispatch with a
variant call that implicitly constructs/destructs omp_interop_t objects,
we ought to be able to pass the same array to both GOMP_interop calls
and know that the constructor call doesn't clobber the contents of the
array itself (only the objects its elements point to.) And if you have
such a thing in a loop, the optimizers ought to be able to hoist the
array initialization completely out of the loop so that it only happens
once.
Likewise I think prefer_type could be declared as "const char * const
prefer_type"?
Does the attached patch reflect what you have in mind?
If so, I guess I can commit it soon.
--
PA
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 36ed797b0a9..54c244e0f13 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
@@ -5348,10 +5348,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;
