Page MenuHomePhabricator

[Clang] Initial support for linking offloading code in tool
ClosedPublic

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

Details

Summary

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
offloading.

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?

clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
414

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

428

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

460

keep exit success, makes it obvious this worked.

486

.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.

614

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

630

do we really want to return here?

jhuber6 added inline comments.Jan 31 2022, 10:07 AM
clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
414

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

428

It is addressed later.

460

Will do.

486

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

630

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.