This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP][NFC] Use `AsyncInfo` as the variable name for a `__tgt_async_info`
ClosedPublic

Authored by jdoerfert on Feb 10 2021, 12:07 PM.

Diff Detail

Event Timeline

jdoerfert created this revision.Feb 10 2021, 12:07 PM
jdoerfert requested review of this revision.Feb 10 2021, 12:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2021, 12:07 PM
Herald added a subscriber: sstefan1. · View Herald Transcript
tianshilei1992 accepted this revision.Feb 10 2021, 2:13 PM

LG with one nit.

openmp/libomptarget/plugins/cuda/src/rtl.cpp
1205

These functions are C interfaces. Do we want to use C style here?

This revision is now accepted and ready to land.Feb 10 2021, 2:13 PM
jdoerfert added inline comments.Feb 10 2021, 3:06 PM
openmp/libomptarget/plugins/cuda/src/rtl.cpp
1205

Because sed apparently doesn't distinguish. I try to manually go through and adjust before committing.

jdoerfert marked an inline comment as done.

Keep "C spelling" for C-interfaces and only modify the internal C++ code.

grokos accepted this revision.Feb 16 2021, 4:18 PM

NFC patch, naming in line with the rest of the library, good to go.

This revision was landed with ongoing or failed builds.Mar 11 2021, 9:31 PM
This revision was automatically updated to reflect the committed changes.
rogfer01 added inline comments.
openmp/libomptarget/plugins/remote/server/Server.cpp
167

I don't think this is caused by this patch, but I'm unable to build this using clang trunk (I'm building openmp as a runtime).

/work/llvm-src/openmp/libomptarget/plugins/remote/server/Server.cpp:169:9: error: non-const lvalue reference to type 'AsyncInfoTy' cannot bind to a temporary of type '__tgt_async_info *'
        (__tgt_async_info *)AsyncInfo));
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/work/llvm-src/openmp/libomptarget/src/device.h:223:36: note: passing argument to parameter 'AsyncInfo' here
  int32_t synchronize(AsyncInfoTy &AsyncInfo);
                                   ^

Has anyone else run into this? What version of clang (or gcc?) do you commonly use to build this code? Thanks!

jdoerfert added inline comments.
openmp/libomptarget/plugins/remote/server/Server.cpp
167

@atmnpatel was looking into this. I forgot to rename that part.