-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[Flang] Search flang_rt in clang_rt path #151954
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
Conversation
get_toolchain_arch_dirname(arch_dirname) | ||
set(outval "${outval}/${arch_dirname}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On my machine, both compiler-rt and flang-rt libraries are installed into <install_dir>/lib/clang/22/lib/darwin
.
IIUC, this change would make flang-rt libraries be installed in a new directory, <install_dir>/lib/clang/22/lib/<target>
, right?
Wouldn't it be better to make flang look for its libraries in the same path as compiler-rt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On my machine, both compiler-rt and flang-rt libraries are installed into
<install_dir>/lib/clang/22/lib/darwin
.
That was the original intention/idea.
IIUC, this change would make flang-rt libraries be installed in a new directory,
<install_dir>/lib/clang/22/lib/<target>
, right? Wouldn't it be better to make flang look for its libraries in the same path as compiler-rt?
ToolChain::addFlangRTLibPath
calls ToolChain::getArchSpecificLibPaths()
which is overridable by the MachO ToolChain (but not actually overridden). The x86_64
is added here:
llvm-project/clang/lib/Driver/ToolChain.cpp
Line 1041 in 852cc92
AddPath({getOSLibName(), llvm::Triple::getArchTypeName(getArch())}); |
It felt best to avoid platform-specific exceptions making the problem more managable.
Turns out ToolChain::getArchSpecificLibPaths()
is only used when using RPath. ToolChain::addFlangRTLibPath
is called to fill getFilePaths
. On non-MachO platforms seem that getRuntimePath()
covers it, but is skipped on MachO1:
llvm-project/clang/lib/Driver/ToolChain.cpp
Lines 1010 to 1011 in 852cc92
if (Triple.isOSDarwin()) | |
return {}; |
Trying another version soon
Footnotes
This patch didn't change the behavior for me, unfortunately. |
This did not resolve the problem because the new location is not in the linker's search path. |
For reference. compiler-rt special Apple code:
flang-rt special Apple code:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me; thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, works on my machine.
Thanks for digging deeper into the cause of the problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works. Thanks.
Testing with the two new commits still fails to locate
Again with
FWIW:
|
@PHHargrove Thanks for the details. There is something weird about your setup.
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Michael Kruse (Meinersbur) Changesclang_rt has two separate systems for find its location (simplified):
To simplify the search path, Flang-RT normally assumes only Fixes #151031 Full diff: https://github.com/llvm/llvm-project/pull/151954.diff 1 Files Affected:
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 25c6b5a486fd5..7667dbddb0ca2 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -855,17 +855,30 @@ void ToolChain::addFortranRuntimeLibs(const ArgList &Args,
void ToolChain::addFortranRuntimeLibraryPath(const llvm::opt::ArgList &Args,
ArgStringList &CmdArgs) const {
- // Default to the <driver-path>/../lib directory. This works fine on the
- // platforms that we have tested so far. We will probably have to re-fine
- // this in the future. In particular, on some platforms, we may need to use
- // lib64 instead of lib.
+ auto AddLibSearchPathIfExists = [&](const Twine &Path) {
+ // Linker may emit warnings about non-existing directories
+ if (!llvm::sys::fs::is_directory(Path))
+ return;
+
+ if (getTriple().isKnownWindowsMSVCEnvironment())
+ CmdArgs.push_back(Args.MakeArgString("-libpath:" + Path));
+ else
+ CmdArgs.push_back(Args.MakeArgString("-L" + Path));
+ };
+
+ // Search for flang_rt.* at the same location as clang_rt.* with
+ // LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=0. On most platforms, flang_rt is
+ // located at the path returned by getRuntimePath() which is already added to
+ // the library search path. This exception is for Apple-Darwin.
+ AddLibSearchPathIfExists(getCompilerRTPath());
+
+ // Fall back to the non-resource directory <driver-path>/../lib. We will
+ // probably have to refine this in the future. In particular, on some
+ // platforms, we may need to use lib64 instead of lib.
SmallString<256> DefaultLibPath =
llvm::sys::path::parent_path(getDriver().Dir);
llvm::sys::path::append(DefaultLibPath, "lib");
- if (getTriple().isKnownWindowsMSVCEnvironment())
- CmdArgs.push_back(Args.MakeArgString("-libpath:" + DefaultLibPath));
- else
- CmdArgs.push_back(Args.MakeArgString("-L" + DefaultLibPath));
+ AddLibSearchPathIfExists(DefaultLibPath);
}
void ToolChain::addFlangRTLibPath(const ArgList &Args,
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/22209 Here is the relevant piece of the build log for the reference
|
Regarding item 2: My I cannot account for the difference, but have determined that Homebrew's Regarding item 1: unexpected library directory. I removed both the build and installation directories, checked out the merge commit which closed this PR (8de4819), reran The flang runtime is installed in TL;DR: I cannot account for either of the "something weird" items, but with a fresh build I no longer reproduce #151031. cmake commoand:
Verbose compile and run:
|
/cherry-pick 8de4819 |
Error: Command failed due to missing milestone. |
/pull-request #152458 |
The clang/flang driver has two separate systems for find the location of clang_rt (simplified): * `getCompilerRTPath()`, e.g. `../lib/clang/22/lib/windows`, used when `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=0` * `getRuntimePath()`, e.g. `../lib/clang/22/lib/x86_64-pc-windows-msvc`, used when `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=1` To simplify the search path, Flang-RT normally assumes only `getRuntimePath()`, i.e. ignoring `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` and always using the `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=1` mechanism. There is an exception for Apple Darwin triples where `getRuntimePath()` returns nothing. The flang-rt/compiler-rt CMake code for library location also ignores `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` but uses the `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=0` path instead. Since only `getRuntimePath()` is automatically added to the linker command line, this patch explicitly adds `getCompilerRTPath()` to the path when linking flang_rt. Fixes llvm#151031 (cherry picked from commit 8de4819)
The clang/flang driver has two separate systems for find the location of clang_rt (simplified):
getCompilerRTPath()
, e.g.../lib/clang/22/lib/windows
, used whenLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=0
getRuntimePath()
, e.g.../lib/clang/22/lib/x86_64-pc-windows-msvc
, used whenLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=1
To simplify the search path, Flang-RT normally assumes only
getRuntimePath()
, i.e. ignoringLLVM_ENABLE_PER_TARGET_RUNTIME_DIR
and always using theLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=1
mechanism. There is an exception for Apple Darwin triples wheregetRuntimePath()
returns nothing. The flang-rt/compiler-rt CMake code for library location also ignoresLLVM_ENABLE_PER_TARGET_RUNTIME_DIR
but uses theLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=0
path instead. Since onlygetRuntimePath()
is automatically added to the linker command line, this patch explicitly addsgetCompilerRTPath()
to the search path when linking flang_rt.Fixes #151031