zero9178 updated this revision to Diff 326022.
zero9178 added a comment.
Undid refactoring of getCompilerRTBasename. Previous diffs introduced a new
internal function that built the basename. In that version
getCompilerRTBasename simply called getCompilerRT and extracted the filename
component.
This lead to issues as Toolchains such as Baremetal override
getCompilerRTBasename to provide a different basename then default.
This diff now instead changes the default implementation of
getCompilerRTBasename to also auto detect whether an architecture suffix should
be used or not. Therefore callers of getCompilerRTBasename can rely on it
picking the right filenames as well. getCompilerRT still calls into
getCompilerRTBasename to get the filename, allowing Toolchains to override the
virtual function.
There is now sadly a bit of repetition as both getCompilerRT and
getCompilerRTBasename have to look through the library path but this should not
cause any issues I believe.
Sorry for the inconvenience and the test failure.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96638/new/
https://reviews.llvm.org/D96638
Files:
clang/lib/Driver/ToolChain.cpp
Index: clang/lib/Driver/ToolChain.cpp
===================================================================
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -22,6 +22,7 @@
#include "clang/Driver/Options.h"
#include "clang/Driver/SanitizerArgs.h"
#include "clang/Driver/XRayArgs.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
@@ -413,12 +414,21 @@
return std::string(Path.str());
}
-static std::string buildCompilerRTBasename(const ToolChain &toolchain,
- const ArgList &Args,
- StringRef Component,
- ToolChain::FileType Type,
- bool AddArch) {
- const llvm::Triple &TT = toolchain.getTriple();
+static Optional<std::string> findInLibraryPath(const ToolChain &TC,
+ StringRef filename) {
+ for (const auto &LibPath : TC.getLibraryPaths()) {
+ SmallString<128> P(LibPath);
+ llvm::sys::path::append(P, filename);
+ if (TC.getVFS().exists(P))
+ return std::string(P.str());
+ }
+ return llvm::None;
+}
+
+std::string ToolChain::getCompilerRTBasename(const ArgList &Args,
+ StringRef Component,
+ FileType Type) const {
+ const llvm::Triple &TT = getTriple();
bool IsITANMSVCWindows =
TT.isWindowsMSVCEnvironment() || TT.isWindowsItaniumEnvironment();
@@ -433,44 +443,35 @@
Suffix = IsITANMSVCWindows ? ".lib" : ".a";
break;
case ToolChain::FT_Shared:
- Suffix = TT.isOSWindows()
- ? (TT.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")
+ Suffix = Triple.isOSWindows()
+ ? (Triple.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")
: ".so";
break;
}
- std::string ArchAndEnv;
- if (AddArch) {
- StringRef Arch = getArchNameForCompilerRTLib(toolchain, Args);
- const char *Env = TT.isAndroid() ? "-android" : "";
- ArchAndEnv = ("-" + Arch + Env).str();
+ std::string CRTWithoutArch =
+ (Prefix + Twine("clang_rt.") + Component + Suffix).str();
+ if (findInLibraryPath(*this, CRTWithoutArch).hasValue()) {
+ return CRTWithoutArch;
}
- return (Prefix + Twine("clang_rt.") + Component + ArchAndEnv + Suffix).str();
-}
-std::string ToolChain::getCompilerRTBasename(const ArgList &Args,
- StringRef Component,
- FileType Type) const {
- std::string absolutePath = getCompilerRT(Args, Component, Type);
- return llvm::sys::path::filename(absolutePath).str();
+ StringRef Arch = getArchNameForCompilerRTLib(*this, Args);
+ const char *Env = TT.isAndroid() ? "-android" : "";
+ std::string ArchAndEnv = ("-" + Arch + Env).str();
+
+ return (Prefix + Twine("clang_rt.") + Component + ArchAndEnv + Suffix).str();
}
std::string ToolChain::getCompilerRT(const ArgList &Args, StringRef Component,
FileType Type) const {
// Check for runtime files in the new layout without the architecture first.
- std::string CRTBasename =
- buildCompilerRTBasename(*this, Args, Component, Type, /*AddArch=*/false);
- for (const auto &LibPath : getLibraryPaths()) {
- SmallString<128> P(LibPath);
- llvm::sys::path::append(P, CRTBasename);
- if (getVFS().exists(P))
- return std::string(P.str());
+ std::string CRTBasename = getCompilerRTBasename(Args, Component, Type);
+ if (auto value = findInLibraryPath(*this, CRTBasename)) {
+ return value.getValue();
}
// Fall back to the old expected compiler-rt name if the new one does not
// exist.
- CRTBasename =
- buildCompilerRTBasename(*this, Args, Component, Type, /*AddArch=*/true);
SmallString<128> Path(getCompilerRTPath());
llvm::sys::path::append(Path, CRTBasename);
return std::string(Path.str());
Index: clang/lib/Driver/ToolChain.cpp
===================================================================
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -22,6 +22,7 @@
#include "clang/Driver/Options.h"
#include "clang/Driver/SanitizerArgs.h"
#include "clang/Driver/XRayArgs.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
@@ -413,12 +414,21 @@
return std::string(Path.str());
}
-static std::string buildCompilerRTBasename(const ToolChain &toolchain,
- const ArgList &Args,
- StringRef Component,
- ToolChain::FileType Type,
- bool AddArch) {
- const llvm::Triple &TT = toolchain.getTriple();
+static Optional<std::string> findInLibraryPath(const ToolChain &TC,
+ StringRef filename) {
+ for (const auto &LibPath : TC.getLibraryPaths()) {
+ SmallString<128> P(LibPath);
+ llvm::sys::path::append(P, filename);
+ if (TC.getVFS().exists(P))
+ return std::string(P.str());
+ }
+ return llvm::None;
+}
+
+std::string ToolChain::getCompilerRTBasename(const ArgList &Args,
+ StringRef Component,
+ FileType Type) const {
+ const llvm::Triple &TT = getTriple();
bool IsITANMSVCWindows =
TT.isWindowsMSVCEnvironment() || TT.isWindowsItaniumEnvironment();
@@ -433,44 +443,35 @@
Suffix = IsITANMSVCWindows ? ".lib" : ".a";
break;
case ToolChain::FT_Shared:
- Suffix = TT.isOSWindows()
- ? (TT.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")
+ Suffix = Triple.isOSWindows()
+ ? (Triple.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")
: ".so";
break;
}
- std::string ArchAndEnv;
- if (AddArch) {
- StringRef Arch = getArchNameForCompilerRTLib(toolchain, Args);
- const char *Env = TT.isAndroid() ? "-android" : "";
- ArchAndEnv = ("-" + Arch + Env).str();
+ std::string CRTWithoutArch =
+ (Prefix + Twine("clang_rt.") + Component + Suffix).str();
+ if (findInLibraryPath(*this, CRTWithoutArch).hasValue()) {
+ return CRTWithoutArch;
}
- return (Prefix + Twine("clang_rt.") + Component + ArchAndEnv + Suffix).str();
-}
-std::string ToolChain::getCompilerRTBasename(const ArgList &Args,
- StringRef Component,
- FileType Type) const {
- std::string absolutePath = getCompilerRT(Args, Component, Type);
- return llvm::sys::path::filename(absolutePath).str();
+ StringRef Arch = getArchNameForCompilerRTLib(*this, Args);
+ const char *Env = TT.isAndroid() ? "-android" : "";
+ std::string ArchAndEnv = ("-" + Arch + Env).str();
+
+ return (Prefix + Twine("clang_rt.") + Component + ArchAndEnv + Suffix).str();
}
std::string ToolChain::getCompilerRT(const ArgList &Args, StringRef Component,
FileType Type) const {
// Check for runtime files in the new layout without the architecture first.
- std::string CRTBasename =
- buildCompilerRTBasename(*this, Args, Component, Type, /*AddArch=*/false);
- for (const auto &LibPath : getLibraryPaths()) {
- SmallString<128> P(LibPath);
- llvm::sys::path::append(P, CRTBasename);
- if (getVFS().exists(P))
- return std::string(P.str());
+ std::string CRTBasename = getCompilerRTBasename(Args, Component, Type);
+ if (auto value = findInLibraryPath(*this, CRTBasename)) {
+ return value.getValue();
}
// Fall back to the old expected compiler-rt name if the new one does not
// exist.
- CRTBasename =
- buildCompilerRTBasename(*this, Args, Component, Type, /*AddArch=*/true);
SmallString<128> Path(getCompilerRTPath());
llvm::sys::path::append(Path, CRTBasename);
return std::string(Path.str());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
