[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
https://github.com/statham-arm closed https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
statham-arm wrote: (This final force-push is the squashed version of the previous stack, rebased to the current head of `main`, so that the builder can run a last test. Thanks both for the approvals; I'll merge it once the tests have finished.) https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
https://github.com/statham-arm updated
https://github.com/llvm/llvm-project/pull/69447
>From 1140903195e555643ee1a6b9f671b47b0c307f9e Mon Sep 17 00:00:00 2001
From: Simon Tatham
Date: Thu, 14 Sep 2023 14:51:17 +0100
Subject: [PATCH] [Driver] Add ExclusiveGroup feature to multilib.yaml.
This allows a YAML-based multilib configuration to specify explicitly
that a subset of its library directories are alternatives to each
other, i.e. at most one of that subset should be selected.
So if you have multiple sysroots each including a full set of headers
and libraries, you can mark them as members of the same mutually
exclusive group, and then you'll be sure that only one of them is
selected, even if two or more are compatible with the compile options.
This is particularly important in multilib setups including the libc++
headers, where selecting the include directories from two different
sysroots can cause an actual build failure. This occurs when including
, for example: libc++'s stdio.h is included first, and will
try to use `#include_next` to fetch the underlying libc's version. But
if there are two include directories from separate multilibs, then
both of their C++ include directories will end up on the include path
first, followed by both the C directories. So the `#include_next` from
the first libc++ stdio.h will include the second libc++ stdio.h, which
will do nothing because it has the same include guard macro, and the
libc header won't ever be included at all.
If more than one of the options in an exclusive group matches the
given flags, the last one wins.
The syntax for specifying this in multilib.yaml is to define a Groups
section in which you specify your group names, and for each one,
declare it to have Type: Exclusive. (This reserves space in the syntax
for maybe adding other group types later, such as a group of mutually
_dependent_ things that you must have all or none of.) Then each
Variant record that's a member of a group has a Group: property giving
that group's name.
---
clang/include/clang/Driver/Multilib.h | 16 ++-
clang/lib/Driver/Multilib.cpp | 108 --
.../baremetal-multilib-exclusive-group.yaml | 79 +
.../baremetal-multilib-group-error.yaml | 27 +
4 files changed, 218 insertions(+), 12 deletions(-)
create mode 100644 clang/test/Driver/baremetal-multilib-exclusive-group.yaml
create mode 100644 clang/test/Driver/baremetal-multilib-group-error.yaml
diff --git a/clang/include/clang/Driver/Multilib.h
b/clang/include/clang/Driver/Multilib.h
index 1416559414f894b..6a9533e6dd831f1 100644
--- a/clang/include/clang/Driver/Multilib.h
+++ b/clang/include/clang/Driver/Multilib.h
@@ -39,13 +39,22 @@ class Multilib {
std::string IncludeSuffix;
flags_list Flags;
+ // Optionally, a multilib can be assigned a string tag indicating that it's
+ // part of a group of mutually exclusive possibilities. If two or more
+ // multilibs have the same non-empty value of ExclusiveGroup, then only the
+ // last matching one of them will be selected.
+ //
+ // Setting this to the empty string is a special case, indicating that the
+ // directory is not mutually exclusive with anything else.
+ std::string ExclusiveGroup;
+
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 IncludeSuffix = {}, const flags_list &Flags =
flags_list(),
+ StringRef ExclusiveGroup = {});
/// Get the detected GCC installation path suffix for the multi-arch
/// target variant. Always starts with a '/', unless empty
@@ -63,6 +72,9 @@ class Multilib {
/// All elements begin with either '-' or '!'
const flags_list &flags() const { return Flags; }
+ /// Get the exclusive group label.
+ const std::string &exclusiveGroup() const { return ExclusiveGroup; }
+
LLVM_DUMP_METHOD void dump() const;
/// print summary of the Multilib
void print(raw_ostream &OS) const;
diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp
index 48a494d9fa38db5..7681c1a3ce6756f 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 "llvm/ADT/DenseSet.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Compiler.h"
@@ -29,9 +30,10 @@ using namespace driver;
using namespace llvm::sys;
Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix,
- StringRef IncludeSuffix, const flags_list &Flags)
+ StringRef IncludeSuffix, const flags_list &Flags,
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
@@ -138,10 +164,34 @@ static const VersionTuple MultilibVersionCurrent(1, 0);
struct MultilibSerialization {
std::string Dir;
std::vector Flags;
+ std::string Group;
+};
+
+struct MultilibGroupSerialization {
+ /*
+ * Future directions:
+ *
+ * If it's needed in future, we could introduce additional group types by
+ * permitting Type to contain strings other than "Exclusive". Another
+ * possibility is a group of library directories that are mutually
+ * _dependent_ rather than mutually exclusive: if you include one you must
+ * include them all.
+ *
+ * It might also be useful to allow groups to be members of other groups, so
+ * that a mutually exclusive group could contain a mutually dependent set of
+ * library directories, or vice versa.
+ *
+ * These additional features would need changes in the implementation, but
+ * the YAML schema is set up so they can be added without requiring changes
+ * in existing users' multilib.yaml files.
+ */
+ std::string Name;
+ std::string Type;
statham-arm wrote:
Yes, apparently we can. I hadn't found that part of the `llvm::yaml` API yet,
but defining a `ScalarEnumerationTraits` for the enum type seems to be the way
to make it Just Work during decoding. Thanks.
https://github.com/llvm/llvm-project/pull/69447
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
https://github.com/petrhosek approved this pull request. https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
@@ -138,10 +164,34 @@ static const VersionTuple MultilibVersionCurrent(1, 0);
struct MultilibSerialization {
std::string Dir;
std::vector Flags;
+ std::string Group;
+};
+
+struct MultilibGroupSerialization {
+ /*
+ * Future directions:
+ *
+ * If it's needed in future, we could introduce additional group types by
+ * permitting Type to contain strings other than "Exclusive". Another
+ * possibility is a group of library directories that are mutually
+ * _dependent_ rather than mutually exclusive: if you include one you must
+ * include them all.
+ *
+ * It might also be useful to allow groups to be members of other groups, so
+ * that a mutually exclusive group could contain a mutually dependent set of
+ * library directories, or vice versa.
+ *
+ * These additional features would need changes in the implementation, but
+ * the YAML schema is set up so they can be added without requiring changes
+ * in existing users' multilib.yaml files.
+ */
+ std::string Name;
+ std::string Type;
petrhosek wrote:
Could we make the type an `enum`?
https://github.com/llvm/llvm-project/pull/69447
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
MaskRay wrote: > (btw, that `squash!` commit contains the revised commit message I plan to put > on the final version, so I need to not forget to do the squash by hand to get > that right) You may edit the first comment in this PR now, and it will be propagated to the textbox when you click "Squash and merge" :) https://discourse.llvm.org/t/force-push-and-rebase/73766 I believe the primary value appending new commits instead of rebasing is to demonstrate that the PR has been updated in the "Commits" tab. An amend without rebasing is equally good, just requiring `compare` links inserted by GitHub. https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
statham-arm wrote: (btw, that `squash!` commit contains the revised commit message I plan to put on the final version, so I need to not forget to do the squash by hand to get that right) https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
statham-arm wrote: OK, here's a version with the syntax that way. I've added another test to demonstrate the new error checks. The implementation of exclusion is still done by having an `ExclusiveGroup` field in the actual `Multilib` class. Implementing mutually-dependent groups or nested groups is enough extra effort that I'd rather leave it until we actually need it! But now the user-facing syntax in `multilib.yaml` is futureproof against wanting to add those features later, so _only_ the implementation should need to change. https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
https://github.com/statham-arm updated
https://github.com/llvm/llvm-project/pull/69447
>From 2a65ae75e8c8e62e7275a439849837919599e896 Mon Sep 17 00:00:00 2001
From: Simon Tatham
Date: Thu, 14 Sep 2023 14:51:17 +0100
Subject: [PATCH 1/4] [Driver] Add ExclusiveGroup feature to multilib.yaml.
This allows a YAML-based multilib configuration to specify explicitly
that a subset of its library directories are alternatives to each
other, i.e. at most one of that subset should be selected.
So if you have multiple sysroots each including a full set of headers
and libraries, you can mark them as members of the same
ExclusiveGroup, and then you'll be sure that only one of them is
selected, even if two or more are compatible with the compile options.
This is particularly important in multilib setups including the libc++
headers, where selecting the include directories from two different
sysroots can cause an actual build failure. This occurs when including
, for example: libc++'s stdio.h is included first, and will
try to use `#include_next` to fetch the underlying libc's version. But
if there are two include directories from separate multilibs, then
both of their C++ include directories will end up on the include path
first, followed by both the C directories. So the `#include_next` from
the first libc++ stdio.h will include the second libc++ stdio.h, which
will do nothing because it has the same include guard macro, and the
libc header won't ever be included at all.
If more than one of the options in an ExclusiveGroup matches the given
flags, the last one wins.
---
clang/include/clang/Driver/Multilib.h | 16 -
clang/lib/Driver/Multilib.cpp | 49 ++---
.../baremetal-multilib-exclusive-group.yaml | 69 +++
3 files changed, 122 insertions(+), 12 deletions(-)
create mode 100644 clang/test/Driver/baremetal-multilib-exclusive-group.yaml
diff --git a/clang/include/clang/Driver/Multilib.h
b/clang/include/clang/Driver/Multilib.h
index 1416559414f894b..6a9533e6dd831f1 100644
--- a/clang/include/clang/Driver/Multilib.h
+++ b/clang/include/clang/Driver/Multilib.h
@@ -39,13 +39,22 @@ class Multilib {
std::string IncludeSuffix;
flags_list Flags;
+ // Optionally, a multilib can be assigned a string tag indicating that it's
+ // part of a group of mutually exclusive possibilities. If two or more
+ // multilibs have the same non-empty value of ExclusiveGroup, then only the
+ // last matching one of them will be selected.
+ //
+ // Setting this to the empty string is a special case, indicating that the
+ // directory is not mutually exclusive with anything else.
+ std::string ExclusiveGroup;
+
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 IncludeSuffix = {}, const flags_list &Flags =
flags_list(),
+ StringRef ExclusiveGroup = {});
/// Get the detected GCC installation path suffix for the multi-arch
/// target variant. Always starts with a '/', unless empty
@@ -63,6 +72,9 @@ class Multilib {
/// All elements begin with either '-' or '!'
const flags_list &flags() const { return Flags; }
+ /// Get the exclusive group label.
+ const std::string &exclusiveGroup() const { return ExclusiveGroup; }
+
LLVM_DUMP_METHOD void dump() const;
/// print summary of the Multilib
void print(raw_ostream &OS) const;
diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp
index 48a494d9fa38db5..085ccee7b25752e 100644
--- a/clang/lib/Driver/Multilib.cpp
+++ b/clang/lib/Driver/Multilib.cpp
@@ -29,9 +29,10 @@ using namespace driver;
using namespace llvm::sys;
Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix,
- StringRef IncludeSuffix, const flags_list &Flags)
+ StringRef IncludeSuffix, const flags_list &Flags,
+ StringRef ExclusiveGroup)
: GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix),
- Flags(Flags) {
+ Flags(Flags), ExclusiveGroup(ExclusiveGroup) {
assert(GCCSuffix.empty() ||
(StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1));
assert(OSSuffix.empty() ||
@@ -96,13 +97,39 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
llvm::SmallVector &Selected) const {
llvm::StringSet<> FlagSet(expandFlags(Flags));
Selected.clear();
- llvm::copy_if(Multilibs, std::back_inserter(Selected),
-[&FlagSet](const Multilib &M) {
- for (const std::string &F : M.flags())
-if (!FlagSet.contains(F))
- return false;
- return
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
petrhosek wrote: > Would you accept `Type: Exclusive` instead of `Exclusive: True`? It seems > more plausible to me that there might be three kinds of group that _can't_ go > together than three group-type flags that you can have in any combination. I like this as it scales better to other types of groups we might have in the future. https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
statham-arm wrote: > my only concern is to make sure we don't unintentionally make it harder to > integrate potential future extensions such as the mutually dependent groups. Hmmm. So if you had both ME and MD groups, you might also need a _group_ to be able to be a member of another group? That way you could specify hierarchies such as "must have all of: A, B, and exactly one of C,D" (a MD group one of whose members is a ME group), or "must have at most one of: (all of A,B,C) or (all of U,V,W)" (a ME group containing MD groups). I suppose that makes sense, and the only change it needs to your structure is that maybe later a group record might also need to have a `Group:` or `Parent:` header. But there's no need to put that part in now, only to make sure there's room to add it in future if needed. Would you accept `Type: Exclusive` instead of `Exclusive: True`? It seems more plausible to me that there might be three kinds of group that _can't_ go together than three group-type flags that you can have in any combination. > although that may not necessarily be a bad thing since you could also warn if > someone accidentally tries to use a group that wasn't previously defined > (e.g. when making a typo). That is true. https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
petrhosek wrote: I'm fine with the feature, my only concern is to make sure we don't unintentionally make it harder to integrate potential future extensions such as the mutually dependent groups. The only alternative I could come up with is representing groups as first class concept with properties, such as being mutually exclusive. To give a concrete example: ``` Groups: - Name: actually_exclude_something Exclusive: True Variants: - Dir: testdir1_non_exclusive Flags: [--target=thumbv7m-none-unknown-eabi] - Dir: testdir2_non_exclusive Flags: [--target=thumbv7em-none-unknown-eabi] - Dir: testdir1_exclusive Flags: [--target=thumbv7m-none-unknown-eabi] Group: actually_exclude_something - Dir: testdir2_exclusive Flags: [--target=thumbv7em-none-unknown-eabi] Group: actually_exclude_something ``` This makes it possible to extend the group concept in the future but it's a little more verbose since you need to define the group first, although that may not necessarily be a bad thing since you could also warn if someone accidentally tries to use a group that wasn't previously defined (e.g. when making a typo). https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
statham-arm wrote: @petrhosek, do you have any further comments? I'll merge this change based on @MaskRay's approval if I haven't heard back in another week. https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
https://github.com/statham-arm edited https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
@@ -96,13 +97,39 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
llvm::SmallVector &Selected) const {
llvm::StringSet<> FlagSet(expandFlags(Flags));
Selected.clear();
- llvm::copy_if(Multilibs, std::back_inserter(Selected),
-[&FlagSet](const Multilib &M) {
- for (const std::string &F : M.flags())
-if (!FlagSet.contains(F))
- return false;
- return true;
-});
+
+ // Decide which multilibs we're going to select at all
+ std::vector IsSelected(Multilibs.size(), false);
+ std::map ExclusiveGroupMembers;
+ for (size_t i = 0, e = Multilibs.size(); i < e; ++i) {
+const Multilib &M = Multilibs[i];
+
+// If this multilib doesn't match all our flags, don't select it
+if (!llvm::all_of(M.flags(), [&FlagSet](const std::string &F) {
+ return FlagSet.contains(F);
+}))
+ continue;
+
+// If this multilib has the same ExclusiveGroup as one we've already
+// selected, de-select the previous one
+const std::string &group = M.exclusiveGroup();
+if (!group.empty()) {
statham-arm wrote:
`insert`, actually – `try_emplace` appears in `DenseMap` but not `DenseSet`.
But otherwise, done.
https://github.com/llvm/llvm-project/pull/69447
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
https://github.com/statham-arm updated
https://github.com/llvm/llvm-project/pull/69447
>From 2a65ae75e8c8e62e7275a439849837919599e896 Mon Sep 17 00:00:00 2001
From: Simon Tatham
Date: Thu, 14 Sep 2023 14:51:17 +0100
Subject: [PATCH 1/3] [Driver] Add ExclusiveGroup feature to multilib.yaml.
This allows a YAML-based multilib configuration to specify explicitly
that a subset of its library directories are alternatives to each
other, i.e. at most one of that subset should be selected.
So if you have multiple sysroots each including a full set of headers
and libraries, you can mark them as members of the same
ExclusiveGroup, and then you'll be sure that only one of them is
selected, even if two or more are compatible with the compile options.
This is particularly important in multilib setups including the libc++
headers, where selecting the include directories from two different
sysroots can cause an actual build failure. This occurs when including
, for example: libc++'s stdio.h is included first, and will
try to use `#include_next` to fetch the underlying libc's version. But
if there are two include directories from separate multilibs, then
both of their C++ include directories will end up on the include path
first, followed by both the C directories. So the `#include_next` from
the first libc++ stdio.h will include the second libc++ stdio.h, which
will do nothing because it has the same include guard macro, and the
libc header won't ever be included at all.
If more than one of the options in an ExclusiveGroup matches the given
flags, the last one wins.
---
clang/include/clang/Driver/Multilib.h | 16 -
clang/lib/Driver/Multilib.cpp | 49 ++---
.../baremetal-multilib-exclusive-group.yaml | 69 +++
3 files changed, 122 insertions(+), 12 deletions(-)
create mode 100644 clang/test/Driver/baremetal-multilib-exclusive-group.yaml
diff --git a/clang/include/clang/Driver/Multilib.h
b/clang/include/clang/Driver/Multilib.h
index 1416559414f894b..6a9533e6dd831f1 100644
--- a/clang/include/clang/Driver/Multilib.h
+++ b/clang/include/clang/Driver/Multilib.h
@@ -39,13 +39,22 @@ class Multilib {
std::string IncludeSuffix;
flags_list Flags;
+ // Optionally, a multilib can be assigned a string tag indicating that it's
+ // part of a group of mutually exclusive possibilities. If two or more
+ // multilibs have the same non-empty value of ExclusiveGroup, then only the
+ // last matching one of them will be selected.
+ //
+ // Setting this to the empty string is a special case, indicating that the
+ // directory is not mutually exclusive with anything else.
+ std::string ExclusiveGroup;
+
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 IncludeSuffix = {}, const flags_list &Flags =
flags_list(),
+ StringRef ExclusiveGroup = {});
/// Get the detected GCC installation path suffix for the multi-arch
/// target variant. Always starts with a '/', unless empty
@@ -63,6 +72,9 @@ class Multilib {
/// All elements begin with either '-' or '!'
const flags_list &flags() const { return Flags; }
+ /// Get the exclusive group label.
+ const std::string &exclusiveGroup() const { return ExclusiveGroup; }
+
LLVM_DUMP_METHOD void dump() const;
/// print summary of the Multilib
void print(raw_ostream &OS) const;
diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp
index 48a494d9fa38db5..085ccee7b25752e 100644
--- a/clang/lib/Driver/Multilib.cpp
+++ b/clang/lib/Driver/Multilib.cpp
@@ -29,9 +29,10 @@ using namespace driver;
using namespace llvm::sys;
Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix,
- StringRef IncludeSuffix, const flags_list &Flags)
+ StringRef IncludeSuffix, const flags_list &Flags,
+ StringRef ExclusiveGroup)
: GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix),
- Flags(Flags) {
+ Flags(Flags), ExclusiveGroup(ExclusiveGroup) {
assert(GCCSuffix.empty() ||
(StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1));
assert(OSSuffix.empty() ||
@@ -96,13 +97,39 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
llvm::SmallVector &Selected) const {
llvm::StringSet<> FlagSet(expandFlags(Flags));
Selected.clear();
- llvm::copy_if(Multilibs, std::back_inserter(Selected),
-[&FlagSet](const Multilib &M) {
- for (const std::string &F : M.flags())
-if (!FlagSet.contains(F))
- return false;
- return
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
https://github.com/MaskRay edited https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
@@ -96,13 +97,39 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
llvm::SmallVector &Selected) const {
llvm::StringSet<> FlagSet(expandFlags(Flags));
Selected.clear();
- llvm::copy_if(Multilibs, std::back_inserter(Selected),
-[&FlagSet](const Multilib &M) {
- for (const std::string &F : M.flags())
-if (!FlagSet.contains(F))
- return false;
- return true;
-});
+
+ // Decide which multilibs we're going to select at all
+ std::vector IsSelected(Multilibs.size(), false);
+ std::map ExclusiveGroupMembers;
+ for (size_t i = 0, e = Multilibs.size(); i < e; ++i) {
+const Multilib &M = Multilibs[i];
+
+// If this multilib doesn't match all our flags, don't select it
+if (!llvm::all_of(M.flags(), [&FlagSet](const std::string &F) {
+ return FlagSet.contains(F);
+}))
+ continue;
+
+// If this multilib has the same ExclusiveGroup as one we've already
+// selected, de-select the previous one
+const std::string &group = M.exclusiveGroup();
+if (!group.empty()) {
MaskRay wrote:
Thanks for the update, but the new code still has the duplicate hash table
lookup/insert problem, which can be fixed by using try_emplace.
https://github.com/llvm/llvm-project/pull/69447
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
@@ -96,13 +98,36 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
llvm::SmallVector &Selected) const {
llvm::StringSet<> FlagSet(expandFlags(Flags));
Selected.clear();
- llvm::copy_if(Multilibs, std::back_inserter(Selected),
-[&FlagSet](const Multilib &M) {
- for (const std::string &F : M.flags())
-if (!FlagSet.contains(F))
- return false;
- return true;
-});
+
+ // Decide which multilibs we're going to select at all.
+ llvm::DenseSet ExclusiveGroupsSelected;
+
MaskRay wrote:
The common llvm-project style does not typically insert a blank line after a
variable declaration, different from the Linux kernel style.
https://github.com/llvm/llvm-project/pull/69447
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
@@ -0,0 +1,70 @@ +# REQUIRES: shell MaskRay wrote: If `LIT_USE_INTERNAL_SHELL=1 llvm-lit ..` passes, you can remove `# REQUIRES: shell` (dependency on an external shell) https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
https://github.com/MaskRay approved this pull request. Looks good to me, but other reviewers likely have opinions https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
https://github.com/statham-arm updated
https://github.com/llvm/llvm-project/pull/69447
>From 2a65ae75e8c8e62e7275a439849837919599e896 Mon Sep 17 00:00:00 2001
From: Simon Tatham
Date: Thu, 14 Sep 2023 14:51:17 +0100
Subject: [PATCH 1/2] [Driver] Add ExclusiveGroup feature to multilib.yaml.
This allows a YAML-based multilib configuration to specify explicitly
that a subset of its library directories are alternatives to each
other, i.e. at most one of that subset should be selected.
So if you have multiple sysroots each including a full set of headers
and libraries, you can mark them as members of the same
ExclusiveGroup, and then you'll be sure that only one of them is
selected, even if two or more are compatible with the compile options.
This is particularly important in multilib setups including the libc++
headers, where selecting the include directories from two different
sysroots can cause an actual build failure. This occurs when including
, for example: libc++'s stdio.h is included first, and will
try to use `#include_next` to fetch the underlying libc's version. But
if there are two include directories from separate multilibs, then
both of their C++ include directories will end up on the include path
first, followed by both the C directories. So the `#include_next` from
the first libc++ stdio.h will include the second libc++ stdio.h, which
will do nothing because it has the same include guard macro, and the
libc header won't ever be included at all.
If more than one of the options in an ExclusiveGroup matches the given
flags, the last one wins.
---
clang/include/clang/Driver/Multilib.h | 16 -
clang/lib/Driver/Multilib.cpp | 49 ++---
.../baremetal-multilib-exclusive-group.yaml | 69 +++
3 files changed, 122 insertions(+), 12 deletions(-)
create mode 100644 clang/test/Driver/baremetal-multilib-exclusive-group.yaml
diff --git a/clang/include/clang/Driver/Multilib.h
b/clang/include/clang/Driver/Multilib.h
index 1416559414f894b..6a9533e6dd831f1 100644
--- a/clang/include/clang/Driver/Multilib.h
+++ b/clang/include/clang/Driver/Multilib.h
@@ -39,13 +39,22 @@ class Multilib {
std::string IncludeSuffix;
flags_list Flags;
+ // Optionally, a multilib can be assigned a string tag indicating that it's
+ // part of a group of mutually exclusive possibilities. If two or more
+ // multilibs have the same non-empty value of ExclusiveGroup, then only the
+ // last matching one of them will be selected.
+ //
+ // Setting this to the empty string is a special case, indicating that the
+ // directory is not mutually exclusive with anything else.
+ std::string ExclusiveGroup;
+
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 IncludeSuffix = {}, const flags_list &Flags =
flags_list(),
+ StringRef ExclusiveGroup = {});
/// Get the detected GCC installation path suffix for the multi-arch
/// target variant. Always starts with a '/', unless empty
@@ -63,6 +72,9 @@ class Multilib {
/// All elements begin with either '-' or '!'
const flags_list &flags() const { return Flags; }
+ /// Get the exclusive group label.
+ const std::string &exclusiveGroup() const { return ExclusiveGroup; }
+
LLVM_DUMP_METHOD void dump() const;
/// print summary of the Multilib
void print(raw_ostream &OS) const;
diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp
index 48a494d9fa38db5..085ccee7b25752e 100644
--- a/clang/lib/Driver/Multilib.cpp
+++ b/clang/lib/Driver/Multilib.cpp
@@ -29,9 +29,10 @@ using namespace driver;
using namespace llvm::sys;
Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix,
- StringRef IncludeSuffix, const flags_list &Flags)
+ StringRef IncludeSuffix, const flags_list &Flags,
+ StringRef ExclusiveGroup)
: GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix),
- Flags(Flags) {
+ Flags(Flags), ExclusiveGroup(ExclusiveGroup) {
assert(GCCSuffix.empty() ||
(StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1));
assert(OSSuffix.empty() ||
@@ -96,13 +97,39 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
llvm::SmallVector &Selected) const {
llvm::StringSet<> FlagSet(expandFlags(Flags));
Selected.clear();
- llvm::copy_if(Multilibs, std::back_inserter(Selected),
-[&FlagSet](const Multilib &M) {
- for (const std::string &F : M.flags())
-if (!FlagSet.contains(F))
- return false;
- return
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
@@ -152,6 +180,7 @@ template <> struct
llvm::yaml::MappingTraits {
static void mapping(llvm::yaml::IO &io, MultilibSerialization &V) {
io.mapRequired("Dir", V.Dir);
io.mapRequired("Flags", V.Flags);
+io.mapOptional("ExclusiveGroup", V.ExclusiveGroup);
statham-arm wrote:
Thanks, @MaskRay. I've left it as `ExclusiveGroup` for now.
https://github.com/llvm/llvm-project/pull/69447
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
@@ -0,0 +1,69 @@ +# REQUIRES: shell +# UNSUPPORTED: system-windows + +# RUN: rm -rf %t + +# RUN: mkdir -p %t/baremetal_multilib/bin +# RUN: ln -s %clang %t/baremetal_multilib/bin/clang + +# RUN: mkdir -p %t/baremetal_multilib/lib/clang-runtimes +# RUN: ln -s %s %t/baremetal_multilib/lib/clang-runtimes/multilib.yaml + +# RUN: %t/baremetal_multilib/bin/clang -no-canonical-prefixes -x c++ %s -### -o %t.out --target=thumbv7em-none-unknown-eabi --sysroot= 2>%t.err + +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR1_NON_EXCLUSIVE +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR2_NON_EXCLUSIVE +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR1_EXCLUSIVE +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR2_EXCLUSIVE +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR1_OWN_GROUP +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR2_OWN_GROUP + +# Expected results: +# +# Due to the Mappings section, all six of these library directories should +# match the command-line flag --target=thumbv7em-none-unknown-eabi. +# +# The two "non_exclusive" directories, which don't have an ExclusiveGroup at +# all, should both be selected. So should the two "own_group", each of which +# specifies a different value of ExclusiveGroup. But the two "exclusive", which +# have the _same_ ExclusiveGroup value, should not: the second one wins. So we +# expect five of these six directories to show up in the clang-cc1 command +# line, but not testdir1_exclusive. + +# TESTDIR1_NON_EXCLUSIVE: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir1_non_exclusive/include/c++/v1" +# TESTDIR2_NON_EXCLUSIVE: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir2_non_exclusive/include/c++/v1" +# TESTDIR2_EXCLUSIVE: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir2_exclusive/include/c++/v1" +# TESTDIR1_OWN_GROUP: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir1_own_group/include/c++/v1" +# TESTDIR2_OWN_GROUP: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir2_own_group/include/c++/v1" statham-arm wrote: Thanks. Yes, I agree it would be nice to have a convenient way to share headers in cases where they don't have to vary. But for the cases where they do have to, this feature is still vital. (And for the cases where we just haven't got round to it yet it's still _useful_ :-) https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
https://github.com/statham-arm updated
https://github.com/llvm/llvm-project/pull/69447
>From 2a65ae75e8c8e62e7275a439849837919599e896 Mon Sep 17 00:00:00 2001
From: Simon Tatham
Date: Thu, 14 Sep 2023 14:51:17 +0100
Subject: [PATCH 1/2] [Driver] Add ExclusiveGroup feature to multilib.yaml.
This allows a YAML-based multilib configuration to specify explicitly
that a subset of its library directories are alternatives to each
other, i.e. at most one of that subset should be selected.
So if you have multiple sysroots each including a full set of headers
and libraries, you can mark them as members of the same
ExclusiveGroup, and then you'll be sure that only one of them is
selected, even if two or more are compatible with the compile options.
This is particularly important in multilib setups including the libc++
headers, where selecting the include directories from two different
sysroots can cause an actual build failure. This occurs when including
, for example: libc++'s stdio.h is included first, and will
try to use `#include_next` to fetch the underlying libc's version. But
if there are two include directories from separate multilibs, then
both of their C++ include directories will end up on the include path
first, followed by both the C directories. So the `#include_next` from
the first libc++ stdio.h will include the second libc++ stdio.h, which
will do nothing because it has the same include guard macro, and the
libc header won't ever be included at all.
If more than one of the options in an ExclusiveGroup matches the given
flags, the last one wins.
---
clang/include/clang/Driver/Multilib.h | 16 -
clang/lib/Driver/Multilib.cpp | 49 ++---
.../baremetal-multilib-exclusive-group.yaml | 69 +++
3 files changed, 122 insertions(+), 12 deletions(-)
create mode 100644 clang/test/Driver/baremetal-multilib-exclusive-group.yaml
diff --git a/clang/include/clang/Driver/Multilib.h
b/clang/include/clang/Driver/Multilib.h
index 1416559414f894b..6a9533e6dd831f1 100644
--- a/clang/include/clang/Driver/Multilib.h
+++ b/clang/include/clang/Driver/Multilib.h
@@ -39,13 +39,22 @@ class Multilib {
std::string IncludeSuffix;
flags_list Flags;
+ // Optionally, a multilib can be assigned a string tag indicating that it's
+ // part of a group of mutually exclusive possibilities. If two or more
+ // multilibs have the same non-empty value of ExclusiveGroup, then only the
+ // last matching one of them will be selected.
+ //
+ // Setting this to the empty string is a special case, indicating that the
+ // directory is not mutually exclusive with anything else.
+ std::string ExclusiveGroup;
+
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 IncludeSuffix = {}, const flags_list &Flags =
flags_list(),
+ StringRef ExclusiveGroup = {});
/// Get the detected GCC installation path suffix for the multi-arch
/// target variant. Always starts with a '/', unless empty
@@ -63,6 +72,9 @@ class Multilib {
/// All elements begin with either '-' or '!'
const flags_list &flags() const { return Flags; }
+ /// Get the exclusive group label.
+ const std::string &exclusiveGroup() const { return ExclusiveGroup; }
+
LLVM_DUMP_METHOD void dump() const;
/// print summary of the Multilib
void print(raw_ostream &OS) const;
diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp
index 48a494d9fa38db5..085ccee7b25752e 100644
--- a/clang/lib/Driver/Multilib.cpp
+++ b/clang/lib/Driver/Multilib.cpp
@@ -29,9 +29,10 @@ using namespace driver;
using namespace llvm::sys;
Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix,
- StringRef IncludeSuffix, const flags_list &Flags)
+ StringRef IncludeSuffix, const flags_list &Flags,
+ StringRef ExclusiveGroup)
: GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix),
- Flags(Flags) {
+ Flags(Flags), ExclusiveGroup(ExclusiveGroup) {
assert(GCCSuffix.empty() ||
(StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1));
assert(OSSuffix.empty() ||
@@ -96,13 +97,39 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
llvm::SmallVector &Selected) const {
llvm::StringSet<> FlagSet(expandFlags(Flags));
Selected.clear();
- llvm::copy_if(Multilibs, std::back_inserter(Selected),
-[&FlagSet](const Multilib &M) {
- for (const std::string &F : M.flags())
-if (!FlagSet.contains(F))
- return false;
- return
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
https://github.com/MaskRay edited https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
https://github.com/MaskRay edited https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
@@ -96,13 +97,39 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
llvm::SmallVector &Selected) const {
llvm::StringSet<> FlagSet(expandFlags(Flags));
Selected.clear();
- llvm::copy_if(Multilibs, std::back_inserter(Selected),
-[&FlagSet](const Multilib &M) {
- for (const std::string &F : M.flags())
-if (!FlagSet.contains(F))
- return false;
- return true;
-});
+
+ // Decide which multilibs we're going to select at all
+ std::vector IsSelected(Multilibs.size(), false);
+ std::map ExclusiveGroupMembers;
+ for (size_t i = 0, e = Multilibs.size(); i < e; ++i) {
+const Multilib &M = Multilibs[i];
+
+// If this multilib doesn't match all our flags, don't select it
MaskRay wrote:
End complete sentences with a period.
https://llvm.org/docs/CodingStandards.html#commenting
https://github.com/llvm/llvm-project/pull/69447
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
@@ -0,0 +1,69 @@ +# REQUIRES: shell +# UNSUPPORTED: system-windows + +# RUN: rm -rf %t + +# RUN: mkdir -p %t/baremetal_multilib/bin +# RUN: ln -s %clang %t/baremetal_multilib/bin/clang + +# RUN: mkdir -p %t/baremetal_multilib/lib/clang-runtimes +# RUN: ln -s %s %t/baremetal_multilib/lib/clang-runtimes/multilib.yaml + +# RUN: %t/baremetal_multilib/bin/clang -no-canonical-prefixes -x c++ %s -### -o %t.out --target=thumbv7em-none-unknown-eabi --sysroot= 2>%t.err + +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR1_NON_EXCLUSIVE +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR2_NON_EXCLUSIVE +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR1_EXCLUSIVE +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR2_EXCLUSIVE +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR1_OWN_GROUP +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR2_OWN_GROUP + +# Expected results: +# +# Due to the Mappings section, all six of these library directories should +# match the command-line flag --target=thumbv7em-none-unknown-eabi. +# +# The two "non_exclusive" directories, which don't have an ExclusiveGroup at +# all, should both be selected. So should the two "own_group", each of which +# specifies a different value of ExclusiveGroup. But the two "exclusive", which +# have the _same_ ExclusiveGroup value, should not: the second one wins. So we +# expect five of these six directories to show up in the clang-cc1 command +# line, but not testdir1_exclusive. + +# TESTDIR1_NON_EXCLUSIVE: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir1_non_exclusive/include/c++/v1" MaskRay wrote: `-DAG` may be useful here to avoid running FileCheck multiple times. https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
@@ -152,6 +180,7 @@ template <> struct
llvm::yaml::MappingTraits {
static void mapping(llvm::yaml::IO &io, MultilibSerialization &V) {
io.mapRequired("Dir", V.Dir);
io.mapRequired("Flags", V.Flags);
+io.mapOptional("ExclusiveGroup", V.ExclusiveGroup);
MaskRay wrote:
`ExclusiveGroup` looks good to me and I agree that `Group` could be confusing.
https://github.com/llvm/llvm-project/pull/69447
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
@@ -0,0 +1,69 @@ +# REQUIRES: shell +# UNSUPPORTED: system-windows + +# RUN: rm -rf %t + +# RUN: mkdir -p %t/baremetal_multilib/bin +# RUN: ln -s %clang %t/baremetal_multilib/bin/clang + +# RUN: mkdir -p %t/baremetal_multilib/lib/clang-runtimes +# RUN: ln -s %s %t/baremetal_multilib/lib/clang-runtimes/multilib.yaml + +# RUN: %t/baremetal_multilib/bin/clang -no-canonical-prefixes -x c++ %s -### -o %t.out --target=thumbv7em-none-unknown-eabi --sysroot= 2>%t.err + +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR1_NON_EXCLUSIVE +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR2_NON_EXCLUSIVE +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR1_EXCLUSIVE +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR2_EXCLUSIVE +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR1_OWN_GROUP +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR2_OWN_GROUP + +# Expected results: +# +# Due to the Mappings section, all six of these library directories should +# match the command-line flag --target=thumbv7em-none-unknown-eabi. +# +# The two "non_exclusive" directories, which don't have an ExclusiveGroup at +# all, should both be selected. So should the two "own_group", each of which +# specifies a different value of ExclusiveGroup. But the two "exclusive", which +# have the _same_ ExclusiveGroup value, should not: the second one wins. So we +# expect five of these six directories to show up in the clang-cc1 command +# line, but not testdir1_exclusive. + +# TESTDIR1_NON_EXCLUSIVE: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir1_non_exclusive/include/c++/v1" +# TESTDIR2_NON_EXCLUSIVE: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir2_non_exclusive/include/c++/v1" +# TESTDIR2_EXCLUSIVE: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir2_exclusive/include/c++/v1" +# TESTDIR1_OWN_GROUP: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir1_own_group/include/c++/v1" +# TESTDIR2_OWN_GROUP: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir2_own_group/include/c++/v1" + +# TESTDIR1_EXCLUSIVE-NOT: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir1_exclusive/include/c++/v1" + +--- +MultilibVersion: 1.0 + +Variants: +- Dir: testdir1_non_exclusive + Flags: [--target=thumbv7m-none-unknown-eabi] + +- Dir: testdir2_non_exclusive + Flags: [--target=thumbv7em-none-unknown-eabi] + +- Dir: testdir1_exclusive + Flags: [--target=thumbv7m-none-unknown-eabi] + ExclusiveGroup: actually_exclude_something MaskRay wrote: If `actually_exclude_something` appears three times, the test will become slightly stronger. https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
@@ -96,13 +97,39 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
llvm::SmallVector &Selected) const {
llvm::StringSet<> FlagSet(expandFlags(Flags));
Selected.clear();
- llvm::copy_if(Multilibs, std::back_inserter(Selected),
-[&FlagSet](const Multilib &M) {
- for (const std::string &F : M.flags())
-if (!FlagSet.contains(F))
- return false;
- return true;
-});
+
+ // Decide which multilibs we're going to select at all
+ std::vector IsSelected(Multilibs.size(), false);
+ std::map ExclusiveGroupMembers;
+ for (size_t i = 0, e = Multilibs.size(); i < e; ++i) {
+const Multilib &M = Multilibs[i];
+
+// If this multilib doesn't match all our flags, don't select it
+if (!llvm::all_of(M.flags(), [&FlagSet](const std::string &F) {
+ return FlagSet.contains(F);
+}))
+ continue;
+
+// If this multilib has the same ExclusiveGroup as one we've already
+// selected, de-select the previous one
+const std::string &group = M.exclusiveGroup();
+if (!group.empty()) {
MaskRay wrote:
The hash table entry is looked up twice, and can be simplified to once.
```
if (!group.empty()) {
auto [It, Inserted] = ExclusiveGroupMembers.try_emplace(...);
if (!Inserted) {
IsSelected[it->second] = false;
it->second = i;
}
}
```
Actually, I'd iterate over the array in the reverse order and add an element if
its `ExclusiveGroup` hasn't been seen. Then reverse `Selected`
https://github.com/llvm/llvm-project/pull/69447
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
@@ -152,6 +180,7 @@ template <> struct
llvm::yaml::MappingTraits {
static void mapping(llvm::yaml::IO &io, MultilibSerialization &V) {
io.mapRequired("Dir", V.Dir);
io.mapRequired("Flags", V.Flags);
+io.mapOptional("ExclusiveGroup", V.ExclusiveGroup);
statham-arm wrote:
I'll rename it if you like, but I worry that that might be ambiguous, or at
least unclear. Within the general context of linking and libraries, "group"
need not mean a _mutually exclusive_ group; it could mean a grouping of
libraries for other purposes too, like a mutually _dependent_ group (you must
select all of these or none).
https://github.com/llvm/llvm-project/pull/69447
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
@@ -0,0 +1,69 @@ +# REQUIRES: shell +# UNSUPPORTED: system-windows + +# RUN: rm -rf %t + +# RUN: mkdir -p %t/baremetal_multilib/bin +# RUN: ln -s %clang %t/baremetal_multilib/bin/clang + +# RUN: mkdir -p %t/baremetal_multilib/lib/clang-runtimes +# RUN: ln -s %s %t/baremetal_multilib/lib/clang-runtimes/multilib.yaml + +# RUN: %t/baremetal_multilib/bin/clang -no-canonical-prefixes -x c++ %s -### -o %t.out --target=thumbv7em-none-unknown-eabi --sysroot= 2>%t.err + +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR1_NON_EXCLUSIVE +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR2_NON_EXCLUSIVE +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR1_EXCLUSIVE +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR2_EXCLUSIVE +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR1_OWN_GROUP +# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=TESTDIR2_OWN_GROUP + +# Expected results: +# +# Due to the Mappings section, all six of these library directories should +# match the command-line flag --target=thumbv7em-none-unknown-eabi. +# +# The two "non_exclusive" directories, which don't have an ExclusiveGroup at +# all, should both be selected. So should the two "own_group", each of which +# specifies a different value of ExclusiveGroup. But the two "exclusive", which +# have the _same_ ExclusiveGroup value, should not: the second one wins. So we +# expect five of these six directories to show up in the clang-cc1 command +# line, but not testdir1_exclusive. + +# TESTDIR1_NON_EXCLUSIVE: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir1_non_exclusive/include/c++/v1" +# TESTDIR2_NON_EXCLUSIVE: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir2_non_exclusive/include/c++/v1" +# TESTDIR2_EXCLUSIVE: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir2_exclusive/include/c++/v1" +# TESTDIR1_OWN_GROUP: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir1_own_group/include/c++/v1" +# TESTDIR2_OWN_GROUP: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir2_own_group/include/c++/v1" petrhosek wrote: This comment is unrelated to this change, but the fact that we end up with `[[SYSROOT]]/bin/../lib/clang-runtimes//include/c++/v1` is suboptimal. libc++ headers are designed to be target agnostic with the exception of `__config_site` to avoid having to distribute multiple copies of headers, one for each target or multilib. Ideally, what we should end up with is something like this: ``` -internal-isystem [[SYSROOT]]/bin/../include/thumbv7m-none-unknown-eabi/testdir1_non_exclusive/include/c++/v1 -internal-isystem [[SYSROOT]]/bin/../include/thumbv7m-none-unknown-eabi/testdir2_non_exclusive/include/c++/v1 -internal-isystem [[SYSROOT]]/bin/../include/thumbv7m-none-unknown-eabi/testdir2_exclusive/include/c++/v1 -internal-isystem [[SYSROOT]]/bin/../include/thumbv7m-none-unknown-eabi/testdir1_own_group/include/c++/v1 -internal-isystem [[SYSROOT]]/bin/../include/thumbv7m-none-unknown-eabi/testdir2_own_group/include/c++/v1 -internal-isystem [[SYSROOT]]/bin/../include/c++/v1 ``` I have filed https://github.com/llvm/llvm-project/issues/70172 to track this. https://github.com/llvm/llvm-project/pull/69447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
@@ -152,6 +180,7 @@ template <> struct
llvm::yaml::MappingTraits {
static void mapping(llvm::yaml::IO &io, MultilibSerialization &V) {
io.mapRequired("Dir", V.Dir);
io.mapRequired("Flags", V.Flags);
+io.mapOptional("ExclusiveGroup", V.ExclusiveGroup);
petrhosek wrote:
Could we just call it a `Group` for simplicity?
https://github.com/llvm/llvm-project/pull/69447
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
https://github.com/statham-arm updated
https://github.com/llvm/llvm-project/pull/69447
>From 2a65ae75e8c8e62e7275a439849837919599e896 Mon Sep 17 00:00:00 2001
From: Simon Tatham
Date: Thu, 14 Sep 2023 14:51:17 +0100
Subject: [PATCH] [Driver] Add ExclusiveGroup feature to multilib.yaml.
This allows a YAML-based multilib configuration to specify explicitly
that a subset of its library directories are alternatives to each
other, i.e. at most one of that subset should be selected.
So if you have multiple sysroots each including a full set of headers
and libraries, you can mark them as members of the same
ExclusiveGroup, and then you'll be sure that only one of them is
selected, even if two or more are compatible with the compile options.
This is particularly important in multilib setups including the libc++
headers, where selecting the include directories from two different
sysroots can cause an actual build failure. This occurs when including
, for example: libc++'s stdio.h is included first, and will
try to use `#include_next` to fetch the underlying libc's version. But
if there are two include directories from separate multilibs, then
both of their C++ include directories will end up on the include path
first, followed by both the C directories. So the `#include_next` from
the first libc++ stdio.h will include the second libc++ stdio.h, which
will do nothing because it has the same include guard macro, and the
libc header won't ever be included at all.
If more than one of the options in an ExclusiveGroup matches the given
flags, the last one wins.
---
clang/include/clang/Driver/Multilib.h | 16 -
clang/lib/Driver/Multilib.cpp | 49 ++---
.../baremetal-multilib-exclusive-group.yaml | 69 +++
3 files changed, 122 insertions(+), 12 deletions(-)
create mode 100644 clang/test/Driver/baremetal-multilib-exclusive-group.yaml
diff --git a/clang/include/clang/Driver/Multilib.h
b/clang/include/clang/Driver/Multilib.h
index 1416559414f894b..6a9533e6dd831f1 100644
--- a/clang/include/clang/Driver/Multilib.h
+++ b/clang/include/clang/Driver/Multilib.h
@@ -39,13 +39,22 @@ class Multilib {
std::string IncludeSuffix;
flags_list Flags;
+ // Optionally, a multilib can be assigned a string tag indicating that it's
+ // part of a group of mutually exclusive possibilities. If two or more
+ // multilibs have the same non-empty value of ExclusiveGroup, then only the
+ // last matching one of them will be selected.
+ //
+ // Setting this to the empty string is a special case, indicating that the
+ // directory is not mutually exclusive with anything else.
+ std::string ExclusiveGroup;
+
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 IncludeSuffix = {}, const flags_list &Flags =
flags_list(),
+ StringRef ExclusiveGroup = {});
/// Get the detected GCC installation path suffix for the multi-arch
/// target variant. Always starts with a '/', unless empty
@@ -63,6 +72,9 @@ class Multilib {
/// All elements begin with either '-' or '!'
const flags_list &flags() const { return Flags; }
+ /// Get the exclusive group label.
+ const std::string &exclusiveGroup() const { return ExclusiveGroup; }
+
LLVM_DUMP_METHOD void dump() const;
/// print summary of the Multilib
void print(raw_ostream &OS) const;
diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp
index 48a494d9fa38db5..085ccee7b25752e 100644
--- a/clang/lib/Driver/Multilib.cpp
+++ b/clang/lib/Driver/Multilib.cpp
@@ -29,9 +29,10 @@ using namespace driver;
using namespace llvm::sys;
Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix,
- StringRef IncludeSuffix, const flags_list &Flags)
+ StringRef IncludeSuffix, const flags_list &Flags,
+ StringRef ExclusiveGroup)
: GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix),
- Flags(Flags) {
+ Flags(Flags), ExclusiveGroup(ExclusiveGroup) {
assert(GCCSuffix.empty() ||
(StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1));
assert(OSSuffix.empty() ||
@@ -96,13 +97,39 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
llvm::SmallVector &Selected) const {
llvm::StringSet<> FlagSet(expandFlags(Flags));
Selected.clear();
- llvm::copy_if(Multilibs, std::back_inserter(Selected),
-[&FlagSet](const Multilib &M) {
- for (const std::string &F : M.flags())
-if (!FlagSet.contains(F))
- return false;
- return true
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
https://github.com/statham-arm updated
https://github.com/llvm/llvm-project/pull/69447
>From e4d860c2968e4bf2e0ca198bdfe00dad4e985d40 Mon Sep 17 00:00:00 2001
From: Simon Tatham
Date: Thu, 14 Sep 2023 14:51:17 +0100
Subject: [PATCH] [Driver] Add ExclusiveGroup feature to multilib.yaml.
This allows a YAML-based multilib configuration to specify explicitly
that a subset of its library directories are alternatives to each
other, i.e. at most one of that subset should be selected.
So if you have multiple sysroots each including a full set of headers
and libraries, you can mark them as members of the same
ExclusiveGroup, and then you'll be sure that only one of them is
selected, even if two or more are compatible with the compile options.
This is particularly important in multilib setups including the libc++
headers, where selecting the include directories from two different
sysroots can cause an actual build failure. This occurs when including
, for example: libc++'s stdio.h is included first, and will
try to use `#include_next` to fetch the underlying libc's version. But
if there are two include directories from separate multilibs, then
both of their C++ include directories will end up on the include path
first, followed by both the C directories. So the `#include_next` from
the first libc++ stdio.h will include the second libc++ stdio.h, which
will do nothing because it has the same include guard macro, and the
libc header won't ever be included at all.
If more than one of the options in an ExclusiveGroup matches the given
flags, the last one wins.
---
clang/include/clang/Driver/Multilib.h | 16 -
clang/lib/Driver/Multilib.cpp | 49 ++---
.../baremetal-multilib-exclusive-group.yaml | 69 +++
3 files changed, 122 insertions(+), 12 deletions(-)
create mode 100644 clang/test/Driver/baremetal-multilib-exclusive-group.yaml
diff --git a/clang/include/clang/Driver/Multilib.h
b/clang/include/clang/Driver/Multilib.h
index 1416559414f894b..6a9533e6dd831f1 100644
--- a/clang/include/clang/Driver/Multilib.h
+++ b/clang/include/clang/Driver/Multilib.h
@@ -39,13 +39,22 @@ class Multilib {
std::string IncludeSuffix;
flags_list Flags;
+ // Optionally, a multilib can be assigned a string tag indicating that it's
+ // part of a group of mutually exclusive possibilities. If two or more
+ // multilibs have the same non-empty value of ExclusiveGroup, then only the
+ // last matching one of them will be selected.
+ //
+ // Setting this to the empty string is a special case, indicating that the
+ // directory is not mutually exclusive with anything else.
+ std::string ExclusiveGroup;
+
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 IncludeSuffix = {}, const flags_list &Flags =
flags_list(),
+ StringRef ExclusiveGroup = {});
/// Get the detected GCC installation path suffix for the multi-arch
/// target variant. Always starts with a '/', unless empty
@@ -63,6 +72,9 @@ class Multilib {
/// All elements begin with either '-' or '!'
const flags_list &flags() const { return Flags; }
+ /// Get the exclusive group label.
+ const std::string &exclusiveGroup() const { return ExclusiveGroup; }
+
LLVM_DUMP_METHOD void dump() const;
/// print summary of the Multilib
void print(raw_ostream &OS) const;
diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp
index 48a494d9fa38db5..085ccee7b25752e 100644
--- a/clang/lib/Driver/Multilib.cpp
+++ b/clang/lib/Driver/Multilib.cpp
@@ -29,9 +29,10 @@ using namespace driver;
using namespace llvm::sys;
Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix,
- StringRef IncludeSuffix, const flags_list &Flags)
+ StringRef IncludeSuffix, const flags_list &Flags,
+ StringRef ExclusiveGroup)
: GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix),
- Flags(Flags) {
+ Flags(Flags), ExclusiveGroup(ExclusiveGroup) {
assert(GCCSuffix.empty() ||
(StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1));
assert(OSSuffix.empty() ||
@@ -96,13 +97,39 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
llvm::SmallVector &Selected) const {
llvm::StringSet<> FlagSet(expandFlags(Flags));
Selected.clear();
- llvm::copy_if(Multilibs, std::back_inserter(Selected),
-[&FlagSet](const Multilib &M) {
- for (const std::string &F : M.flags())
-if (!FlagSet.contains(F))
- return false;
- return true
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
https://github.com/statham-arm updated
https://github.com/llvm/llvm-project/pull/69447
>From 3a0481134343339ce8132419fde875ac9977b734 Mon Sep 17 00:00:00 2001
From: Simon Tatham
Date: Thu, 14 Sep 2023 14:51:17 +0100
Subject: [PATCH] [Driver] Add ExclusiveGroup feature to multilib.yaml.
This allows a YAML-based multilib configuration to specify explicitly
that a subset of its library directories are alternatives to each
other, i.e. at most one of that subset should be selected.
So if you have multiple sysroots each including a full set of headers
and libraries, you can mark them as members of the same
ExclusiveGroup, and then you'll be sure that only one of them is
selected, even if two or more are compatible with the compile options.
This is particularly important in multilib setups including the libc++
headers, where selecting the include directories from two different
sysroots can cause an actual build failure. This occurs when including
, for example: libc++'s stdio.h is included first, and will
try to use `#include_next` to fetch the underlying libc's version. But
if there are two include directories from separate multilibs, then
both of their C++ include directories will end up on the include path
first, followed by both the C directories. So the `#include_next` from
the first libc++ stdio.h will include the second libc++ stdio.h, which
will do nothing because it has the same include guard macro, and the
libc header won't ever be included at all.
If more than one of the options in an ExclusiveGroup matches the given
flags, the last one wins.
---
clang/include/clang/Driver/Multilib.h | 16 -
clang/lib/Driver/Multilib.cpp | 49 ++---
.../baremetal-multilib-exclusive-group.yaml | 69 +++
3 files changed, 122 insertions(+), 12 deletions(-)
create mode 100644 clang/test/Driver/baremetal-multilib-exclusive-group.yaml
diff --git a/clang/include/clang/Driver/Multilib.h
b/clang/include/clang/Driver/Multilib.h
index 1416559414f894b..6a9533e6dd831f1 100644
--- a/clang/include/clang/Driver/Multilib.h
+++ b/clang/include/clang/Driver/Multilib.h
@@ -39,13 +39,22 @@ class Multilib {
std::string IncludeSuffix;
flags_list Flags;
+ // Optionally, a multilib can be assigned a string tag indicating that it's
+ // part of a group of mutually exclusive possibilities. If two or more
+ // multilibs have the same non-empty value of ExclusiveGroup, then only the
+ // last matching one of them will be selected.
+ //
+ // Setting this to the empty string is a special case, indicating that the
+ // directory is not mutually exclusive with anything else.
+ std::string ExclusiveGroup;
+
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 IncludeSuffix = {}, const flags_list &Flags =
flags_list(),
+ StringRef ExclusiveGroup = {});
/// Get the detected GCC installation path suffix for the multi-arch
/// target variant. Always starts with a '/', unless empty
@@ -63,6 +72,9 @@ class Multilib {
/// All elements begin with either '-' or '!'
const flags_list &flags() const { return Flags; }
+ /// Get the exclusive group label.
+ const std::string &exclusiveGroup() const { return ExclusiveGroup; }
+
LLVM_DUMP_METHOD void dump() const;
/// print summary of the Multilib
void print(raw_ostream &OS) const;
diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp
index ba466af39e2dcaf..a8eff30f1416852 100644
--- a/clang/lib/Driver/Multilib.cpp
+++ b/clang/lib/Driver/Multilib.cpp
@@ -29,9 +29,10 @@ using namespace driver;
using namespace llvm::sys;
Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix,
- StringRef IncludeSuffix, const flags_list &Flags)
+ StringRef IncludeSuffix, const flags_list &Flags,
+ StringRef ExclusiveGroup)
: GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix),
- Flags(Flags) {
+ Flags(Flags), ExclusiveGroup(ExclusiveGroup) {
assert(GCCSuffix.empty() ||
(StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1));
assert(OSSuffix.empty() ||
@@ -96,13 +97,39 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
llvm::SmallVector &Selected) const {
llvm::StringSet<> FlagSet(expandFlags(Flags));
Selected.clear();
- llvm::copy_if(Multilibs, std::back_inserter(Selected),
-[&FlagSet](const Multilib &M) {
- for (const std::string &F : M.flags())
-if (!FlagSet.contains(F))
- return false;
- return true
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff ebdb0cbef5e9be57237403c46bfdbe985313bb1c
5b3289a7ad40850cbe1c438345a181b01c500639 --
clang/include/clang/Driver/Multilib.h clang/lib/Driver/Multilib.cpp
``
View the diff from clang-format here.
``diff
diff --git a/clang/include/clang/Driver/Multilib.h
b/clang/include/clang/Driver/Multilib.h
index 46f23a2ff..6a9533e6d 100644
--- a/clang/include/clang/Driver/Multilib.h
+++ b/clang/include/clang/Driver/Multilib.h
@@ -53,8 +53,7 @@ public:
/// 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 IncludeSuffix = {}, const flags_list &Flags =
flags_list(),
StringRef ExclusiveGroup = {});
/// Get the detected GCC installation path suffix for the multi-arch
``
https://github.com/llvm/llvm-project/pull/69447
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
llvmbot wrote: @llvm/pr-subscribers-clang-driver Author: Simon Tatham (statham-arm) Changes This allows a YAML-based multilib configuration to specify explicitly that a subset of its library directories are alternatives to each other, i.e. at most one of that subset should be selected. So if you have multiple sysroots each including a full set of headers and libraries, you can mark them as members of the same ExclusiveGroup, and then you'll be sure that only one of them is selected, even if two or more are compatible with the compile options. This is particularly important in multilib setups including the libc++ headers, where selecting the include directories from two different sysroots can cause an actual build failure. This occurs when including, for example: libc++'s stdio.h is included first, and will try to use `#include_next` to fetch the underlying libc's version. But if there are two include directories from separate multilibs, then both of their C++ include directories will end up on the include path first, followed by both the C directories. So the `#include_next` from the first libc++ stdio.h will include the second libc++ stdio.h, which will do nothing because it has the same include guard macro, and the libc header won't ever be included at all. If more than one of the options in an ExclusiveGroup matches the given flags, the last one wins. --- Full diff: https://github.com/llvm/llvm-project/pull/69447.diff 3 Files Affected: - (modified) clang/include/clang/Driver/Multilib.h (+14-1) - (modified) clang/lib/Driver/Multilib.cpp (+39-10) - (added) clang/test/Driver/baremetal-multilib-exclusive-group.yaml (+69) ``diff diff --git a/clang/include/clang/Driver/Multilib.h b/clang/include/clang/Driver/Multilib.h index 1416559414f894b..46f23a2ff5fabac 100644 --- a/clang/include/clang/Driver/Multilib.h +++ b/clang/include/clang/Driver/Multilib.h @@ -39,13 +39,23 @@ class Multilib { std::string IncludeSuffix; flags_list Flags; + // Optionally, a multilib can be assigned a string tag indicating that it's + // part of a group of mutually exclusive possibilities. If two or more + // multilibs have the same non-empty value of ExclusiveGroup, then only the + // last matching one of them will be selected. + // + // Setting this to the empty string is a special case, indicating that the + // directory is not mutually exclusive with anything else. + std::string ExclusiveGroup; + 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()); + const flags_list &Flags = flags_list(), + StringRef ExclusiveGroup = {}); /// Get the detected GCC installation path suffix for the multi-arch /// target variant. Always starts with a '/', unless empty @@ -63,6 +73,9 @@ class Multilib { /// All elements begin with either '-' or '!' const flags_list &flags() const { return Flags; } + /// Get the exclusive group label. + const std::string &exclusiveGroup() const { return ExclusiveGroup; } + LLVM_DUMP_METHOD void dump() const; /// print summary of the Multilib void print(raw_ostream &OS) const; diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp index ba466af39e2dcaf..a8eff30f1416852 100644 --- a/clang/lib/Driver/Multilib.cpp +++ b/clang/lib/Driver/Multilib.cpp @@ -29,9 +29,10 @@ using namespace driver; using namespace llvm::sys; Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix, - StringRef IncludeSuffix, const flags_list &Flags) + StringRef IncludeSuffix, const flags_list &Flags, + StringRef ExclusiveGroup) : GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix), - Flags(Flags) { + Flags(Flags), ExclusiveGroup(ExclusiveGroup) { assert(GCCSuffix.empty() || (StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1)); assert(OSSuffix.empty() || @@ -96,13 +97,39 @@ bool MultilibSet::select(const Multilib::flags_list &Flags, llvm::SmallVector &Selected) const { llvm::StringSet<> FlagSet(expandFlags(Flags)); Selected.clear(); - llvm::copy_if(Multilibs, std::back_inserter(Selected), -[&FlagSet](const Multilib &M) { - for (const std::string &F : M.flags()) -if (!FlagSet.contains(F)) - return false; - return true; -}); + + // Decide which multilibs we're going to select at all + std::vector IsSelected(Multilibs.size(), false); + std::map ExclusiveGroupMembers; + for (size_t i = 0, e = Multilibs.size(); i < e; ++i) { +co
[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)
https://github.com/statham-arm created
https://github.com/llvm/llvm-project/pull/69447
This allows a YAML-based multilib configuration to specify explicitly that a
subset of its library directories are alternatives to each other, i.e. at most
one of that subset should be selected.
So if you have multiple sysroots each including a full set of headers and
libraries, you can mark them as members of the same ExclusiveGroup, and then
you'll be sure that only one of them is selected, even if two or more are
compatible with the compile options.
This is particularly important in multilib setups including the libc++ headers,
where selecting the include directories from two different sysroots can cause
an actual build failure. This occurs when including , for example:
libc++'s stdio.h is included first, and will try to use `#include_next` to
fetch the underlying libc's version. But if there are two include directories
from separate multilibs, then both of their C++ include directories will end up
on the include path first, followed by both the C directories. So the
`#include_next` from the first libc++ stdio.h will include the second libc++
stdio.h, which will do nothing because it has the same include guard macro, and
the libc header won't ever be included at all.
If more than one of the options in an ExclusiveGroup matches the given flags,
the last one wins.
>From 5b3289a7ad40850cbe1c438345a181b01c500639 Mon Sep 17 00:00:00 2001
From: Simon Tatham
Date: Thu, 14 Sep 2023 14:51:17 +0100
Subject: [PATCH] [Driver] Add ExclusiveGroup feature to multilib.yaml.
This allows a YAML-based multilib configuration to specify explicitly
that a subset of its library directories are alternatives to each
other, i.e. at most one of that subset should be selected.
So if you have multiple sysroots each including a full set of headers
and libraries, you can mark them as members of the same
ExclusiveGroup, and then you'll be sure that only one of them is
selected, even if two or more are compatible with the compile options.
This is particularly important in multilib setups including the libc++
headers, where selecting the include directories from two different
sysroots can cause an actual build failure. This occurs when including
, for example: libc++'s stdio.h is included first, and will
try to use `#include_next` to fetch the underlying libc's version. But
if there are two include directories from separate multilibs, then
both of their C++ include directories will end up on the include path
first, followed by both the C directories. So the `#include_next` from
the first libc++ stdio.h will include the second libc++ stdio.h, which
will do nothing because it has the same include guard macro, and the
libc header won't ever be included at all.
If more than one of the options in an ExclusiveGroup matches the given
flags, the last one wins.
---
clang/include/clang/Driver/Multilib.h | 15 +++-
clang/lib/Driver/Multilib.cpp | 49 ++---
.../baremetal-multilib-exclusive-group.yaml | 69 +++
3 files changed, 122 insertions(+), 11 deletions(-)
create mode 100644 clang/test/Driver/baremetal-multilib-exclusive-group.yaml
diff --git a/clang/include/clang/Driver/Multilib.h
b/clang/include/clang/Driver/Multilib.h
index 1416559414f894b..46f23a2ff5fabac 100644
--- a/clang/include/clang/Driver/Multilib.h
+++ b/clang/include/clang/Driver/Multilib.h
@@ -39,13 +39,23 @@ class Multilib {
std::string IncludeSuffix;
flags_list Flags;
+ // Optionally, a multilib can be assigned a string tag indicating that it's
+ // part of a group of mutually exclusive possibilities. If two or more
+ // multilibs have the same non-empty value of ExclusiveGroup, then only the
+ // last matching one of them will be selected.
+ //
+ // Setting this to the empty string is a special case, indicating that the
+ // directory is not mutually exclusive with anything else.
+ std::string ExclusiveGroup;
+
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());
+ const flags_list &Flags = flags_list(),
+ StringRef ExclusiveGroup = {});
/// Get the detected GCC installation path suffix for the multi-arch
/// target variant. Always starts with a '/', unless empty
@@ -63,6 +73,9 @@ class Multilib {
/// All elements begin with either '-' or '!'
const flags_list &flags() const { return Flags; }
+ /// Get the exclusive group label.
+ const std::string &exclusiveGroup() const { return ExclusiveGroup; }
+
LLVM_DUMP_METHOD void dump() const;
/// print summary of the Multilib
void print(raw_ostream &OS) const;
diff --git a/clang/lib/
