[clang] [Driver] Add ExclusiveGroup feature to multilib.yaml. (PR #69447)

Simon Tatham via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 16 05:52:38 PST 2023


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 <simon.tatham at arm.com>
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
<stdio.h>, 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<Multilib> &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<bool> IsSelected(Multilibs.size(), false);
+  std::map<std::string, size_t> 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()) {
+      auto it = ExclusiveGroupMembers.find(group);
+      if (it != ExclusiveGroupMembers.end())
+        IsSelected[it->second] = false;
+    }
+
+    // Select this multilib
+    IsSelected[i] = true;
+    if (!group.empty())
+      ExclusiveGroupMembers[group] = i;
+  }
+
+  // Now write the selected multilibs into the output
+  for (size_t i = 0, e = Multilibs.size(); i < e; ++i)
+    if (IsSelected[i])
+      Selected.push_back(Multilibs[i]);
+
   return !Selected.empty();
 }
 
@@ -138,6 +165,7 @@ static const VersionTuple MultilibVersionCurrent(1, 0);
 struct MultilibSerialization {
   std::string Dir;
   std::vector<std::string> Flags;
+  std::string ExclusiveGroup;
 };
 
 struct MultilibSetSerialization {
@@ -152,6 +180,7 @@ template <> struct llvm::yaml::MappingTraits<MultilibSerialization> {
   static void mapping(llvm::yaml::IO &io, MultilibSerialization &V) {
     io.mapRequired("Dir", V.Dir);
     io.mapRequired("Flags", V.Flags);
+    io.mapOptional("ExclusiveGroup", V.ExclusiveGroup);
   }
   static std::string validate(IO &io, MultilibSerialization &V) {
     if (StringRef(V.Dir).starts_with("/"))
@@ -214,7 +243,7 @@ MultilibSet::parseYaml(llvm::MemoryBufferRef Input,
     std::string Dir;
     if (M.Dir != ".")
       Dir = "/" + M.Dir;
-    Multilibs.emplace_back(Dir, Dir, Dir, M.Flags);
+    Multilibs.emplace_back(Dir, Dir, Dir, M.Flags, M.ExclusiveGroup);
   }
 
   return MultilibSet(std::move(Multilibs), std::move(MS.FlagMatchers));
diff --git a/clang/test/Driver/baremetal-multilib-exclusive-group.yaml b/clang/test/Driver/baremetal-multilib-exclusive-group.yaml
new file mode 100644
index 000000000000000..dc23e90bbeb538b
--- /dev/null
+++ b/clang/test/Driver/baremetal-multilib-exclusive-group.yaml
@@ -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
+
+- Dir: testdir2_exclusive
+  Flags: [--target=thumbv7em-none-unknown-eabi]
+  ExclusiveGroup: actually_exclude_something
+
+- Dir: testdir1_own_group
+  Flags: [--target=thumbv7m-none-unknown-eabi]
+  ExclusiveGroup: foo
+
+- Dir: testdir2_own_group
+  Flags: [--target=thumbv7em-none-unknown-eabi]
+  ExclusiveGroup: bar
+
+Mappings:
+- Match: --target=thumbv7em-none-unknown-eabi
+  Flags: [--target=thumbv7m-none-unknown-eabi]

>From 95f231df63374dd48ce1ba0f06b8f620fe75537a Mon Sep 17 00:00:00 2001
From: Simon Tatham <simon.tatham at arm.com>
Date: Fri, 27 Oct 2023 09:55:11 +0100
Subject: [PATCH 2/4] fixup! [Driver] Add ExclusiveGroup feature to
 multilib.yaml.

Review changes:
 - reverse-order algorithm for selecting multilibs, eliminating a
   string-keyed map
 - added a third exclusive-group member in the test case
 - cut down on the number of FileCheck runs by using -DAG

Not changed:
 - ExclusiveGroup is still called ExclusiveGroup
---
 clang/lib/Driver/Multilib.cpp                 | 34 +++++++++----------
 .../baremetal-multilib-exclusive-group.yaml   | 33 +++++++++---------
 2 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp
index 085ccee7b25752e..e4b1df310faa73f 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"
@@ -98,37 +99,34 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
   llvm::StringSet<> FlagSet(expandFlags(Flags));
   Selected.clear();
 
-  // Decide which multilibs we're going to select at all
-  std::vector<bool> IsSelected(Multilibs.size(), false);
-  std::map<std::string, size_t> ExclusiveGroupMembers;
-  for (size_t i = 0, e = Multilibs.size(); i < e; ++i) {
-    const Multilib &M = Multilibs[i];
+  // Decide which multilibs we're going to select at all.
+  llvm::DenseSet<StringRef> ExclusiveGroupsSelected;
 
-    // If this multilib doesn't match all our flags, don't select it
+  for (const Multilib &M : llvm::reverse(Multilibs)) {
+    // 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()) {
-      auto it = ExclusiveGroupMembers.find(group);
-      if (it != ExclusiveGroupMembers.end())
-        IsSelected[it->second] = false;
+      // If this multilib has the same ExclusiveGroup as one we've already
+      // selected, skip it. We're iterating in reverse order, so the group
+      // member we've selected already is preferred.
+      if (ExclusiveGroupsSelected.contains(group))
+        continue;
     }
 
-    // Select this multilib
-    IsSelected[i] = true;
+    // Select this multilib, and mark its group as selected if it has one.
+    Selected.push_back(M);
     if (!group.empty())
-      ExclusiveGroupMembers[group] = i;
+      ExclusiveGroupsSelected.insert(group);
   }
 
-  // Now write the selected multilibs into the output
-  for (size_t i = 0, e = Multilibs.size(); i < e; ++i)
-    if (IsSelected[i])
-      Selected.push_back(Multilibs[i]);
+  // We iterated in reverse order, so now put Selected back the right way
+  // round.
+  std::reverse(Selected.begin(), Selected.end());
 
   return !Selected.empty();
 }
diff --git a/clang/test/Driver/baremetal-multilib-exclusive-group.yaml b/clang/test/Driver/baremetal-multilib-exclusive-group.yaml
index dc23e90bbeb538b..f27dd172db8df8e 100644
--- a/clang/test/Driver/baremetal-multilib-exclusive-group.yaml
+++ b/clang/test/Driver/baremetal-multilib-exclusive-group.yaml
@@ -11,12 +11,8 @@
 
 # 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
+# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=POS
+# RUN: FileCheck -DSYSROOT=%t/baremetal_multilib %s < %t.err --check-prefix=NEG
 
 # Expected results:
 #
@@ -25,18 +21,19 @@
 #
 # 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.
+# specifies a different value of ExclusiveGroup. But the three "exclusive",
+# which have the _same_ ExclusiveGroup value, should not: the third one wins.
+# So we expect five of these seven directories to show up in the clang-cc1
+# command line, but not testdir1_exclusive or testdir2_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"
+# POS-DAG: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir1_non_exclusive/include/c++/v1"
+# POS-DAG: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir2_non_exclusive/include/c++/v1"
+# POS-DAG: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir3_exclusive/include/c++/v1"
+# POS-DAG: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir1_own_group/include/c++/v1"
+# POS-DAG: "-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"
+# NEG-NOT: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir1_exclusive/include/c++/v1"
+# NEG-NOT: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/testdir2_exclusive/include/c++/v1"
 
 ---
 MultilibVersion: 1.0
@@ -56,6 +53,10 @@ Variants:
   Flags: [--target=thumbv7em-none-unknown-eabi]
   ExclusiveGroup: actually_exclude_something
 
+- Dir: testdir3_exclusive
+  Flags: [--target=thumbv7em-none-unknown-eabi]
+  ExclusiveGroup: actually_exclude_something
+
 - Dir: testdir1_own_group
   Flags: [--target=thumbv7m-none-unknown-eabi]
   ExclusiveGroup: foo

>From f79093486e7b8f9fb835fd364ed3c55f0de9d13e Mon Sep 17 00:00:00 2001
From: Simon Tatham <simon.tatham at arm.com>
Date: Mon, 30 Oct 2023 09:36:06 +0000
Subject: [PATCH 3/4] fixup! [Driver] Add ExclusiveGroup feature to
 multilib.yaml.

---
 clang/lib/Driver/Multilib.cpp                         | 11 ++++++-----
 .../Driver/baremetal-multilib-exclusive-group.yaml    |  1 -
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp
index e4b1df310faa73f..091efeebda7359e 100644
--- a/clang/lib/Driver/Multilib.cpp
+++ b/clang/lib/Driver/Multilib.cpp
@@ -101,7 +101,6 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
 
   // Decide which multilibs we're going to select at all.
   llvm::DenseSet<StringRef> ExclusiveGroupsSelected;
-
   for (const Multilib &M : llvm::reverse(Multilibs)) {
     // If this multilib doesn't match all our flags, don't select it.
     if (!llvm::all_of(M.flags(), [&FlagSet](const std::string &F) {
@@ -114,14 +113,16 @@ bool MultilibSet::select(const Multilib::flags_list &Flags,
       // If this multilib has the same ExclusiveGroup as one we've already
       // selected, skip it. We're iterating in reverse order, so the group
       // member we've selected already is preferred.
-      if (ExclusiveGroupsSelected.contains(group))
+      //
+      // Otherwise, add the group name to the set of groups we've already
+      // selected a member of.
+      auto [It, Inserted] = ExclusiveGroupsSelected.insert(group);
+      if (!Inserted)
         continue;
     }
 
-    // Select this multilib, and mark its group as selected if it has one.
+    // Select this multilib.
     Selected.push_back(M);
-    if (!group.empty())
-      ExclusiveGroupsSelected.insert(group);
   }
 
   // We iterated in reverse order, so now put Selected back the right way
diff --git a/clang/test/Driver/baremetal-multilib-exclusive-group.yaml b/clang/test/Driver/baremetal-multilib-exclusive-group.yaml
index f27dd172db8df8e..1ed73a99461da62 100644
--- a/clang/test/Driver/baremetal-multilib-exclusive-group.yaml
+++ b/clang/test/Driver/baremetal-multilib-exclusive-group.yaml
@@ -1,4 +1,3 @@
-# REQUIRES: shell
 # UNSUPPORTED: system-windows
 
 # RUN: rm -rf %t

>From e701959820846c08e7cc5d4264077bf3fa827340 Mon Sep 17 00:00:00 2001
From: Simon Tatham <simon.tatham at arm.com>
Date: Thu, 16 Nov 2023 10:51:59 +0000
Subject: [PATCH 4/4] squash! [Driver] Allow mutually-exclusive groups in
 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
<stdio.h>, 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/lib/Driver/Multilib.cpp                 | 57 ++++++++++++++++++-
 .../baremetal-multilib-exclusive-group.yaml   | 20 +++++--
 .../baremetal-multilib-group-error.yaml       | 27 +++++++++
 3 files changed, 96 insertions(+), 8 deletions(-)
 create mode 100644 clang/test/Driver/baremetal-multilib-group-error.yaml

diff --git a/clang/lib/Driver/Multilib.cpp b/clang/lib/Driver/Multilib.cpp
index 091efeebda7359e..0fbbebf8f245580 100644
--- a/clang/lib/Driver/Multilib.cpp
+++ b/clang/lib/Driver/Multilib.cpp
@@ -164,11 +164,34 @@ static const VersionTuple MultilibVersionCurrent(1, 0);
 struct MultilibSerialization {
   std::string Dir;
   std::vector<std::string> Flags;
-  std::string ExclusiveGroup;
+  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;
 };
 
 struct MultilibSetSerialization {
   llvm::VersionTuple MultilibVersion;
+  std::vector<MultilibGroupSerialization> Groups;
   std::vector<MultilibSerialization> Multilibs;
   std::vector<MultilibSet::FlagMatcher> FlagMatchers;
 };
@@ -179,7 +202,7 @@ template <> struct llvm::yaml::MappingTraits<MultilibSerialization> {
   static void mapping(llvm::yaml::IO &io, MultilibSerialization &V) {
     io.mapRequired("Dir", V.Dir);
     io.mapRequired("Flags", V.Flags);
-    io.mapOptional("ExclusiveGroup", V.ExclusiveGroup);
+    io.mapOptional("Group", V.Group);
   }
   static std::string validate(IO &io, MultilibSerialization &V) {
     if (StringRef(V.Dir).starts_with("/"))
@@ -188,6 +211,19 @@ template <> struct llvm::yaml::MappingTraits<MultilibSerialization> {
   }
 };
 
+template <> struct llvm::yaml::MappingTraits<MultilibGroupSerialization> {
+  static void mapping(llvm::yaml::IO &io, MultilibGroupSerialization &V) {
+    io.mapRequired("Name", V.Name);
+    io.mapOptional("Type", V.Type);
+  }
+  static std::string validate(IO &io, MultilibGroupSerialization &V) {
+    if (V.Type != "Exclusive")
+      return "group \"" + V.Name + "\" has unrecognized type \"" + V.Type +
+             "\" (only \"Exclusive\" is currently valid)";
+    return std::string{};
+  }
+};
+
 template <> struct llvm::yaml::MappingTraits<MultilibSet::FlagMatcher> {
   static void mapping(llvm::yaml::IO &io, MultilibSet::FlagMatcher &M) {
     io.mapRequired("Match", M.Match);
@@ -208,6 +244,7 @@ template <> struct llvm::yaml::MappingTraits<MultilibSetSerialization> {
   static void mapping(llvm::yaml::IO &io, MultilibSetSerialization &M) {
     io.mapRequired("MultilibVersion", M.MultilibVersion);
     io.mapRequired("Variants", M.Multilibs);
+    io.mapOptional("Groups", M.Groups);
     io.mapOptional("Mappings", M.FlagMatchers);
   }
   static std::string validate(IO &io, MultilibSetSerialization &M) {
@@ -219,11 +256,25 @@ template <> struct llvm::yaml::MappingTraits<MultilibSetSerialization> {
     if (M.MultilibVersion.getMinor() > MultilibVersionCurrent.getMinor())
       return "multilib version " + M.MultilibVersion.getAsString() +
              " is unsupported";
+    for (const MultilibSerialization &Lib : M.Multilibs) {
+      if (!Lib.Group.empty()) {
+        bool Found = false;
+        for (const MultilibGroupSerialization &Group : M.Groups)
+          if (Group.Name == Lib.Group) {
+            Found = true;
+            break;
+          }
+        if (!Found)
+          return "multilib \"" + Lib.Dir +
+                 "\" specifies undefined group name \"" + Lib.Group + "\"";
+      }
+    }
     return std::string{};
   }
 };
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSerialization)
+LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibGroupSerialization)
 LLVM_YAML_IS_SEQUENCE_VECTOR(MultilibSet::FlagMatcher)
 
 llvm::ErrorOr<MultilibSet>
@@ -242,7 +293,7 @@ MultilibSet::parseYaml(llvm::MemoryBufferRef Input,
     std::string Dir;
     if (M.Dir != ".")
       Dir = "/" + M.Dir;
-    Multilibs.emplace_back(Dir, Dir, Dir, M.Flags, M.ExclusiveGroup);
+    Multilibs.emplace_back(Dir, Dir, Dir, M.Flags, M.Group);
   }
 
   return MultilibSet(std::move(Multilibs), std::move(MS.FlagMatchers));
diff --git a/clang/test/Driver/baremetal-multilib-exclusive-group.yaml b/clang/test/Driver/baremetal-multilib-exclusive-group.yaml
index 1ed73a99461da62..a98549efea4f0a9 100644
--- a/clang/test/Driver/baremetal-multilib-exclusive-group.yaml
+++ b/clang/test/Driver/baremetal-multilib-exclusive-group.yaml
@@ -37,6 +37,16 @@
 ---
 MultilibVersion: 1.0
 
+Groups:
+- Name: actually_exclude_something
+  Type: Exclusive
+
+- Name: foo
+  Type: Exclusive
+
+- Name: bar
+  Type: Exclusive
+
 Variants:
 - Dir: testdir1_non_exclusive
   Flags: [--target=thumbv7m-none-unknown-eabi]
@@ -46,23 +56,23 @@ Variants:
 
 - Dir: testdir1_exclusive
   Flags: [--target=thumbv7m-none-unknown-eabi]
-  ExclusiveGroup: actually_exclude_something
+  Group: actually_exclude_something
 
 - Dir: testdir2_exclusive
   Flags: [--target=thumbv7em-none-unknown-eabi]
-  ExclusiveGroup: actually_exclude_something
+  Group: actually_exclude_something
 
 - Dir: testdir3_exclusive
   Flags: [--target=thumbv7em-none-unknown-eabi]
-  ExclusiveGroup: actually_exclude_something
+  Group: actually_exclude_something
 
 - Dir: testdir1_own_group
   Flags: [--target=thumbv7m-none-unknown-eabi]
-  ExclusiveGroup: foo
+  Group: foo
 
 - Dir: testdir2_own_group
   Flags: [--target=thumbv7em-none-unknown-eabi]
-  ExclusiveGroup: bar
+  Group: bar
 
 Mappings:
 - Match: --target=thumbv7em-none-unknown-eabi
diff --git a/clang/test/Driver/baremetal-multilib-group-error.yaml b/clang/test/Driver/baremetal-multilib-group-error.yaml
new file mode 100644
index 000000000000000..d04327c17e5db76
--- /dev/null
+++ b/clang/test/Driver/baremetal-multilib-group-error.yaml
@@ -0,0 +1,27 @@
+# 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 %s < %t.err
+
+---
+MultilibVersion: 1.0
+
+Groups:
+- Name: group1
+  Type: Nonsense
+
+Variants:
+- Dir: testdir1
+  Flags: [--target=thumbv7m-none-unknown-eabi]
+  Group: nonexistent_group_name
+
+# CHECK: error: group "group1" has unrecognized type "Nonsense" (only "Exclusive" is currently valid)
+# CHECK: error: multilib "testdir1" specifies undefined group name "nonexistent_group_name"



More information about the cfe-commits mailing list