[PATCH] D106674: Runtime for Interop directive

[Ravi Narayanaswamy via Phabricator via cfe-commits]

RaviNarayanaswamy added inline comments.


================
Comment at: openmp/libomptarget/src/interop.cpp:194-196
+  if (device_id == -1) {
+    device_id = omp_get_default_device();
+  }
----------------
Need to check if device is available for all 3 __tgt_interop_init/use/destroy


================
Comment at: openmp/libomptarget/src/interop.cpp:198-201
+  if (interop_type == kmp_interop_type_tasksync) {
+    __kmpc_omp_wait_deps(loc_ref, gtid, ndeps, dep_list, ndeps_noalias,
+                         noalias_dep_list);
+  }
----------------
Interop object does not wait for any task from libomp.  
Init just initializes the interop object.
The initialization of interop object should  be done by the plugin as only the plugin knows what properties are supported


================
Comment at: openmp/libomptarget/src/interop.cpp:239-242
+  if (interop_val->interop_type == kmp_interop_type_tasksync) {
+    __kmpc_omp_wait_deps(loc_ref, gtid, ndeps, dep_list, ndeps_noalias,
+                         noalias_dep_list);
+  }
----------------
Need to flush the queue if interop object was created with targetsync


================
Comment at: openmp/libomptarget/src/interop.cpp:260-263
+  if (interop_val->interop_type == kmp_interop_type_tasksync) {
+    __kmpc_omp_wait_deps(loc_ref, gtid, ndeps, dep_list, ndeps_noalias,
+                         noalias_dep_list);
+  }
----------------
You don't wait for the omp tasks.  Need to flush the queue associated with the interop through the plugin


================
Comment at: openmp/runtime/src/kmp_ftn_entry.h:1449-1494
+/// TODO: Include the `omp.h` of the current build
+/* OpenMP 5.1 interop */
+typedef intptr_t omp_intptr_t;
+
+/* 0..omp_get_num_interop_properties()-1 are reserved for implementation-defined
+ * properties */
+typedef enum omp_interop_property {
----------------
Why do you have all this in openmp/runtime.  Openmp should call libomptarget to get interop properties. if libomptarget is not loaded it should return 0


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106674/new/

https://reviews.llvm.org/D106674