-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang][driver] Cleanup UEFI toolchain driver #111473
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
Conversation
✅ 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.
c241379
to
9b55509
Compare
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Prabhuk (Prabhuk) ChangesUpdating 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:
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()); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
@RossComputerGuy FYI |
@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. |
There was a problem hiding this 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.
Updating UEFI header includes to not include system include directories, adding includes from compiler resource directory and minor cleanups.