[OpenMP] Fix library path missing when using OpenMP

[jhuber6]

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.

[jhuber6]

IncludePath -> LibraryPath

[jdoerfert]

s/include/library (commit message and code)

add test.

Assuming both are easy to fix, LG.

[MaskRay]

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.

[jhuber6]

In D122592#3411741, @MaskRay wrote:

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.

[ye-luo]

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]

Add test.

[MaskRay]

In D122592#3411795, @ye-luo wrote:

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]

clang/test/OpenMP/library-path.c

1 ↗

(On Diff #418628)

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 ↗

(On Diff #418628)

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

[jhuber6]

Making suggested changes to test.

[jdoerfert]

In D122592#3411812, @MaskRay wrote:

In D122592#3411795, @ye-luo wrote:

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.

[jhuber6]

In D122592#3411877, @jdoerfert wrote:

In D122592#3411812, @MaskRay wrote:

In D122592#3411795, @ye-luo wrote:

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.

clang/lib/Driver/ToolChains/CommonArgs.h

void AddAssemblerKPIC(const ToolChain &ToolChain,
                       const llvm::opt::ArgList &Args,
                       llvm::opt::ArgStringList &CmdArgs);
 
 void addOpenMPRuntimeSpecificRPath(const ToolChain &TC,
                                    const llvm::opt::ArgList &Args,
                                    llvm::opt::ArgStringList &CmdArgs);
 void addArchSpecificRPath(const ToolChain &TC, const llvm::opt::ArgList &Args,
                           llvm::opt::ArgStringList &CmdArgs);
+void addOpenMPRuntimeLibraryPath(const ToolChain &TC,
+                                 const llvm::opt::ArgList &Args,
+                                 llvm::opt::ArgStringList &CmdArgs);
 /// Returns true, if an OpenMP runtime has been added.
 bool addOpenMPRuntime(llvm::opt::ArgStringList &CmdArgs, const ToolChain &TC,
                       const llvm::opt::ArgList &Args,
                       bool ForceStaticHostRuntime = false,
                       bool IsOffloadingHost = false, bool GompNeedsRT = false);
 
 const char *getAsNeededOption(const ToolChain &TC, bool as_needed);
 

clang/lib/Driver/ToolChains/CommonArgs.cpp

if (Args.hasFlag(options::OPT_fopenmp_implicit_rpath,
     SmallString<256> DefaultLibPath =
         llvm::sys::path::parent_path(TC.getDriver().Dir);
     llvm::sys::path::append(DefaultLibPath, Twine("lib") + CLANG_LIBDIR_SUFFIX);
     CmdArgs.push_back("-rpath");
     CmdArgs.push_back(Args.MakeArgString(DefaultLibPath));
   }
 }
 
+void tools::addOpenMPRuntimeLibraryPath(const ToolChain &TC,
+                                        const ArgList &Args,
+                                        ArgStringList &CmdArgs) {
+  // Default to clang lib / lib64 folder, i.e. the same location as device
+  // runtime.
+  SmallString<256> DefaultLibPath =
+      llvm::sys::path::parent_path(TC.getDriver().Dir);
+  llvm::sys::path::append(DefaultLibPath, Twine("lib") + CLANG_LIBDIR_SUFFIX);
+  CmdArgs.push_back(Args.MakeArgString("-L" + DefaultLibPath));
+}
+
 void tools::addArchSpecificRPath(const ToolChain &TC, const ArgList &Args,
                                  ArgStringList &CmdArgs) {
   // Enable -frtlib-add-rpath by default for the case of VE.
   const bool IsVE = TC.getTriple().isVE();
   bool DefaultValue = IsVE;
   if (!Args.hasFlag(options::OPT_frtlib_add_rpath,
                     options::OPT_fno_rtlib_add_rpath, DefaultValue))
     return;
bool tools::addOpenMPRuntime(ArgStringList &CmdArgs, const ToolChain &TC,
 
   if (IsOffloadingHost)
     CmdArgs.push_back("-lomptarget");
 
   addArchSpecificRPath(TC, Args, CmdArgs);
 
   if (RTKind == Driver::OMPRT_OMP)
     addOpenMPRuntimeSpecificRPath(TC, Args, CmdArgs);
+  addOpenMPRuntimeLibraryPath(TC, Args, CmdArgs);
 
   return true;
 }
 
 static void addSanitizerRuntime(const ToolChain &TC, const ArgList &Args,
                                 ArgStringList &CmdArgs, StringRef Sanitizer,
                                 bool IsShared, bool IsWhole) {
   // Wrap any static runtimes that must be forced into executable in

clang/test/Driver/openmp-library-path.c

+// RUN: %clang -### -fopenmp=libomp --target=x86_64-linux-gnu -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin %s 2>&1 | FileCheck %s
+
+void foo() {}
+
+// CHECK: -L{{.*}}Inputs{{.*}}basic_linux_tree