jdoerfert added a comment.
Thanks for the latest updates. The duplication is still my main concern here.
See below.
================
Comment at: openmp/libomptarget/src/api.cpp:343
+static int __kmpc_target_memcpy_rect_async_helper(kmp_int32 Gtid,
+ kmp_task_t *Task) {
+ if (Task == nullptr)
----------------
My point about the "5 character difference" was that this function is basically
the same as `__kmpc_target_memcpy_async_helper` and similarly
`omp_target_memcpy_rect_async` is almost the same as `omp_target_memcpy_async`.
Can we use common helper functions to avoid duplicating all that code twice?
(Put differently: I assume that you implemented one of these functions and then
copied it and renamed a few things to get the other. Doing that should have
made it obvious that there is duplication and we want to avoid duplication.)
================
Comment at: openmp/libomptarget/src/api.cpp:409
+ omp_depend_t DepObj = DepObj_List[i];
+ DepObjs[i] = *((kmp_depend_info_t *)DepObj);
+ }
----------------
That's not how vectors work. `push_back(...)`
================
Comment at: openmp/libomptarget/src/private.h:222
+ DstDimensions(DstDimensions_), SrcDimensions(SrcDimensions_),
+ DstDevice(DstDevice_), SrcDevice(SrcDevice_){};
+};
----------------
These structs need doxygen comments describing where they are used.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136103/new/
https://reviews.llvm.org/D136103
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
