Page MenuHomePhabricator

[CUDA][HIP] Use device side kernel and variable names when registering them
ClosedPublic

Authored by yaxunl on Feb 12 2019, 7:57 PM.

Details

Summary

__hipRegisterFunction and __hipRegisterVar need to accept device side kernel and variable names
so that HIP runtime can associate kernel stub functions in host code with kernel symbols in fat binaries,
and associate shadow variables in host code with device variables in fat binaries.

Currently, clang assumes kernel functions and device variables have the same name as the kernel
stub functions and shadow variables. However, when host is compiled in windows with MSVC C++
ABI and device is compiled with Itanium C++ ABI (e.g. AMDGPU), kernels and device symbols in fat
binary are mangled differently than host.

This patch gets the device side kernel and variable name by mangling them in the mangle context
of aux target.

Diff Detail

Repository
rC Clang

Event Timeline

yaxunl created this revision.Feb 12 2019, 7:57 PM
tra added inline comments.Feb 13 2019, 10:27 AM
lib/CodeGen/CGCUDANV.cpp
128

Nit: VarInfo is not needed. Compiler should be able to infer it.

403

Can we just call getDeviceSideName() here ? Mangling the name early and carrying it around does not seem to buy us anything.

yaxunl marked an inline comment as done.Feb 13 2019, 11:08 AM
yaxunl added inline comments.
lib/CodeGen/CGCUDANV.cpp
403

getDeviceSideName() need to know Decl of the function. If we want to call it here, we have to carry Decl of the functions around.

tra accepted this revision.Feb 13 2019, 11:25 AM
tra added inline comments.
lib/CodeGen/CGCUDANV.cpp
403

I see. We appear to get both the Decl and llvm::function from CodeGenFunction.
Perhaps we can make EmittedKernels a vector<CodeGenFunction*>.

It's just a nit. I'll leave it up to you.

This revision is now accepted and ready to land.Feb 13 2019, 11:25 AM
yaxunl marked an inline comment as done.Feb 13 2019, 11:27 AM
yaxunl added inline comments.
lib/CodeGen/CGCUDANV.cpp
403

Carrying Decl around is better than carrying string around since it can save some space. Will do it.

yaxunl updated this revision to Diff 186711.Feb 13 2019, 11:42 AM

Revised by Artem's comments.

tra accepted this revision.Feb 13 2019, 11:45 AM

Thank you. LGTM.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 13 2019, 5:59 PM