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

[Jisheng Zhao via Phabricator via Openmp-commits]

jz10 updated this revision to Diff 469772.
jz10 added a comment.

Thanks Johannes and Shilei

1. '385-387 are the same as in omp_target_memcpy_async. Can we also not duplicate those lines?'

I put the common code (i.e.helper task creation) into another static function

2. 'In this code there are also various places with variables not named according to the style guide'

fixed, please check if there's remaining issues

3. 'The problem with the int32 Flags I mentioned already. '

The flag variable was defined as 'kmp_int32', since its consumer '__kmpc_omp_target_task_alloc' needs  'kmp_int32' type as input. the type cast to 'kmp_tasking_flags_t' is to set the 'hidden_helper' bit. So it seems that there's no better option for us. Please let me know your suggestion.

4. "Can we put all KMP related code into a separate header"

we used both kmp relevent data structure/types and APIs, so should I wrap all those relevant code into several tool functions and put them into separate header file?

5. 'aving a variable suffix with _ is generally not a good coding style'

fixed


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

https://reviews.llvm.org/D136103

Files:
  openmp/libomptarget/include/interop.h
  openmp/libomptarget/src/api.cpp
  openmp/libomptarget/src/exports
  openmp/libomptarget/src/private.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D136103.469772.patch
Type: text/x-patch
Size: 14188 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20221021/306683af/attachment-0001.bin>