This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP][NFCI] Cleanup the target worksharing implementation
Needs ReviewPublic

Authored by jdoerfert on Jul 4 2019, 12:49 PM.

Details

Reviewers
openmp-commits
Summary

Note: WIP patch 3/3 to go with a RFC for the device RTL design (see D64218)

This NFCI patch includes the following cleanup steps:

  • Rename loop.cu to worksharing.cpp.
  • Adjust most of the code according to the LLVM coding style, especially wrt. variable and method names.
  • Document parts of the code with doxygen comments.
  • Wrap CUDA specific calls into __kmpc_impl_XXX functions and define them in an own target_impl.h file.
  • Use macro generators to reduce code duplication.

Event Timeline

jdoerfert created this revision.Jul 4 2019, 12:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 4 2019, 12:49 PM
Herald added a subscriber: mgorny. · View Herald Transcript
ABataev added inline comments.Jul 4 2019, 3:54 PM
openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
50

Hmm, the file has name worksharing.cpp, here it is named worksharing.cu. Did you try to compile it?

openmp/libomptarget/deviceRTLs/nvptx/src/worksharing.cpp
1

Better to mark it Cuda, since code uses cuda specific constructs

25

Maybe, it is worth it to rename functions here ti follow LLVM coding rules?

568

TidTy us always int32_t, you can hardcode it in the macro.

jdoerfert marked 2 inline comments as done.Jul 5 2019, 10:50 AM
jdoerfert added inline comments.
openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
50

I fixed it on my remote test machine but forgot to fix it locally before the commit. Same as the below leftover addition of the void return type. (I did run ninja check-all and ninja check-openmp and passed everything).

jdoerfert marked 3 inline comments as done.Jul 17 2019, 6:56 AM
jdoerfert added inline comments.
openmp/libomptarget/deviceRTLs/nvptx/src/worksharing.cpp
25

will do.

568

sure.

openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt