jz10 updated this revision to Diff 470684.
jz10 added a comment.
Thanks Shilei
1. "Can you check all resolved comments to make sure there is no open comments?"
checked through the comments you and Johannes made, no more issues
2. "Since this function is not part of `libomp` and it's not gonna b
jz10 added a subscriber: jdoerfert.
jz10 added a comment.
hi Johannes and Shilei, is there revision that needs to be done on this
patch? please let me know
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136103/new/
https://reviews.llvm.org/D136103
___
jz10 updated this revision to Diff 470357.
jz10 added a comment.
Thanks Shilei
1. "Does it work on AMDGPU and other targets? Why does it require Nvidia here?"
No, I just remove this line
2. "We don't have any performance test cases yet, and I'm not sure we need them
right now."
Yes, I remove
jz10 updated this revision to Diff 470354.
jz10 added a comment.
Thanks Shilei
Add RUN line for each test cases
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
jz10 updated this revision to Diff 470339.
jz10 added a comment.
Thanks Johannes and Shilei
I added few test cases for asynchronous routine at test/api folder
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136103/new/
https://reviews.llvm.org/D136103
Files:
openmp/libomptarget/includ
jz10 added a comment.
Sure, where should I add those tests?
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/mail
jz10 added a comment.
I checked through private.h, this header actually does the functionality
that contains all kmp and kmpc related data structures and APIs, so should
we still have to split a separate header file?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136103/new/
https://revi
jz10 added a comment.
Regarding this, can we just move those two helper functions to private.h ?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136103/new/
https://reviews.llvm.org/D136103
___
cfe-commits mailing list
cfe-commits@lists.llvm.or
jz10 updated this revision to Diff 469845.
jz10 added a comment.
Thanks Johannes
I revised those issues, please check if those work
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136103/new/
https://reviews.llvm.org/D136103
Files:
openmp/libomptarget/include/interop.h
openmp/libomp
jz10 updated this revision to Diff 469810.
jz10 added a comment.
Thanks Johannes
1. 'I feel like I'm missing something. As said before, all but 3 lines are
identical in these two functions.
Now you created a helper for 1/3 of those identical lines but left the other
2/3 being duplicated. Could
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 a
jz10 updated this revision to Diff 469719.
jz10 added a comment.
Thanks Johannes and Shilei
1. using a common helper function
fixed
2. using push_back for SmallVector
fixed
3. add doxygen comments for struct
added, please check if that works
CHANGES SINCE LAST ACTION
https://reviews.llvm
jz10 updated this revision to Diff 469328.
jz10 added a comment.
redo the proper patch, i.e. get rid of content related to clang/doc
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136103/new/
https://reviews.llvm.org/D136103
Files:
openmp/libomptarget/include/interop.h
openmp/libomp
jz10 updated this revision to Diff 469112.
jz10 added a comment.
1. use SmallVector
fixed
2. "5 characters"
ran clang-format, please check if that works
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136103/new/
https://reviews.llvm.org/D136103
Files:
clang/docs/ReleaseNotes.rst
jz10 updated this revision to Diff 469110.
jz10 added a comment.
Thanks Johannes
1. use SmallVector
fixed
2. "module 5 characters"
ran clang-format , please check it that works
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136103/new/
https://reviews.llvm.org/D136103
Files:
clang
jz10 updated this revision to Diff 469060.
jz10 added a comment.
one minors fix, i.e. delete the depobj_list from TargetMemcpyArgsTy struct
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136103/new/
https://reviews.llvm.org/D136103
Files:
clang/docs/ReleaseNotes.rst
openmp/libomptar
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 , s
jz10 added a comment.
I'm not sure if it copies, will check it to confirm
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.or
jz10 updated this revision to Diff 468695.
jz10 added a comment.
Thanks Johannes for your comments, and I relied them below
1. format issues
I ran clang-format to reformat, please check if there's any missed things;
2. replace '0' with 'nullptr'
fixed
3. proper return value for helper functio
jz10 created this revision.
jz10 added a reviewer: jdoerfert.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
jz10 requested review of this revision.
Herald added subscribers: openmp-commits, cfe-commits, sstefan1.
Herald added projects: clang, OpenMP.
We introduced the im
20 matches
Mail list logo
