This is an archive of the discontinued LLVM Phabricator instance.

Differential D140335

[OpenMP] Export `ompx::` symbols from the device runtime
ClosedPublic

Authored by jdoerfert on Dec 19 2022, 12:01 PM.

Details

Reviewers
jhuber6
tianshilei1992
Commits
rG77197b56512d: [OpenMP] Export `ompx::` symbols from the device runtime

Diff Detail

Event Timeline

jdoerfert created this revision.Dec 19 2022, 12:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 19 2022, 12:01 PM
Herald added subscribers: guansong, bollu, yaxunl. · View Herald Transcript
jdoerfert requested review of this revision.Dec 19 2022, 12:01 PM
Herald added a subscriber: sstefan1. · View Herald TranscriptDec 19 2022, 12:01 PM
tianshilei1992 added inline comments.Dec 19 2022, 12:03 PM
openmp/libomptarget/DeviceRTL/src/exports
5

I'd prefer our ompx extension to be C interfaces instead of C++.

jdoerfert added inline comments.Dec 19 2022, 12:04 PM
openmp/libomptarget/DeviceRTL/src/exports
5

Why should they be one or the other?

ompx_
and
ompx::

can (and will) coexist, no?

Harbormaster completed remote builds in B203972: Diff 484033.Dec 19 2022, 12:05 PM
jhuber6 added inline comments.Dec 19 2022, 12:05 PM
openmp/libomptarget/DeviceRTL/src/exports
5

Could we do *ompx* instead to avoid the mangled name in the list?

tianshilei1992 added inline comments.Dec 19 2022, 12:09 PM
openmp/libomptarget/DeviceRTL/src/exports
5

Could we do *ompx* instead to avoid the mangled name in the list?

I think that is even worse.

If the interface function calls will be generated by the front end, non-mangled interface functions are better. After all, name mangling is part of ABI.

jdoerfert added inline comments.Dec 19 2022, 12:09 PM
openmp/libomptarget/DeviceRTL/src/exports
5

we could, but I'd honestly prefer to restrict it to the prefix and namespace. Though, given that this is our runtime, we might be able to avoid using ompx in the middle of a symbol name.

jdoerfert added a comment.Dec 19 2022, 12:10 PM

If the interface function calls will be generated by the front end, non-mangled interface functions are better. After all, name mangling is part of ABI.

But they are not (always). The user will be able to call ompx::mapping::getThreadIdInBlock (or a renamed version of it).

tianshilei1992 added inline comments.Dec 19 2022, 12:15 PM
openmp/libomptarget/DeviceRTL/src/exports
5

If the uses are for users, that would be fine. Wonder how LLVM libraries expose their C++ interfaces.

tianshilei1992 accepted this revision.Dec 19 2022, 12:23 PM

LGTM

This revision is now accepted and ready to land.Dec 19 2022, 12:23 PM
Closed by commit rG77197b56512d: [OpenMP] Export `ompx::` symbols from the device runtime (authored by jdoerfert). · Explain WhyDec 19 2022, 2:48 PM
This revision was automatically updated to reflect the committed changes.
jdoerfert added a commit: rG77197b56512d: [OpenMP] Export `ompx::` symbols from the device runtime.
Herald added a project: Restricted Project. · View Herald TranscriptDec 19 2022, 2:48 PM
Herald added a subscriber: openmp-commits. · View Herald Transcript

Revision Contents

PathSize
openmp/
libomptarget/
DeviceRTL/
src/
2 lines

Diff 484086

openmp/libomptarget/DeviceRTL/src/exports

omp_* omp_*
*llvm_* *llvm_*
__kmpc_* __kmpc_*
_ZN4ompx*
tianshilei1992Unsubmitted
Not Done
ReplyInline Actions

I'd prefer our ompx extension to be C interfaces instead of C++.

tianshilei1992: I'd prefer our `ompx` extension to be C interfaces instead of C++.
jdoerfertAuthorUnsubmitted
Done
ReplyInline Actions

Why should they be one or the other?

ompx_
and
ompx::

can (and will) coexist, no?

jdoerfert: Why should they be one or the other? `ompx_` and `ompx::` can (and will) coexist, no?
jhuber6Unsubmitted
Not Done
ReplyInline Actions

Could we do *ompx* instead to avoid the mangled name in the list?

jhuber6: Could we do `*ompx*` instead to avoid the mangled name in the list?
tianshilei1992Unsubmitted
Not Done
ReplyInline Actions

Could we do *ompx* instead to avoid the mangled name in the list?

I think that is even worse.

If the interface function calls will be generated by the front end, non-mangled interface functions are better. After all, name mangling is part of ABI.

tianshilei1992: > Could we do *ompx* instead to avoid the mangled name in the list? I think that is even worse.
jdoerfertAuthorUnsubmitted
Done
ReplyInline Actions

we could, but I'd honestly prefer to restrict it to the prefix and namespace. Though, given that this is our runtime, we might be able to avoid using ompx in the middle of a symbol name.

jdoerfert: we could, but I'd honestly prefer to restrict it to the prefix and namespace. Though, given…
tianshilei1992Unsubmitted
Not Done
ReplyInline Actions

If the uses are for users, that would be fine. Wonder how LLVM libraries expose their C++ interfaces.

tianshilei1992: If the uses are for users, that would be fine. Wonder how LLVM libraries expose their C++…
__keep_alive __keep_alive
IsSPMDMode IsSPMDMode
memcmp memcmp
printf printf
__assert_fail __assert_fail