Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

[Clang] Initial support for linking offloading code in tool

Authored by jhuber6 on Jan 4 2022, 2:20 PM.



This patch adds the initial support for linking NVPTX offloading code
using the clang-linker-wrapper tool. This uses the extracted device
files and runs nvlink on them. Currently this is then passed to the
existing toolchain for creating linkable OpenMP offloading programs
using clang-offload-wrapper and compiling it manually using llc.
More work is required to support LTO, Bitcode linking, AMDGPU, and x86

Depends on D116545

Diff Detail

Event Timeline

jhuber6 created this revision.Jan 4 2022, 2:20 PM
jhuber6 requested review of this revision.Jan 4 2022, 2:20 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 4 2022, 2:20 PM
jhuber6 updated this revision to Diff 397589.Jan 5 2022, 8:22 AM

Small changes.

jhuber6 updated this revision to Diff 397593.Jan 5 2022, 8:34 AM

Need to tell llc to make position independent code so we can link it properly.

Same as the other commit, where is this tested?


Unsure why we look at the linker bin dir first, PATH seems to be the right choice here.


If this is not addressed in a follow up, it seems trivial to add it.


keep exit success, makes it obvious this worked.


.str() above to get a key is somewhat ok, but then parsing the key again is weird.
Can we make it a densemap and provide a densemapinfo for struct DeviceFile. It can just inherit from the DenseMapInfo<std::string>. Might even work w/o explicitly calling the "super" functions with the .str() version if we have a std::string operator. Might then even work w/o DenseMapInfo by telling the DenseMap to use the std::string specialization of DenseMapInfo in the first place.


Try to replace auto where the type is short and helpful. E.g., here it would help to know what it is.


do we really want to return here?

jhuber6 added inline comments.Jan 31 2022, 10:07 AM

Copied it from somewhere, should change it. Right now it's just a weird way to look through /bin/


It is addressed later.


Will do.


Probably a smarter idea, I needed a way to group all files with the same device file type. I'll look into it.


Might be a good idea to keep trying to remove temp files even if one fails, will address.

jhuber6 updated this revision to Diff 404598.Jan 31 2022, 10:24 AM

Maxing suggested changes.

jdoerfert accepted this revision.Jan 31 2022, 6:36 PM

LG, as part of the patch set, and given the runtime test for coverage.

Some nits are left for later though.

This revision is now accepted and ready to land.Jan 31 2022, 6:36 PM
This revision was landed with ongoing or failed builds.Jan 31 2022, 8:12 PM
This revision was automatically updated to reflect the committed changes.