[clang][Driver] Add a custom error option in multilib.yaml. #105684
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sometimes a collection of multilibs has a gap in it, where a set of driver command-line options can't work with any of the available libraries. For example, the Arm MVE extension requires special startup code (you need to initialize FPSCR.LTPSIZE), and also benefits greatly from -mfloat-abi=hard. So a multilib provider might build a library for systems without MVE, and another for MVE with -mfloat-abi=hard, anticipating that that's what most MVE users would want. But then if a user compiles for MVE _without_ -mfloat-abi=hard, thhey can't use either of those libraries – one has an ABI mismatch, and the other will fail to set up LTPSIZE. In that situation, it's useful to include a multilib.yaml entry for the unworkable intermediate situation, and have it map to a fatal error message rather than a set of actual libraries. Then the user gets a build failure with a sensible explanation, instead of selecting an unworkable library and silently generating bad output. The new regression test demonstrates this case. This patch introduces extra syntax into multilib.yaml, so that a record in the `Variants` list can omit the `Dir` key, and in its place, provide a `FatalError` key. Then, if that variant is selected, the error message is emitted as a clang diagnostic, and multilib selection fails. In order to emit the error message in `MultilibSet::select`, I had to pass a `Driver &` to that function, which involved plumbing one through to every call site, and in the unit tests, constructing one specially.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Simon Tatham (statham-arm) ChangesSometimes a collection of multilibs has a gap in it, where a set of driver command-line options can't work with any of the available libraries. For example, the Arm MVE extension requires special startup code (you need to initialize FPSCR.LTPSIZE), and also benefits greatly from -mfloat-abi=hard. So a multilib provider might build a library for systems without MVE, and another for MVE with -mfloat-abi=hard, anticipating that that's what most MVE users would want. But then if a user compiles for MVE without -mfloat-abi=hard, thhey can't use either of those libraries – one has an ABI mismatch, and the other will fail to set up LTPSIZE. In that situation, it's useful to include a multilib.yaml entry for the unworkable intermediate situation, and have it map to a fatal error message rather than a set of actual libraries. Then the user gets a build failure with a sensible explanation, instead of selecting an unworkable library and silently generating bad output. The new regression test demonstrates this case. This patch introduces extra syntax into multilib.yaml, so that a record in the In order to emit the error message in Patch is 36.93 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/105684.diff 13 Files Affected:
diff --git a/clang/docs/Multilib.rst b/clang/docs/Multilib.rst
index 063fe9a336f2fe..6d77fda3623b20 100644
--- a/clang/docs/Multilib.rst
+++ b/clang/docs/Multilib.rst
@@ -200,6 +200,12 @@ For a more comprehensive example see
# to be a match.
Flags: [--target=thumbv7m-none-eabi, -mfpu=fpv4-sp-d16]
+ # If there is no multilib available for a particular set of flags, and the
+ # other multilibs are not adequate fallbacks, then you can define a variant
+ # record with a FatalError key in place of the Dir key.
+ - FatalError: this multilib collection has no hard-float ABI support
+ Flags: [--target=thumbv7m-none-eabi, -mfloat-abi=hard]
+
# The second section of the file is a list of regular expressions that are
# used to map from flags generated from command line options to custom flags.
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index ba90742fbdaabc..97573fcf20c1fb 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -810,6 +810,8 @@ def warn_drv_missing_multilib : Warning<
InGroup<DiagGroup<"missing-multilib">>;
def note_drv_available_multilibs : Note<
"available multilibs are:%0">;
+def err_drv_multilib_custom_error : Error<
+ "multilib configuration error: %0">;
def err_drv_experimental_crel : Error<
"-Wa,--allow-experimental-crel must be specified to use -Wa,--crel. "
diff --git a/clang/include/clang/Driver/Multilib.h b/clang/include/clang/Driver/Multilib.h
index 9a2cc9bb1ba134..2b6a64187f7783 100644
--- a/clang/include/clang/Driver/Multilib.h
+++ b/clang/include/clang/Driver/Multilib.h
@@ -18,6 +18,7 @@
#include "llvm/Support/SourceMgr.h"
#include <cassert>
#include <functional>
+#include <optional>
#include <string>
#include <utility>
#include <vector>
@@ -25,6 +26,8 @@
namespace clang {
namespace driver {
+class Driver;
+
/// This corresponds to a single GCC Multilib, or a segment of one controlled
/// by a command line flag.
/// See also MultilibBuilder for building a multilib by mutating it
@@ -48,13 +51,19 @@ class Multilib {
// directory is not mutually exclusive with anything else.
std::string ExclusiveGroup;
+ // Some Multilib objects don't actually represent library directories you can
+ // select. Instead, they represent failures of multilib selection, of the
+ // form 'Sorry, we don't have any library compatible with these constraints'.
+ std::optional<std::string> FatalError;
+
public:
/// GCCSuffix, OSSuffix & IncludeSuffix will be appended directly to the
/// sysroot string so they must either be empty or begin with a '/' character.
/// This is enforced with an assert in the constructor.
Multilib(StringRef GCCSuffix = {}, StringRef OSSuffix = {},
StringRef IncludeSuffix = {}, const flags_list &Flags = flags_list(),
- StringRef ExclusiveGroup = {});
+ StringRef ExclusiveGroup = {},
+ std::optional<StringRef> FatalError = std::nullopt);
/// Get the detected GCC installation path suffix for the multi-arch
/// target variant. Always starts with a '/', unless empty
@@ -84,6 +93,10 @@ class Multilib {
{ return GCCSuffix.empty() && OSSuffix.empty() && IncludeSuffix.empty(); }
bool operator==(const Multilib &Other) const;
+
+ bool isFatalError() const { return FatalError.has_value(); }
+
+ const std::string &getFatalError() const { return FatalError.value(); }
};
raw_ostream &operator<<(raw_ostream &OS, const Multilib &M);
@@ -130,7 +143,7 @@ class MultilibSet {
/// Select compatible variants, \returns false if none are compatible
bool select(const Multilib::flags_list &Flags,
- llvm::SmallVectorImpl<Multilib> &) const;
+ llvm::SmallVectorImpl<Multilib> &, const Driver &D) const;
unsigned size() const { return Multilibs.size(); }
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index e12416e51f8d24..0ad911972a11ed 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -2296,7 +2296,8 @@ bool Driver::HandleImmediateArgs(Compilation &C) {
if (C.getArgs().hasArg(options::OPT_print_multi_lib)) {
for (const Multilib &Multilib : TC.getMultilibs())
- llvm::outs() << Multilib << "\n";
+ if (!Multilib.isFatalError())
+ llvm::outs() << Multilib << "\n";
return false;
}
diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp
index 9c091bbfdabab1..4114b7faa15270 100644
--- a/clang/lib/Driver/Multilib.cpp
+++ b/clang/lib/Driver/Multilib.cpp
@@ -9,6 +9,7 @@
#include "clang/Driver/Multilib.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/Version.h"
+#include "clang/Driver/Driver.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
@@ -31,9 +32,10 @@ using namespace llvm::sys;
Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix,
StringRef IncludeSuffix, const flags_list &Flags,
- StringRef ExclusiveGroup)
+ StringRef ExclusiveGroup,
+ std::optional<StringRef> FatalError)
: GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix),
- Flags(Flags), ExclusiveGroup(ExclusiveGroup) {
+ Flags(Flags), ExclusiveGroup(ExclusiveGroup), FatalError(FatalError) {
assert(GCCSuffix.empty() ||
(StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1));
assert(OSSuffix.empty() ||
@@ -95,7 +97,8 @@ MultilibSet &MultilibSet::FilterOut(FilterCallback F) {
void MultilibSet::push_back(const Multilib &M) { Multilibs.push_back(M); }
bool MultilibSet::select(const Multilib::flags_list &Flags,
- llvm::SmallVectorImpl<Multilib> &Selected) const {
+ llvm::SmallVectorImpl<Multilib> &Selected,
+ const Driver &D) const {
llvm::StringSet<> FlagSet(expandFlags(Flags));
Selected.clear();
@@ -121,6 +124,14 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
continue;
}
+ // If this multilib is actually a placeholder containing a fatal
+ // error message written by the multilib.yaml author, display that
+ // error message, and return failure.
+ if (M.isFatalError()) {
+ D.Diag(clang::diag::err_drv_multilib_custom_error) << M.getFatalError();
+ return false;
+ }
+
// Select this multilib.
Selected.push_back(M);
}
@@ -162,7 +173,8 @@ namespace {
static const VersionTuple MultilibVersionCurrent(1, 0);
struct MultilibSerialization {
- std::string Dir;
+ std::string Dir; // if this record successfully selects a library dir
+ std::string FatalError; // if this record reports a fatal error message
std::vector<std::string> Flags;
std::string Group;
};
@@ -205,11 +217,16 @@ struct MultilibSetSerialization {
template <> struct llvm::yaml::MappingTraits<MultilibSerialization> {
static void mapping(llvm::yaml::IO &io, MultilibSerialization &V) {
- io.mapRequired("Dir", V.Dir);
+ io.mapOptional("Dir", V.Dir);
+ io.mapOptional("FatalError", V.FatalError);
io.mapRequired("Flags", V.Flags);
io.mapOptional("Group", V.Group);
}
static std::string validate(IO &io, MultilibSerialization &V) {
+ if (V.Dir.empty() && V.FatalError.empty())
+ return "one of the 'Dir' and 'FatalError' keys must be specified";
+ if (!V.Dir.empty() && !V.FatalError.empty())
+ return "the 'Dir' and 'FatalError' keys may not both be specified";
if (StringRef(V.Dir).starts_with("/"))
return "paths must be relative but \"" + V.Dir + "\" starts with \"/\"";
return std::string{};
@@ -295,14 +312,18 @@ MultilibSet::parseYaml(llvm::MemoryBufferRef Input,
multilib_list Multilibs;
Multilibs.reserve(MS.Multilibs.size());
for (const auto &M : MS.Multilibs) {
- std::string Dir;
- if (M.Dir != ".")
- Dir = "/" + M.Dir;
- // We transfer M.Group straight into the ExclusiveGroup parameter for the
- // Multilib constructor. If we later support more than one type of group,
- // we'll have to look up the group name in MS.Groups, check its type, and
- // decide what to do here.
- Multilibs.emplace_back(Dir, Dir, Dir, M.Flags, M.Group);
+ if (!M.FatalError.empty()) {
+ Multilibs.emplace_back("", "", "", M.Flags, M.Group, M.FatalError);
+ } else {
+ std::string Dir;
+ if (M.Dir != ".")
+ Dir = "/" + M.Dir;
+ // We transfer M.Group straight into the ExclusiveGroup parameter for the
+ // Multilib constructor. If we later support more than one type of group,
+ // we'll have to look up the group name in MS.Groups, check its type, and
+ // decide what to do here.
+ Multilibs.emplace_back(Dir, Dir, Dir, M.Flags, M.Group);
+ }
}
return MultilibSet(std::move(Multilibs), std::move(MS.FlagMatchers));
diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index 0f5f32f3f8f495..e26cdae6bf0391 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -58,7 +58,7 @@ static bool findRISCVMultilibs(const Driver &D,
Result.Multilibs =
MultilibSetBuilder().Either(Imac, Imafdc).makeMultilibSet();
- return Result.Multilibs.select(Flags, Result.SelectedMultilibs);
+ return Result.Multilibs.select(Flags, Result.SelectedMultilibs, D);
}
if (TargetTriple.isRISCV32()) {
MultilibBuilder Imac =
@@ -92,7 +92,7 @@ static bool findRISCVMultilibs(const Driver &D,
Result.Multilibs =
MultilibSetBuilder().Either(I, Im, Iac, Imac, Imafc).makeMultilibSet();
- return Result.Multilibs.select(Flags, Result.SelectedMultilibs);
+ return Result.Multilibs.select(Flags, Result.SelectedMultilibs, D);
}
return false;
}
@@ -182,12 +182,13 @@ static void findMultilibsFromYAML(const ToolChain &TC, const Driver &D,
if (ErrorOrMultilibSet.getError())
return;
Result.Multilibs = ErrorOrMultilibSet.get();
- if (Result.Multilibs.select(Flags, Result.SelectedMultilibs))
+ if (Result.Multilibs.select(Flags, Result.SelectedMultilibs, D))
return;
D.Diag(clang::diag::warn_drv_missing_multilib) << llvm::join(Flags, " ");
std::stringstream ss;
for (const Multilib &Multilib : Result.Multilibs)
- ss << "\n" << llvm::join(Multilib.flags(), " ");
+ if (!Multilib.isFatalError())
+ ss << "\n" << llvm::join(Multilib.flags(), " ");
D.Diag(clang::diag::note_drv_available_multilibs) << ss.str();
}
diff --git a/clang/lib/Driver/ToolChains/Fuchsia.cpp b/clang/lib/Driver/ToolChains/Fuchsia.cpp
index 6daa73c7a54c88..d4145685c19b5f 100644
--- a/clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ b/clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -324,7 +324,7 @@ Fuchsia::Fuchsia(const Driver &D, const llvm::Triple &Triple,
Multilibs.setFilePathsCallback(FilePaths);
- if (Multilibs.select(Flags, SelectedMultilibs)) {
+ if (Multilibs.select(Flags, SelectedMultilibs, D)) {
// Ensure that -print-multi-directory only outputs one multilib directory.
Multilib LastSelected = SelectedMultilibs.back();
SelectedMultilibs = {LastSelected};
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index 52c2ee90b1b286..f65ba639fa5239 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -1041,7 +1041,8 @@ static bool isMSP430(llvm::Triple::ArchType Arch) {
return Arch == llvm::Triple::msp430;
}
-static bool findMipsCsMultilibs(const Multilib::flags_list &Flags,
+static bool findMipsCsMultilibs(const Driver &D,
+ const Multilib::flags_list &Flags,
FilterNonExistent &NonExistent,
DetectedMultilibs &Result) {
// Check for Code Sourcery toolchain multilibs
@@ -1135,7 +1136,7 @@ static bool findMipsCsMultilibs(const Multilib::flags_list &Flags,
if (CSMipsMultilibs.size() < DebianMipsMultilibs.size())
std::iter_swap(Candidates, Candidates + 1);
for (const MultilibSet *Candidate : Candidates) {
- if (Candidate->select(Flags, Result.SelectedMultilibs)) {
+ if (Candidate->select(Flags, Result.SelectedMultilibs, D)) {
if (Candidate == &DebianMipsMultilibs)
Result.BiarchSibling = Multilib();
Result.Multilibs = *Candidate;
@@ -1145,7 +1146,8 @@ static bool findMipsCsMultilibs(const Multilib::flags_list &Flags,
return false;
}
-static bool findMipsAndroidMultilibs(llvm::vfs::FileSystem &VFS, StringRef Path,
+static bool findMipsAndroidMultilibs(const Driver &D,
+ llvm::vfs::FileSystem &VFS, StringRef Path,
const Multilib::flags_list &Flags,
FilterNonExistent &NonExistent,
DetectedMultilibs &Result) {
@@ -1184,14 +1186,15 @@ static bool findMipsAndroidMultilibs(llvm::vfs::FileSystem &VFS, StringRef Path,
MS = &AndroidMipselMultilibs;
else if (VFS.exists(Path + "/32"))
MS = &AndroidMips64elMultilibs;
- if (MS->select(Flags, Result.SelectedMultilibs)) {
+ if (MS->select(Flags, Result.SelectedMultilibs, D)) {
Result.Multilibs = *MS;
return true;
}
return false;
}
-static bool findMipsMuslMultilibs(const Multilib::flags_list &Flags,
+static bool findMipsMuslMultilibs(const Driver &D,
+ const Multilib::flags_list &Flags,
FilterNonExistent &NonExistent,
DetectedMultilibs &Result) {
// Musl toolchain multilibs
@@ -1218,14 +1221,15 @@ static bool findMipsMuslMultilibs(const Multilib::flags_list &Flags,
{"/../sysroot" + M.osSuffix() + "/usr/include"});
});
}
- if (MuslMipsMultilibs.select(Flags, Result.SelectedMultilibs)) {
+ if (MuslMipsMultilibs.select(Flags, Result.SelectedMultilibs, D)) {
Result.Multilibs = MuslMipsMultilibs;
return true;
}
return false;
}
-static bool findMipsMtiMultilibs(const Multilib::flags_list &Flags,
+static bool findMipsMtiMultilibs(const Driver &D,
+ const Multilib::flags_list &Flags,
FilterNonExistent &NonExistent,
DetectedMultilibs &Result) {
// CodeScape MTI toolchain v1.2 and early.
@@ -1403,7 +1407,7 @@ static bool findMipsMtiMultilibs(const Multilib::flags_list &Flags,
});
}
for (auto *Candidate : {&MtiMipsMultilibsV1, &MtiMipsMultilibsV2}) {
- if (Candidate->select(Flags, Result.SelectedMultilibs)) {
+ if (Candidate->select(Flags, Result.SelectedMultilibs, D)) {
Result.Multilibs = *Candidate;
return true;
}
@@ -1411,7 +1415,8 @@ static bool findMipsMtiMultilibs(const Multilib::flags_list &Flags,
return false;
}
-static bool findMipsImgMultilibs(const Multilib::flags_list &Flags,
+static bool findMipsImgMultilibs(const Driver &D,
+ const Multilib::flags_list &Flags,
FilterNonExistent &NonExistent,
DetectedMultilibs &Result) {
// CodeScape IMG toolchain v1.2 and early.
@@ -1509,7 +1514,7 @@ static bool findMipsImgMultilibs(const Multilib::flags_list &Flags,
});
}
for (auto *Candidate : {&ImgMultilibsV1, &ImgMultilibsV2}) {
- if (Candidate->select(Flags, Result.SelectedMultilibs)) {
+ if (Candidate->select(Flags, Result.SelectedMultilibs, D)) {
Result.Multilibs = *Candidate;
return true;
}
@@ -1556,25 +1561,25 @@ bool clang::driver::findMIPSMultilibs(const Driver &D,
addMultilibFlag(!isMipsEL(TargetArch), "-EB", Flags);
if (TargetTriple.isAndroid())
- return findMipsAndroidMultilibs(D.getVFS(), Path, Flags, NonExistent,
+ return findMipsAndroidMultilibs(D, D.getVFS(), Path, Flags, NonExistent,
Result);
if (TargetTriple.getVendor() == llvm::Triple::MipsTechnologies &&
TargetTriple.getOS() == llvm::Triple::Linux &&
TargetTriple.getEnvironment() == llvm::Triple::UnknownEnvironment)
- return findMipsMuslMultilibs(Flags, NonExistent, Result);
+ return findMipsMuslMultilibs(D, Flags, NonExistent, Result);
if (TargetTriple.getVendor() == llvm::Triple::MipsTechnologies &&
TargetTriple.getOS() == llvm::Triple::Linux &&
TargetTriple.isGNUEnvironment())
- return findMipsMtiMultilibs(Flags, NonExistent, Result);
+ return findMipsMtiMultilibs(D, Flags, NonExistent, Result);
if (TargetTriple.getVendor() == llvm::Triple::ImaginationTechnologies &&
TargetTriple.getOS() == llvm::Triple::Linux &&
TargetTriple.isGNUEnvironment())
- return findMipsImgMultilibs(Flags, NonExistent, Result);
+ return findMipsImgMultilibs(D, Flags, NonExistent, Result);
- if (findMipsCsMultilibs(Flags, NonExistent, Result))
+ if (findMipsCsMultilibs(D, Flags, NonExistent, Result))
return true;
// Fallback to the regular toolchain-tree structure.
@@ -1582,7 +1587,7 @@ bool clang::driver::findMIPSMultilibs(const Driver &D,
Result.Multilibs.push_back(Default);
Result.Multilibs.FilterOut(NonExistent);
- if (Result.Multilibs.select(Flags, Result.SelectedMultilibs)) {
+ if (Result.Multilibs.select(Flags, Result.SelectedMultilibs, D)) {
Result.BiarchSibling = Multilib();
return true;
}
@@ -1629,7 +1634,7 @@ static void findAndroidArmMultilibs(const Driver &D,
addMultilibFlag(IsArmV7Mode, "-march=armv7-a", Flags);
addMultilibFlag(IsThumbMode, "-mthumb", Flags);
- if (AndroidArmMultilibs.select(Flags, Result.SelectedMultilibs))
+ if (AndroidArmMultilibs.select(Flags, Result.SelectedMultilibs, D))
Result.Multilibs = AndroidArmMultilibs;
}
@@ -1655,7 +1660,7 @@ static bool findMSP430Multilibs(const Driver &D,
addMultilibFlag(Args.hasFlag(options::OPT_fexceptions,
options::OPT_fno_exceptions, false),
"-exceptions", Flags);
- if (Result.Multilibs.select(Flags, Result.SelectedMultilibs))
+ if (Result.Multilibs.select(Flags, Result.SelectedMultilibs, D))
return true;
return false;
@@ -1722,7 +1727,7 @@ static void findCSKYMultilibs(const Driver &D, const llvm::Triple &TargetTriple,
.makeMultilibSet()
.FilterOut(NonExistent);
- if (CSKYMultilibs.select(Flags, Result.SelectedMultilibs))
+ if (CSKYMultilibs.select(Flags, Result.SelectedMultilibs, D))
Result.Multilibs = CSKYMultilibs;
}
@@ -1737,11 +1742,11 @@ static void findCSKYMultilibs(const Driver &D, const llvm::Triple &TargetTriple,
/// march=rv32ima are not compatible, because software and hardware
/// atomic operation can't work together correctly.
static bool
-selectRISCVMultilib(const MultilibSet &RISCVMultilibSet, StringRef Arch,
- const Multilib::flags_list &Flags,
+selectRISCVMultilib(const Driver &D, const MultilibSet &RISCVMultilibSet,
+ StringRef Arch, const Multilib::flags_list &Flags,
llvm::SmallVectorImpl<Multilib> &SelectedMultilibs) {
// Try to find the perfect matching multi-lib first.
- if (RISCVMultilibSet.select(Flags, SelectedMultilibs))
+ if (RISCVMultilibSet.select(Flags, SelectedMultilibs, D))
return true;
Multilib::flags_list NewFlags;
@@ -1833,7 +1838,7 @@ selectRISCVMultilib(const MultilibSet &RISCVMultilibSet, StringRef Arch,
MultilibSet NewRISCVMultilibs =
MultilibSetBuilder().Either(NewMultilibs).makeMultilibSet();
- if (NewRISCVMultilibs.select(NewFlags, SelectedMultilibs))
+ if (NewRISCVMultilibs.select(NewFlags, SelectedMultilibs, D))
for (const Multilib &NewSelectedM : SelectedMultilibs)
for (const auto &M : RISCVMultilibSet)
// Look up the corresponding multi-lib entry in original multi-lib set.
@@ -1894,7 +1899,7 @@ static void findRISCVBareMetalMultilibs(const Driver &D,
}
}
- if (selectRISCVMultilib(RISCVMultilibs, MArch, Flags,
+ if (selectRISCVMultilib(D, RISCVMultilibs, MArch, Flags,
Result.SelectedMultilibs))
Result.Multilibs = RISCVMultilibs;
}
@@ -1937,7 +1942,7 @@ static void findRISCVMultilibs(const Driver &D,
add...
[truncated]
|
MaskRay
reviewed
Aug 23, 2024
| @@ -130,7 +143,7 @@ class MultilibSet { | |||
|
|
|||
| /// Select compatible variants, \returns false if none are compatible | |||
| bool select(const Multilib::flags_list &Flags, | |||
There was a problem hiding this comment.
For other parts of Driver, I think const Driver & is more commonly used as the first parameter
| # REQUIRES: shell | ||
| # UNSUPPORTED: system-windows | ||
|
|
||
| # RUN: rm -rf %T/baremetal_multilib |
There was a problem hiding this comment.
do not use %T
https://llvm.org/docs/CommandGuide/lit.html#substitutions
just use rm -rf %t && mkdir %t for what you do with %T/baremetal_multilib ?
Review feedback wants the `const Driver &` to come first.
I'd copied the framework of this test from one of the existing ones, and was caught out by the existing test using a deprecated lit feature.
|
There are two failed CI checks here, but they both seem to be complaining about a libc++ test called |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/4754 Here is the relevant piece of the build log for the reference |
|
For the record, that also looks spurious: again it's completely unrelated to this patch, and also, build 4752 of the same buildbot failed in the same way, two commits earlier. It looks more likely that that test is just flaky. |
|
This is just a nit, but is there a reason for calling this |
|
I'd intended it to clarify that it will cause your link to fail, as opposed to some more recoverable type of error of the form "falling back to this less-desirable library but your link will still at least work". I haven't actually implemented such a thing, but it made sense to me to leave space for it in the (implied) YAML schema. I suppose you could call those "error" and "warning" respectively, although to me "warning" in a compiler context has a connotation of warning the user about their own potential error, whereas here, the fact that some particular toolchain provider hasn't built exactly the library the user would most like doesn't mean the user was wrong to want it. |
|
I think |
|
To be clear, are you asking me to make a followup PR to change that identifier in this already-landed patch, or are you going to do it? (Just to avoid the situation where both of us do it, or both of us think the other is going to) |
If you have time to implement it, that would be great. I might be able to take a look, but it's going to be a few weeks before I have time, and if we're going to rename |
Closed
statham-arm
added a commit
to statham-arm/llvm-project
that referenced
this pull request
Oct 2, 2024
This is a late-breaking change to llvm#105684, suggested after the original patch was already landed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sometimes a collection of multilibs has a gap in it, where a set of driver command-line options can't work with any of the available libraries.
For example, the Arm MVE extension requires special startup code (you need to initialize FPSCR.LTPSIZE), and also benefits greatly from -mfloat-abi=hard. So a multilib provider might build a library for systems without MVE, and another for MVE with -mfloat-abi=hard, anticipating that that's what most MVE users would want. But then if a user compiles for MVE without -mfloat-abi=hard, thhey can't use either of those libraries – one has an ABI mismatch, and the other will fail to set up LTPSIZE.
In that situation, it's useful to include a multilib.yaml entry for the unworkable intermediate situation, and have it map to a fatal error message rather than a set of actual libraries. Then the user gets a build failure with a sensible explanation, instead of selecting an unworkable library and silently generating bad output. The new regression test demonstrates this case.
This patch introduces extra syntax into multilib.yaml, so that a record in the
Variantslist can omit theDirkey, and in its place, provide aFatalErrorkey. Then, if that variant is selected, the error message is emitted as a clang diagnostic, and multilib selection fails.In order to emit the error message in
MultilibSet::select, I had to pass aDriver &to that function, which involved plumbing one through to every call site, and in the unit tests, constructing one specially.