This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Fix library path missing when using OpenMP
ClosedPublic

Authored by jhuber6 on Mar 28 2022, 9:27 AM.

Details

Summary

The changes in D122444 caused OpenMP programs built with the
LLVM_ENABLE_RUNTIMES options to stop finding the libraries. We generally
expect to link against the libraries associated with the clang
installation itself but we no longer implicitly included that directory.
This patch adds in the include path of the clang installations library
to ensure we can find them.

Diff Detail

Event Timeline

jhuber6 created this revision.Mar 28 2022, 9:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 28 2022, 9:27 AM
jhuber6 requested review of this revision.Mar 28 2022, 9:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 28 2022, 9:27 AM
jhuber6 updated this revision to Diff 418617.Mar 28 2022, 9:33 AM

IncludePath -> LibraryPath

jdoerfert accepted this revision.Mar 28 2022, 9:33 AM

s/include/library (commit message and code)

add test.

Assuming both are easy to fix, LG.

This revision is now accepted and ready to land.Mar 28 2022, 9:33 AM
MaskRay accepted this revision.Mar 28 2022, 9:39 AM

Another idea is to refactor tools::addOpenMPRuntime to use the full path of libomp.so or libomp.a instead of -LXXX -lomp.
The two ways have identical semantics when libomp.so has a DT_SONAME tag.

In the long term, perhaps libomp.so should be installed to the multiarch directory like lib/x86_64-unknown-linux-gnu/libc++.so.

clang/lib/Driver/ToolChains/CommonArgs.cpp
741

XXXIncludePath usually refers to the preprocessor include path.

Another idea is to refactor tools::addOpenMPRuntime to use the full path of libomp.so or libomp.a instead of -LXXX -lomp.
The two ways have identical semantics when libomp.so has a DT_SONAME tag.

It's technically possible for the runtime libraries to live somewhere else, I forget if any of our testers have a setup like that.

In the long term, perhaps libomp.so should be installed to the multiarch directory like lib/x86_64-unknown-linux-gnu/libc++.so.

Could you elaborate on the benefits of using the multiarch directory? I'm not familiar with it.

using the multiarch directory

If we can cross compile libomp and libomptarget to the target system. We may have
lib/x86_64-unknown-linux-gnu/libomp.so
lib/aarch64-unknown-linux-gnu/libomp.so
Compile clang once but compile runtime library for multiple architectures.

jhuber6 updated this revision to Diff 418628.Mar 28 2022, 10:02 AM

Add test.

using the multiarch directory

If we can cross compile libomp and libomptarget to the target system. We may have
lib/x86_64-unknown-linux-gnu/libomp.so
lib/aarch64-unknown-linux-gnu/libomp.so
Compile clang once but compile runtime library for multiple architectures.

Yes, mostly for deambiguating cross compiling and multilib style -m32/-m64.
In addition, I think all LLVM_ENABLE_RUNTIMES projects but openmp switched to lib/$triple/xxx.so for runtime libraries, e.g. libc++.so, libc++abi.so, libunwind.a.
By switching openmp we will improve consistency. Downstream distributions can use the same way to install runtime libraries, instead of doing different things for openmp and non-openmp.

MaskRay added inline comments.Mar 28 2022, 10:10 AM
clang/test/OpenMP/library-path.c
1

Suggest the style in linux-cross.cpp: use -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin to specify InstalledDir and remove -v.

1

Without a --target= I suspect this may fail on some targets.

jhuber6 updated this revision to Diff 418638.Mar 28 2022, 10:25 AM

Making suggested changes to test.

using the multiarch directory

If we can cross compile libomp and libomptarget to the target system. We may have
lib/x86_64-unknown-linux-gnu/libomp.so
lib/aarch64-unknown-linux-gnu/libomp.so
Compile clang once but compile runtime library for multiple architectures.

Yes, mostly for deambiguating cross compiling and multilib style -m32/-m64.
In addition, I think all LLVM_ENABLE_RUNTIMES projects but openmp switched to lib/$triple/xxx.so for runtime libraries, e.g. libc++.so, libc++abi.so, libunwind.a.
By switching openmp we will improve consistency. Downstream distributions can use the same way to install runtime libraries, instead of doing different things for openmp and non-openmp.

I'm all for it ;). Assuming @jhuber6 is not looking at that right now we need an issue to track this.

using the multiarch directory

If we can cross compile libomp and libomptarget to the target system. We may have
lib/x86_64-unknown-linux-gnu/libomp.so
lib/aarch64-unknown-linux-gnu/libomp.so
Compile clang once but compile runtime library for multiple architectures.

Yes, mostly for deambiguating cross compiling and multilib style -m32/-m64.
In addition, I think all LLVM_ENABLE_RUNTIMES projects but openmp switched to lib/$triple/xxx.so for runtime libraries, e.g. libc++.so, libc++abi.so, libunwind.a.
By switching openmp we will improve consistency. Downstream distributions can use the same way to install runtime libraries, instead of doing different things for openmp and non-openmp.

I'm all for it ;). Assuming @jhuber6 is not looking at that right now we need an issue to track this.

Sounds good, I won't look at it immediately but I could set up an issue to track the transition.

This revision was landed with ongoing or failed builds.Mar 28 2022, 11:30 AM
This revision was automatically updated to reflect the committed changes.