[PATCH] D106674: Runtime for Interop directive

[Sri Hari Krishna Narayanan via Phabricator via llvm-commits]

sriharikrishna marked 3 inline comments as done.
sriharikrishna added inline comments.


================
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);
+  }
----------------
RaviNarayanaswamy wrote:
> Need to flush the queue if interop object was created with targetsync
A TODO has been left because flushing is not supported.


================
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);
+  }
----------------
jdoerfert wrote:
> RaviNarayanaswamy wrote:
> > You don't wait for the omp tasks.  Need to flush the queue associated with the interop through the plugin
> Waiting for omp task is necessary and the above should do it. Flushing and signaling out dependences will be added in the next revision.
A TODOs been left because flushing is not supported.


================
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 {
----------------
jdoerfert wrote:
> RaviNarayanaswamy wrote:
> > 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
> That is exactly what this code does, no? Look for libomptarget and redirect to that one, otherwise return 0.
> Unsure I see your point. FWIW, this is copied from other functions we duplicate in libomp but that actually are part of libomptarget.
Im leaving it as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106674