Skip to content

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 force-pushed the update_uefi_driver branch from c241379 to 9b55509 Compare October 9, 2024 04:13
@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
Member

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be fine so long as it includes lib/<triple/ and lib/clang/20/lib/<triple>. Do we have an existing clang driver UEFI target test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have a minimal test here: https://github.com/llvm/llvm-project/blob/main/clang/test/Driver/uefi-constructed-args.c

This is the header search paths with this PR. My original goal of this change was to remove /usr/include and /usr/local/include from the search list.

[hi on] prabhukr@prabhukr:~/llvm-builds/b2$ ./bin/clang -E -v --target=x86_64-uefi - < /dev/null
Fuchsia clang version 20.0.0git ([email protected]:prabhuk/llvm-project.git e30c180214f2ca102ba6fcf1b542db7c5f2ac5fd)
Target: x86_64-unknown-uefi
Thread model: posix
InstalledDir: /usr/local/google/home/prabhukr/llvm-builds/b2/bin
Build config: +assertions
 (in-process)
 "/usr/local/google/home/prabhukr/llvm-builds/b2/bin/llvm" "clang" -cc1 -triple x86_64-unknown-uefi -E -disable-free -clear-ast-before-backend -main-file-name - -mrelocation-model pic -pic-level 2 -mframe-pointer=all -fmath-errno -ffp-contract=on -fno-rounding-math -mconstructor-aliases -funwind-tables=2 -target-cpu x86-64 -tune-cpu generic -debugger-tuning=gdb -fdebug-compilation-dir=/usr/local/google/home/prabhukr/llvm-builds/b2 -v -fcoverage-compilation-dir=/usr/local/google/home/prabhukr/llvm-builds/b2 -resource-dir /usr/local/google/home/prabhukr/llvm-builds/b2/lib/clang/20 -internal-isystem /usr/local/google/home/prabhukr/llvm-builds/b2/lib/clang/20/include -ferror-limit 19 -fgnuc-version=4.2.1 -fskip-odr-check-in-gmf -fcolor-diagnostics -faddrsig -o - -x c -
clang -cc1 version 20.0.0git based upon LLVM 20.0.0git default target x86_64-unknown-linux-gnu
#include "..." search starts here:
#include <...> search starts here:
 /usr/local/google/home/prabhukr/llvm-builds/b2/lib/clang/20/include
End of search list.
# 1 "<stdin>"
# 1 "<built-in>" 1
# 1 "<built-in>" 3
# 389 "<built-in>" 3
# 1 "<command line>" 1
# 1 "<built-in>" 2
# 1 "<stdin>" 2

@Prabhuk
Copy link
Contributor Author

Prabhuk commented Dec 20, 2024

@RossComputerGuy FYI

@Prabhuk Prabhuk requested a review from jhuber6 December 20, 2024 18:36
@Prabhuk
Copy link
Contributor Author

Prabhuk commented Dec 20, 2024

@RossComputerGuy -- I am planning to land this today. Would you like to take a look and let me know if you are ok with these clean ups? I am still not able to add you as a reviewer. Maybe something to do with getting an approval? I'll check the process docs.

Copy link
Member

@RossComputerGuy RossComputerGuy left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@Prabhuk Prabhuk merged commit dce2245 into llvm:main Dec 22, 2024
14 checks passed
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.

5 participants