Skip to content
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

[clang][driver] Cleanup UEFI toolchain driver #111473

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Prabhuk
Copy link
Contributor

@Prabhuk Prabhuk commented Oct 8, 2024

Updating UEFI header includes to not include system include directories, adding includes from compiler resource directory and minor cleanups.

Copy link

github-actions bot commented Oct 8, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

UEFI toolchain predefines clean up. Introducing new __PECOFF__ predefine
for targets that produce PE COFF binaries. Updating UEFI header includes
to not include system include directories.
@Prabhuk Prabhuk marked this pull request as ready for review October 9, 2024 17:10
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 9, 2024
@Prabhuk Prabhuk requested a review from petrhosek October 9, 2024 17:11
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 9, 2024

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Prabhuk (Prabhuk)

Changes

Updating UEFI header includes to not include system include directories, adding includes from compiler resource directory and minor cleanups.


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

4 Files Affected:

  • (modified) clang/lib/Basic/Targets/X86.h (-5)
  • (modified) clang/lib/Driver/ToolChains/UEFI.cpp (+18)
  • (modified) clang/lib/Driver/ToolChains/UEFI.h (+4)
  • (modified) clang/lib/Lex/InitHeaderSearch.cpp (+1)
diff --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index a99ae62984c7d5..de371743481144 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -826,11 +826,6 @@ class LLVM_LIBRARY_VISIBILITY UEFIX86_64TargetInfo
                           "i64:64-i128:128-f80:128-n8:16:32:64-S128");
   }
 
-  void getTargetDefines(const LangOptions &Opts,
-                        MacroBuilder &Builder) const override {
-    getOSDefines(Opts, X86TargetInfo::getTriple(), Builder);
-  }
-
   BuiltinVaListKind getBuiltinVaListKind() const override {
     return TargetInfo::CharPtrBuiltinVaList;
   }
diff --git a/clang/lib/Driver/ToolChains/UEFI.cpp b/clang/lib/Driver/ToolChains/UEFI.cpp
index 66cbbec59246c0..a9d7e7892c5a64 100644
--- a/clang/lib/Driver/ToolChains/UEFI.cpp
+++ b/clang/lib/Driver/ToolChains/UEFI.cpp
@@ -35,6 +35,24 @@ UEFI::UEFI(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
 
 Tool *UEFI::buildLinker() const { return new tools::uefi::Linker(*this); }
 
+void UEFI::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
+                                     ArgStringList &CC1Args) const {
+  if (DriverArgs.hasArg(options::OPT_nostdinc))
+    return;
+
+  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
+    SmallString<128> Dir(getDriver().ResourceDir);
+    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);
+}
+
 void tools::uefi::Linker::ConstructJob(Compilation &C, const JobAction &JA,
                                        const InputInfo &Output,
                                        const InputInfoList &Inputs,
diff --git a/clang/lib/Driver/ToolChains/UEFI.h b/clang/lib/Driver/ToolChains/UEFI.h
index a126ac32db6c6c..7e038b5cb8b186 100644
--- a/clang/lib/Driver/ToolChains/UEFI.h
+++ b/clang/lib/Driver/ToolChains/UEFI.h
@@ -51,6 +51,10 @@ class LLVM_LIBRARY_VISIBILITY UEFI : public ToolChain {
     return false;
   }
   bool isPICDefaultForced() const override { return true; }
+
+  void
+  AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+                            llvm::opt::ArgStringList &CC1Args) const override;
 };
 
 } // namespace toolchains
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp b/clang/lib/Lex/InitHeaderSearch.cpp
index 2218db15013d92..ed404333bd7b3d 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -305,6 +305,7 @@ bool InitHeaderSearch::ShouldAddDefaultIncludePaths(
   case llvm::Triple::PS5:
   case llvm::Triple::RTEMS:
   case llvm::Triple::Solaris:
+  case llvm::Triple::UEFI:
   case llvm::Triple::WASI:
   case llvm::Triple::ZOS:
     return false;

if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
SmallString<128> Dir(getDriver().ResourceDir);
llvm::sys::path::append(Dir, "include");
addSystemInclude(DriverArgs, CC1Args, Dir.str());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@petrhosek -- Introduced this to include headers from clang resource directory. Rest of this method implementation is copied from Baremetal driver without much thought. PTAL.

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:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants