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

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added inline comments.


================
Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:1150-1156
+      CUresult Err = cuDeviceGet(&Dev, DeviceId);
+      if (Err == CUDA_SUCCESS) {
+        DeviceInfo->Device = reinterpret_cast<void *>(Dev);
+      } else {
+        cuGetErrorString(Err, errStr);
+        return OFFLOAD_FAIL;
+      }
----------------



================
Comment at: openmp/libomptarget/src/interop.cpp:43
+const char *getVendorIdToStr(intptr_t vendor_id) {
+  switch (vendor_id) {
+  case 1:
----------------
Does it make sense to use enum here?


================
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:
> 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.
Looks better. Can you check https://llvm.org/docs/CodingStandards.html for the code style?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106674