[PATCH] D136103: OpenMP asynchronous memory copy support

Jisheng Zhao via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 19 14:51:07 PDT 2022


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

Thanks Johannes and Shilei

1. "So, you are saying the task_with_deps function does *not* copy the dependences and therefore the array has to outlive the function?"

I checked the omp_task_with_deps, it does some copy operations , so I move the depobj_list related code out of the TargetMemcpyArgsTy and TargetMemcpyRectArgsTy objects now. Those two classes are also redefined as 'struct'.

2. "> This would be the third location where this struct is duplicated: interop.h, kmp.h and this file. Would it make sense to try to add it to another common header file?

IIRC, there are some (non-technical) issues on using `kmp.h` in `libomptarget`."
So should I keep the data structure definitions in private.h? or create a new header file?

3. no release of Args

fixed

4. "I'm not sure if it is a failure if source and destination are same"

we thought about this, and should allow user to do this

5. "This is really not LLVM code style."

I changed the 'class' definition to 'struct' and reformat them. Please check if that works

6. few formatting issues

fixed


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

https://reviews.llvm.org/D136103

Files:
  clang/docs/ReleaseNotes.rst
  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.469051.patch
Type: text/x-patch
Size: 14974 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221019/266881c1/attachment-0001.bin>


More information about the cfe-commits mailing list