Skip to content

[Driver] Support using toolchain libc and libc++ for baremetal #96736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 2, 2024

Conversation

petrhosek
Copy link
Member

We want to support using a complete Clang/LLVM toolchain that includes LLVM libc and libc++ for baremetal targets. To do so, we need the driver to add the necessary include paths.

We want to support using a complete Clang/LLVM toolchain that includes
LLVM libc and libc++ for baremetal targets. To do so, we need the driver
to add the necessary include paths.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Jun 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 26, 2024

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Petr Hosek (petrhosek)

Changes

We want to support using a complete Clang/LLVM toolchain that includes LLVM libc and libc++ for baremetal targets. To do so, we need the driver to add the necessary include paths.


Full diff: https://github.com/llvm/llvm-project/pull/96736.diff

9 Files Affected:

  • (modified) clang/include/clang/Driver/ToolChain.h (+3)
  • (modified) clang/lib/Driver/ToolChain.cpp (+6)
  • (modified) clang/lib/Driver/ToolChains/BareMetal.cpp (+54-9)
  • (added) clang/test/Driver/Inputs/basic_baremetal_tree/bin/.keep ()
  • (added) clang/test/Driver/Inputs/basic_baremetal_tree/include/armv6m-unknown-none-eabi/.keep ()
  • (added) clang/test/Driver/Inputs/basic_baremetal_tree/include/armv6m-unknown-none-eabi/c++/v1/.keep ()
  • (added) clang/test/Driver/Inputs/basic_baremetal_tree/include/c++/v1/.keep ()
  • (added) clang/test/Driver/Inputs/basic_baremetal_tree/lib/armv6m-unknown-none-eabi/.keep ()
  • (modified) clang/test/Driver/baremetal.cpp (+15-1)
diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h
index 1f93bd612e9b0..ece1384d5d3c0 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -526,6 +526,9 @@ class ToolChain {
   // Returns target specific standard library path if it exists.
   std::optional<std::string> getStdlibPath() const;
 
+  // Returns target specific standard library include path if it exists.
+  std::optional<std::string> getStdlibIncludePath() const;
+
   // Returns <ResourceDir>/lib/<OSName>/<arch> or <ResourceDir>/lib/<triple>.
   // This is used by runtimes (such as OpenMP) to find arch-specific libraries.
   virtual path_list getArchSpecificLibPaths() const;
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 40ab2e91125d1..04021cc0a8f3f 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -811,6 +811,12 @@ std::optional<std::string> ToolChain::getStdlibPath() const {
   return getTargetSubDirPath(P);
 }
 
+std::optional<std::string> ToolChain::getStdlibIncludePath() const {
+  SmallString<128> P(D.Dir);
+  llvm::sys::path::append(P, "..", "include");
+  return getTargetSubDirPath(P);
+}
+
 ToolChain::path_list ToolChain::getArchSpecificLibPaths() const {
   path_list Paths;
 
diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index dd365e62e084e..4eb333efe2314 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -270,15 +270,19 @@ void BareMetal::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
     addSystemInclude(DriverArgs, CC1Args, Dir.str());
   }
 
-  if (!DriverArgs.hasArg(options::OPT_nostdlibinc)) {
-    const SmallString<128> SysRoot(computeSysRoot());
-    if (!SysRoot.empty()) {
-      for (const Multilib &M : getOrderedMultilibs()) {
-        SmallString<128> Dir(SysRoot);
-        llvm::sys::path::append(Dir, M.includeSuffix());
-        llvm::sys::path::append(Dir, "include");
-        addSystemInclude(DriverArgs, CC1Args, Dir.str());
-      }
+  if (DriverArgs.hasArg(options::OPT_nostdlibinc))
+    return;
+
+  if (std::optional<std::string> Path = getStdlibIncludePath())
+    addSystemInclude(DriverArgs, CC1Args, *Path);
+
+  const SmallString<128> SysRoot(computeSysRoot());
+  if (!SysRoot.empty()) {
+    for (const Multilib &M : getOrderedMultilibs()) {
+      SmallString<128> Dir(SysRoot);
+      llvm::sys::path::append(Dir, M.includeSuffix());
+      llvm::sys::path::append(Dir, "include");
+      addSystemInclude(DriverArgs, CC1Args, Dir.str());
     }
   }
 }
@@ -296,6 +300,47 @@ void BareMetal::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
     return;
 
   const Driver &D = getDriver();
+  std::string Target = getTripleString();
+
+  auto AddCXXIncludePath = [&](StringRef Path) {
+    std::string Version = detectLibcxxVersion(Path);
+    if (Version.empty())
+      return;
+
+    // First add the per-target multilib include dir.
+    if (!SelectedMultilibs.empty() && !SelectedMultilibs.back().isDefault()) {
+      const Multilib &M = SelectedMultilibs.back();
+      SmallString<128> TargetDir(Path);
+      llvm::sys::path::append(TargetDir, Target, M.gccSuffix(), "c++", Version);
+      if (getVFS().exists(TargetDir)) {
+        addSystemInclude(DriverArgs, CC1Args, TargetDir);
+      }
+    }
+
+    // Second add the per-target include dir.
+    SmallString<128> TargetDir(Path);
+    llvm::sys::path::append(TargetDir, Target, "c++", Version);
+    if (getVFS().exists(TargetDir))
+      addSystemInclude(DriverArgs, CC1Args, TargetDir);
+
+    // Third the generic one.
+    SmallString<128> Dir(Path);
+    llvm::sys::path::append(Dir, "c++", Version);
+    addSystemInclude(DriverArgs, CC1Args, Dir);
+  };
+
+  switch (GetCXXStdlibType(DriverArgs)) {
+    case ToolChain::CST_Libcxx: {
+      SmallString<128> P(D.Dir);
+      llvm::sys::path::append(P, "..", "include");
+      AddCXXIncludePath(P);
+      break;
+    }
+    case ToolChain::CST_Libstdcxx:
+      // We only support libc++ toolchain installation.
+      break;
+  }
+
   std::string SysRoot(computeSysRoot());
   if (SysRoot.empty())
     return;
diff --git a/clang/test/Driver/Inputs/basic_baremetal_tree/bin/.keep b/clang/test/Driver/Inputs/basic_baremetal_tree/bin/.keep
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_baremetal_tree/include/armv6m-unknown-none-eabi/.keep b/clang/test/Driver/Inputs/basic_baremetal_tree/include/armv6m-unknown-none-eabi/.keep
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_baremetal_tree/include/armv6m-unknown-none-eabi/c++/v1/.keep b/clang/test/Driver/Inputs/basic_baremetal_tree/include/armv6m-unknown-none-eabi/c++/v1/.keep
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_baremetal_tree/include/c++/v1/.keep b/clang/test/Driver/Inputs/basic_baremetal_tree/include/c++/v1/.keep
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/basic_baremetal_tree/lib/armv6m-unknown-none-eabi/.keep b/clang/test/Driver/Inputs/basic_baremetal_tree/lib/armv6m-unknown-none-eabi/.keep
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/baremetal.cpp b/clang/test/Driver/baremetal.cpp
index cc14f045df3f9..d3b0bd2e0e07b 100644
--- a/clang/test/Driver/baremetal.cpp
+++ b/clang/test/Driver/baremetal.cpp
@@ -13,7 +13,7 @@
 // CHECK-V6M-C-SAME: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-V6M-C-SAME: "-isysroot" "[[SYSROOT:[^"]*]]"
 // CHECK-V6M-C-SAME: "-internal-isystem" "[[SYSROOT]]{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
-// CHECk-V6M-C-SAME: "-internal-isystem" "[[SYSROOT]]{{[/\\]+}}include"
+// CHECK-V6M-C-SAME: "-internal-isystem" "[[SYSROOT]]{{[/\\]+}}include"
 // CHECK-V6M-C-SAME: "-x" "c++" "{{.*}}baremetal.cpp"
 // CHECK-V6M-C-NEXT: ld{{(.exe)?}}" "{{.*}}.o" "-Bstatic" "-EL"
 // CHECK-V6M-C-SAME: "-T" "semihosted.lds" "-Lsome{{[/\\]+}}directory{{[/\\]+}}user{{[/\\]+}}asked{{[/\\]+}}for"
@@ -26,6 +26,20 @@
 // RUN:     --sysroot=%S/Inputs/baremetal_arm | FileCheck --check-prefix=CHECK-V6M-LIBINC %s
 // CHECK-V6M-LIBINC-NOT: "-internal-isystem"
 
+// RUN: %clang %s -### --target=armv6m-none-eabi -o %t.out 2>&1 \
+// RUN:     -ccc-install-dir %S/Inputs/basic_baremetal_tree/bin \
+// RUN:   | FileCheck --check-prefix=CHECK-V6M-TREE %s
+// CHECK-V6M-TREE:      InstalledDir: [[INSTALLED_DIR:.+]]
+// CHECK-V6M-TREE:      "-cc1" "-triple" "thumbv6m-unknown-none-eabi"
+// CHECK-V6M-TREE-SAME: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-V6M-TREE-SAME: "-internal-isystem" "[[INSTALLED_DIR]]{{[/\\]+}}..{{[/\\]+}}include{{[/\\]+}}armv6m-unknown-none-eabi{{[/\\]+}}c++{{[/\\]+}}v1"
+// CHECK-V6M-TREE-SAME: "-internal-isystem" "[[INSTALLED_DIR]]{{[/\\]+}}..{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
+// CHECK-V6M-TREE-SAME: "-internal-isystem" "[[INSTALLED_DIR]]{{[/\\]+}}..{{[/\\]+}}include{{[/\\]+}}armv6m-unknown-none-eabi"
+// CHECK-V6M-TREE-SAME: "-x" "c++" "{{.*}}baremetal.cpp"
+// CHECK-V6M-TREE-NEXT: ld{{(.exe)?}}" "{{.*}}.o" "-Bstatic" "-EL"
+// CHECK-V6M-TREE-SAME: "-L[[INSTALLED_DIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}armv6m-unknown-none-eabi"
+// CHECK-V6M-TREE-SAME: "-lc" "-lm" "{{[^"]*}}libclang_rt.builtins.a" "--target2=rel" "-o" "{{.*}}.tmp.out"
+
 // RUN: %clang %s -### --target=armv7m-vendor-none-eabi -rtlib=compiler-rt 2>&1 \
 // RUN:     --sysroot=%S/Inputs/baremetal_arm \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \

@nickdesaulniers nickdesaulniers requested a review from MaskRay June 26, 2024 15:47
Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description can specify the target triple you are adding support for.
LGTM, but other folks more familiar with your planned changes need to stamp as well.

@petrhosek
Copy link
Member Author

petrhosek commented Jul 2, 2024

The description can specify the target triple you are adding support for.

Any target supported by the baremetal driver should be supported as long as you build the necessary runtime libraries.

LGTM, but other folks more familiar with your planned changes need to stamp as well.

@nickdesaulniers is on paternity leave but he didn't raise any concerns, @PiJoules is familiar with the changes.

@petrhosek petrhosek merged commit 135483b into llvm:main Jul 2, 2024
3 of 6 checks passed
Copy link

github-actions bot commented Jul 2, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 7f1a74429dfd62a410d4b51d2e75d3677429a51a b52e1015bf45a1f503ff4b7a890efd1a449881da -- clang/include/clang/Driver/ToolChain.h clang/lib/Driver/ToolChain.cpp clang/lib/Driver/ToolChains/BareMetal.cpp clang/test/Driver/baremetal.cpp
View the diff from clang-format here.
diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index 11f94877a5..a4c4ecb4e5 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -323,15 +323,15 @@ void BareMetal::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
   };
 
   switch (GetCXXStdlibType(DriverArgs)) {
-    case ToolChain::CST_Libcxx: {
-      SmallString<128> P(D.Dir);
-      llvm::sys::path::append(P, "..", "include");
-      AddCXXIncludePath(P);
-      break;
-    }
-    case ToolChain::CST_Libstdcxx:
-      // We only support libc++ toolchain installation.
-      break;
+  case ToolChain::CST_Libcxx: {
+    SmallString<128> P(D.Dir);
+    llvm::sys::path::append(P, "..", "include");
+    AddCXXIncludePath(P);
+    break;
+  }
+  case ToolChain::CST_Libstdcxx:
+    // We only support libc++ toolchain installation.
+    break;
   }
 
   std::string SysRoot(computeSysRoot());

lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024
…96736)

We want to support using a complete Clang/LLVM toolchain that includes
LLVM libc and libc++ for baremetal targets. To do so, we need the driver
to add the necessary include paths.
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
…96736)

We want to support using a complete Clang/LLVM toolchain that includes
LLVM libc and libc++ for baremetal targets. To do so, we need the driver
to add the necessary include paths.
egorzhdan added a commit to swiftlang/llvm-project that referenced this pull request Sep 27, 2024
llvm#96736)"

This reverts commit 135483b.

This change caused Swift to inadvertently use libc++ for Embedded Swift builds, when the libc++ package is installed. Let's temporarily revert this to unblock Embedded Swift builds.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants