This is an archive of the discontinued LLVM Phabricator instance.

[libomptarget] Add support for target memory allocators to cuda RTL
ClosedPublic

Authored by jhuber6 on May 6 2021, 9:47 AM.

Details

Summary

The allocator interface added in D97883 allows the RTL to allocate shared and
host-pinned memory from the cuda plugin. This patch adds support for these to
the runtime.

Diff Detail

Event Timeline

jhuber6 created this revision.May 6 2021, 9:47 AM
jhuber6 requested review of this revision.May 6 2021, 9:47 AM

Two nits, otherwise this looks sensible to me.

openmp/libomptarget/plugins/common/MemoryManager/MemoryManager.h
34

can we have an enum not a int32_t, also variable name Kind

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

if everything but alloc_host goes to cumemfree, only put alloc_host in the set. If it is not in the set, it's cumemfree teritory.

jhuber6 updated this revision to Diff 343457.May 6 2021, 10:42 AM

Making suggested changes.

grokos accepted this revision.May 6 2021, 10:50 AM

Looks good (with one naming nit)

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

Can you rename AllocKinds to something like HostPinnedAllocs? AllocKinds does not make sense anymore after the latest change.

This revision is now accepted and ready to land.May 6 2021, 10:50 AM
jhuber6 updated this revision to Diff 343462.May 6 2021, 11:15 AM
This revision was landed with ongoing or failed builds.May 7 2021, 7:29 AM
This revision was automatically updated to reflect the committed changes.
xiangzhangllvm added inline comments.
openmp/libomptarget/plugins/cuda/src/rtl.cpp
327

Sorry, where is the def of "cuMemAllocHost"? I didn't find it.

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

cuMemFreeHost needs to be added to dynamic_cuda/cuda.h. Possibly other functions too, don't have this building successfully yet