This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Add basic support for properly handling static libraries
ClosedPublic

Authored by jhuber6 on May 6 2022, 6:40 AM.

Details

Summary

Currently we handle static libraries like any other object in the
linker wrapper. However, this does not preserve the sematnics that
dictate static libraries should be lazily loaded as the symbols are
needed. This allows us to ignore linking in architectures that are not
used by the main application being compiled. This patch adds the basic
support for detecting if a file came from a static library, and only
including it in the link job if it's used by other object files.

This patch only adds the basic support, to be more correct we should
check the symbols and only inclue the library if the link job contains
symbols that are needed. Ideally we could just put this on the linker
itself, but nvlink doesn't seem to support .a files.

Diff Detail

Event Timeline

jhuber6 created this revision.May 6 2022, 6:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 6 2022, 6:40 AM
jhuber6 requested review of this revision.May 6 2022, 6:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 6 2022, 6:40 AM
jdoerfert accepted this revision.May 6 2022, 8:00 AM

Typo(s) in the commit message, otherwise this seems reasonable. LG

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

Nit: count?

This revision is now accepted and ready to land.May 6 2022, 8:00 AM

Ideally we could just put this on the linker itself, but nvlink doesn't seem to support .a files.

Yes, nvlink does not support archives. So we used a wrapper to extract cubin files from the archive and pass them to nvlink. Please see, Clang Nvlink Wrapper.

Ideally we could just put this on the linker itself, but nvlink doesn't seem to support .a files.

Yes, nvlink does not support archives. So we used a wrapper to extract cubin files from the archive and pass them to nvlink. Please see, Clang Nvlink Wrapper.

Yeah, it seems we'll need to do a manual work-around later to handle the following case:

// foo.o
void foo() { return; }
//libfoo.a
void foo() { return; }
clang foo.o libfoo.a // no conflict, symbol foo isn't undefined in foo.o so libfoo.a is never loaded
nvlink foo.cubin libfoo.cubin // conflict, both are loaded and linked together

This is a little bit of an edge-case so I just went for the basic support here, since AMD just uses lld we could just pass in libfoo.a and it should handle it correctly for us. Add it to the list of Nvidia problems we need to work around externally.