[Openmp-commits] [PATCH] D136103: OpenMP asynchronous memory copy support

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added inline comments.


================
Comment at: openmp/libomptarget/src/private.h:101
 typedef int kmp_int32;
+typedef int64_t kmp_int64;
 typedef intptr_t kmp_intptr_t;
----------------
Can we put all KMP related code into a separate header, but of course not called `kmp.h`? I don't think it's a good idea to have them everywhere. That's quite unfortunate that we have to duplicate the code.


================
Comment at: openmp/libomptarget/src/private.h:223
+   */
+  TargetMemcpyArgsTy(void *Dst_, const void *Src_, size_t Length_,
+                     size_t DstOffset_, size_t SrcOffset_, int DstDevice_,
----------------
Having a variable suffix with `_` is generally not a good coding style, not LLVM style. You can potentially use the same name. The compiler can tell them apart. For those values, such as `NumDims`, you could potentially do `int NumDims = 0` when defining them.


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

https://reviews.llvm.org/D136103