Skip to content

[clang] [Driver] Fix respecting libdir when locating flang runtime #127345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

mgorny
Copy link
Member

@mgorny mgorny commented Feb 15, 2025

Fix tools::addFortranRuntimeLibraryPath to use
CLANG_INSTALL_LIBDIR_BASENAME rather than hardwired lib directory. This fixes flang being unable to find its runtime when installed into nonstandard directory on a system using lib64 for 64-bit libraries. While technically flang runtime could be installed with a different libdir value than clang, it seems rather unlikely.

Fix `tools::addFortranRuntimeLibraryPath` to use
`CLANG_INSTALL_LIBDIR_BASENAME` rather than hardwired `lib` directory.
This fixes flang being unable to find its runtime when installed into
nonstandard directory on a system using `lib64` for 64-bit libraries.
While technically flang runtime could be installed with a different
libdir value than clang, it seems rather unlikely.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Feb 15, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 15, 2025

@llvm/pr-subscribers-flang-runtime
@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Michał Górny (mgorny)

Changes

Fix tools::addFortranRuntimeLibraryPath to use
CLANG_INSTALL_LIBDIR_BASENAME rather than hardwired lib directory. This fixes flang being unable to find its runtime when installed into nonstandard directory on a system using lib64 for 64-bit libraries. While technically flang runtime could be installed with a different libdir value than clang, it seems rather unlikely.


Full diff: https://github.com/llvm/llvm-project/pull/127345.diff

1 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+1-1)
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 2d01943ca1ac4..47e6fd9dbf902 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -1367,7 +1367,7 @@ void tools::addFortranRuntimeLibraryPath(const ToolChain &TC,
   // lib64 instead of lib.
   SmallString<256> DefaultLibPath =
       llvm::sys::path::parent_path(TC.getDriver().Dir);
-  llvm::sys::path::append(DefaultLibPath, "lib");
+  llvm::sys::path::append(DefaultLibPath, CLANG_INSTALL_LIBDIR_BASENAME);
   if (TC.getTriple().isKnownWindowsMSVCEnvironment())
     CmdArgs.push_back(Args.MakeArgString("-libpath:" + DefaultLibPath));
   else

@MaskRay
Copy link
Member

MaskRay commented Feb 15, 2025

CLANG_INSTALL_LIBDIR_BASENAME can be "lib" or "lib64". I think it should only be used for libraries outside of llvm-project, like how cuda/amdgpu libs are installed by system. For llvm-project installed libs, it's much better to stick with a uniform "lib", which is easy to test in clang/test/Driver %clang -### and avoid platform differences (the test will not break on a different installation just because CLANG_INSTALL_LIBDIR_BASENAME is different).

I am not familiar with offload/cuda/amdgpu. It's possible that CLANG_INSTALL_LIBDIR_BASENAME gets abused (https://reviews.llvm.org/D130586 might be related) in places where it should not be used less.

@mgorny
Copy link
Member Author

mgorny commented Feb 16, 2025

But lib is used for 32-bit libraries, and lib64 for 64-bit libraries.

@mgorny
Copy link
Member Author

mgorny commented Feb 16, 2025

But lib is used for 32-bit libraries, and lib64 for 64-bit libraries.

…and just to be clear, Flang runtimes are installed into plain system CMAKE_INSTALL_LIBDIR rather than clang/lib/... path (along with the "host" Flang libraries), and they don't have any arch-suffix, so different libdirs is the only way to distinguish 32-bit and 64-bit runtime.

@mgorny
Copy link
Member Author

mgorny commented Feb 16, 2025

Hmm, but thinking about it, it should probably grab the right libdir from some multilib config rather than the hardwired build-time value.

@mgorny
Copy link
Member Author

mgorny commented Feb 16, 2025

Sigh, looks like clang's driver is a mess around lib/lib64 after all. So a proper solution would probably involve adding arch to the library name, and moving it into clang's resource directory.

@mgorny
Copy link
Member Author

mgorny commented Apr 15, 2025

Closed in favor of #134362.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category flang:runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants