[Openmp-commits] [PATCH] D106674: Runtime for Interop directive

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added inline comments.


================
Comment at: openmp/libomptarget/src/interop.cpp:13
+static omp_interop_rc_t
+__kmpc_interop_get_property_err_type(omp_interop_property_t property) {
+  switch (property) {
----------------
tianshilei1992 wrote:
> There are some conventions in current `libomptarget`.
> 1. If a function is internal, use LLVM code style.
> 2. If a function is exported and exposed to compiler, it should be `extern "C"` and use code style similar to existing functions whose name prefix with `__tgt`.
> 
> So basically, if these functions are only visible to this file, please format it with LLVM code style, and use anonymous name space.
I mean, this function doesn't have to start with `__tgt` because it is internal. Functions starting with `__tgt` are usually interface functions. From my perspective, it is better to name it as `getPropertyErrorType`, a.k.a. to use LLVM code style.


================
Comment at: openmp/libomptarget/src/interop.cpp:63
+template <typename PropertyTy>
+PropertyTy __tgt_interop_get_property(omp_interop_val_t &interop_val,
+                                      omp_interop_property_t property,
----------------
Same here


================
Comment at: openmp/runtime/src/kmp_ftn_entry.h:1501
+    return (*fptr)(interop);
+  } else { // liboffload & libomptarget don't exist
+    return 0;
----------------
ditto


================
Comment at: openmp/runtime/src/kmp_ftn_entry.h:1515
+    return (*fptr)(interop, property_id, err);
+  } else { // liboffload & libomptarget don't exist
+    return 0;
----------------
ditto


================
Comment at: openmp/runtime/src/kmp_ftn_entry.h:1527
+    return (*fptr)(interop, property_id, err);
+  } else { // liboffload & libomptarget don't exist
+    return (void *)0;
----------------
ditto


================
Comment at: openmp/runtime/src/kmp_ftn_entry.h:1539
+    return (*fptr)(interop, property_id, err);
+  } else { // liboffload & libomptarget don't exist
+    return (const char *)0;
----------------
ditto


================
Comment at: openmp/runtime/src/kmp_ftn_entry.h:1550
+    return (*fptr)(interop, property_id);
+  } else { // liboffload & libomptarget don't exist
+    return (const char *)0;
----------------
same as below


================
Comment at: openmp/runtime/src/kmp_ftn_entry.h:1561
+    return (*fptr)(interop, property_id);
+  } else { // liboffload & libomptarget don't exist
+    return (const char *)0;
----------------
same as below


================
Comment at: openmp/runtime/src/kmp_ftn_entry.h:1570-1574
+  if ((*(void **)(&fptr) = KMP_DLSYM_NEXT("omp_get_interop_rec_desc"))) {
+    return (*fptr)(interop, property_id);
+  } else { // liboffload & libomptarget don't exist
+    return (const char *)0;
+  }
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106674