[Basic] Support 64-bit x86 target for UEFI

[Prabhuk]

Summary

Adding support for X86_64 UEFI target to begin with.

[Prabhuk]

Update commit message.

[phosek]

This needs a lit test.

clang/lib/Driver/ToolChains/UEFI.h

55–56

Nit: Can you leave an empty line here for consistency?

[phosek]

clang/lib/Driver/ToolChains/UEFI.cpp

93–98

This would ideally be handled by ToolChain::GetLinkerPath, can you leave a comment here along those lines?

98

This could be simplified as suggested, also the name of the variable should be LinkerPath.

103

The Environment variable is never modified so this condition will never hold.

clang/lib/Driver/ToolChains/UEFI.h

11–12

Nit: Can you add an empty line here?

[Prabhuk]

Addressed comments from last reviews except for adding lit tests.

[Prabhuk]

Adding a simple lit test.

[Prabhuk]

Set the Objectformat in the appropriate place to fix the triple string for UEFI.
Simplify the lit test for UEFI toolchain to C.

[Prabhuk]

Improve lit test for uefi.

[Prabhuk]

Clean up lit test.

[phosek]

clang/lib/Driver/ToolChains/UEFI.cpp

58

We should be also passing -subsystem:efi_application. Note that COFF also has other types of EFI binaries: efi_boot_service_driver, efi_rom, efi_runtime_driver so we may eventually need a way to change the subsystem, but starting with -subsystem:efi_application is probably fine for now.

58

We also need -entry:<entry_function>. Note that the specification doesn't say what the name of the <entry_function> should be. TianoCore EDK II which is the reference UEFI implementation from Intel uses EfiMain so that might the a reasonable choice, but definitely warrants a comment and in the future we'll probably need a flag to override it.

103

This is still not addressed.

clang/lib/Driver/ToolChains/UEFI.h

61

Nit: Can you add an empty line here?

clang/test/Driver/uefi.c

9

This test should cover all defaults and explicit options passed to cc1 and the linker. If I run this command, I get:

"/usr/local/google/home/phosek/llvm/llvm-project/build/fuchsia/bin/llvm" "clang" "-cc1" "-triple" "x86_64-unknown-uefi" "-emit-obj" "-mrelax-all" "-dumpdir" "a-" "-disable-free" "-clear-ast-before-backend" "-main-file-name" "uefi.c" "-mrelocation-model" "pic" "-pic-level" "2" "-mframe-pointer=all" "-fmath-errno" "-ffp-contract=on" "-fno-rounding-math" "-mconstructor-aliases" "-target-cpu" "x86-64" "-tune-cpu" "generic" "-debugger-tuning=gdb" "-fcoverage-compilation-dir=/usr/local/google/home/phosek/llvm/llvm-project/build/fuchsia" "-resource-dir" "/usr/local/google/home/phosek/llvm/llvm-project/build/fuchsia/lib/clang/17" "-isysroot" "/" "-fdebug-compilation-dir=/usr/local/google/home/phosek/llvm/llvm-project/build/fuchsia" "-ferror-limit" "19" "-fgnuc-version=4.2.1" "-fcolor-diagnostics" "-faddrsig" "-o" "/tmp/uefi-b7956f.o" "-x" "c" "../../clang/test/Driver/uefi.c"
"/usr/local/google/home/phosek/llvm/llvm-project/build/fuchsia/./bin/lld-link" "-out:a.out" "-nologo" "/tmp/uefi-b7956f.o"

We should definitely check options like -mrelocation-model, -mframe-pointer, etc. This also raises some questions, like for example should we default to -debugger-tuning=gdb for the UEFI target?

10–11

I don't think this is needed.

11

Can you ensure there's a newline at the end?

[phosek]

For comments that are already address, it's customary to mark those as "Done" to make it clear.

[Prabhuk]

Added subsystem and entry args. Improved driver test.

[Prabhuk]

Minor formatting changes.

[brad]

clang/lib/Driver/ToolChains/UEFI.h

46

You can remove the IsIntegratedAssemblerDefault line.

49

This changed to UnwindTableLevel.

https://github.com/llvm/llvm-project/commit/4388b56d525c08ce3cf941cfbad2428b0e1695b0

[Prabhuk]

Addressed review comments.
Added "-dll" and "-tsaware:no" linker flags for UEFI.

[Prabhuk]

Update X86 backend to use Microsoft ABI for UEFI target.

[Prabhuk]

Minor cleanup.

[phosek]

Would it be possible to split up the LLVM changes into another change so we can land it separately from the Clang support?

llvm/lib/Target/X86/X86Subtarget.h

337 ↗

(On Diff #557154)

It's a bit strange for this method to return true even for UEFI which is not Windows. I think it'd be clear to introduce a new method isTargetUEFI() and modify relevant callsites to use isTargetWin64() || isTargetUEFI(). Ideally that would be done as a separate patch.

[Prabhuk]

Splitting commit into 3 commits.

[Prabhuk]

Clang Driver Changes - https://reviews.llvm.org/D159541
Current - https://reviews.llvm.org/D152206
X86 Backend changes - https://reviews.llvm.org/D159540

[phosek]

Is it possible to abandon this change?

[Prabhuk]

Datalayout update.

[Prabhuk]

Update patch.

[Prabhuk]

@phosek -- Regarding the abandoning commit. I had messed up the commit splitting process earlier. Uploaded the correct set of files for this change now. Please take a look.

[phosek]

LGTM

[MaskRay]

DataLayout needs a unittest in llvm/unittests/IR/DataLayoutTest.cpp

[Prabhuk]

Add DataLayoutTest for UEFI target.

[MaskRay]

llvm/unittests/IR/DataLayoutTest.cpp

108 ↗

(On Diff #557261)

UEFI may be a better name in case we test more properties in the future.

[Prabhuk]

Updated the test name and added comment.

[mbrkusanin]

llvm/unittests/IR/DataLayoutTest.cpp

109 ↗

(On Diff #557430)

This is giving me a linker error:

$ ninja unittests/IR/IRTests
....
/usr/bin/ld: unittests/IR/CMakeFiles/IRTests.dir/DataLayoutTest.cpp.o: undefined reference to symbol '_ZN4llvm6TripleC1ERKNS_5TwineE'
/usr/bin/ld: ....../build/lib/libLLVMTargetParser.so.18git: error adding symbols: DSO missing from command line

Adding TargetParser to link componenets in llvm/unittests/IR/CMakeLists.txt seems to fix it for me, but can you check please?

[Prabhuk]

llvm/unittests/IR/DataLayoutTest.cpp

109 ↗

(On Diff #557430)

Thanks for bringing this to my attention. I am able to build and run the IR tests from top of the tree without any failures. Let me take a closer look on how to reproduce this.

[mbrkusanin]

llvm/unittests/IR/DataLayoutTest.cpp

109 ↗

(On Diff #557430)

It seems only few buildbots fail with this same error: https://lab.llvm.org/buildbot/#/builders/57/builds/30211
Not sure what is common between me and these buildbots.

But I get the same error with lld as well:

ld.lld: error: undefined symbol: llvm::Triple::Triple(llvm::Twine const&)
/>>> referenced by DataLayoutTest.cpp
/>>> unittests/IR/CMakeFiles/IRTests.dir/DataLayoutTest.cpp.o:((anonymous namespace)::DataLayoutTest_UEFI_Test::TestBody())

[kosarev]

@Prabhuk This reproduces with my setup as well. Did you try building with -DBUILD_SHARED_LIBS=ON?

[mbrkusanin]

If there are others experiencing the same error then I can push my fix: https://github.com/llvm/llvm-project/pull/67696 if there are no other suggestions.

[Prabhuk]

llvm/unittests/IR/DataLayoutTest.cpp

109 ↗

(On Diff #557430)

Thank you. I just reverted the change. I will reland this with an appropriate fix to the link failure.

[Prabhuk]

Relanding the change.

[Prabhuk]

Relanding the change as the build dependency issue related to the
original patch is fixed by this commit:
c43aa466b8980e620fd27404ca2ba60210b76111

clang/lib/Basic/Targets.cpp

case llvm::Triple::Fuchsia:
       return std::make_unique<FuchsiaTargetInfo<X86_64TargetInfo>>(Triple,
                                                                    Opts);
     case llvm::Triple::KFreeBSD:
       return std::make_unique<KFreeBSDTargetInfo<X86_64TargetInfo>>(Triple,
                                                                     Opts);
     case llvm::Triple::Solaris:
       return std::make_unique<SolarisTargetInfo<X86_64TargetInfo>>(Triple,
                                                                    Opts);
+    case llvm::Triple::UEFI:
+      return std::make_unique<UEFIX86_64TargetInfo>(Triple, Opts);
+
     case llvm::Triple::Win32: {
       switch (Triple.getEnvironment()) {
       case llvm::Triple::Cygnus:
         return std::make_unique<CygwinX86_64TargetInfo>(Triple, Opts);
       case llvm::Triple::GNU:
         return std::make_unique<MinGWX86_64TargetInfo>(Triple, Opts);
       case llvm::Triple::MSVC:
       default: // Assume MSVC for unknown environments

clang/lib/Basic/Targets/OSTargets.h

ZOSTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
     this->ZeroLengthBitfieldBoundary = 32;
   }
 
   bool areDefaultedSMFStillPOD(const LangOptions &) const override {
     return false;
   }
 };
 
+// UEFI target
+template <typename Target>
+class LLVM_LIBRARY_VISIBILITY UEFITargetInfo : public OSTargetInfo<Target> {
+protected:
+  void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple,
+                    MacroBuilder &Builder) const override {}
+
+public:
+  UEFITargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
+      : OSTargetInfo<Target>(Triple, Opts) {
+    this->WCharType = TargetInfo::UnsignedShort;
+    this->WIntType = TargetInfo::UnsignedShort;
+  }
+};
+
 void addWindowsDefines(const llvm::Triple &Triple, const LangOptions &Opts,
                        MacroBuilder &Builder);
 
 // Windows target
 template <typename Target>
 class LLVM_LIBRARY_VISIBILITY WindowsTargetInfo : public OSTargetInfo<Target> {
 protected:
   void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple,

clang/lib/Basic/Targets/X86.h

public:
   ArrayRef<Builtin::Info> getTargetBuiltins() const override;
 
   bool hasBitIntType() const override { return true; }
   size_t getMaxBitIntWidth() const override {
     return llvm::IntegerType::MAX_INT_BITS;
   }
 };
 
+// x86-64 UEFI target
+class LLVM_LIBRARY_VISIBILITY UEFIX86_64TargetInfo
+    : public UEFITargetInfo<X86_64TargetInfo> {
+public:
+  UEFIX86_64TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
+      : UEFITargetInfo<X86_64TargetInfo>(Triple, Opts) {
+    LongWidth = LongAlign = 32;
+    DoubleAlign = LongLongAlign = 64;
+    IntMaxType = SignedLongLong;
+    Int64Type = SignedLongLong;
+    SizeType = UnsignedLongLong;
+    PtrDiffType = SignedLongLong;
+    IntPtrType = SignedLongLong;
+    LongDoubleWidth = LongDoubleAlign = 64;
+    LongDoubleFormat = &llvm::APFloat::IEEEdouble();
+  }
+
+  void getTargetDefines(const LangOptions &Opts,
+                        MacroBuilder &Builder) const override {
+    getOSDefines(Opts, X86TargetInfo::getTriple(), Builder);
+  }
+
+  BuiltinVaListKind getBuiltinVaListKind() const override {
+    return TargetInfo::CharPtrBuiltinVaList;
+  }
+
+  CallingConvCheckResult checkCallingConvention(CallingConv CC) const override {
+    switch (CC) {
+    case CC_C:
+    case CC_Win64:
+      return CCCR_OK;
+    default:
+      return CCCR_Warning;
+    }
+  }
+
+  TargetInfo::CallingConvKind
+  getCallingConvKind(bool ClangABICompat4) const override {
+    return CCK_MicrosoftWin64;
+  }
+};
+
 // x86-64 Windows target
 class LLVM_LIBRARY_VISIBILITY WindowsX86_64TargetInfo
     : public WindowsTargetInfo<X86_64TargetInfo> {
 public:
   WindowsX86_64TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
       : WindowsTargetInfo<X86_64TargetInfo>(Triple, Opts) {
     LongWidth = LongAlign = 32;
     DoubleAlign = LongLongAlign = 64;

clang/lib/Driver/CMakeLists.txt

add_clang_library(clangDriver
   ToolChains/NetBSD.cpp
   ToolChains/OHOS.cpp
   ToolChains/OpenBSD.cpp
   ToolChains/PS4CPU.cpp
   ToolChains/RISCVToolchain.cpp
   ToolChains/Solaris.cpp
   ToolChains/SPIRV.cpp
   ToolChains/TCE.cpp
+  ToolChains/UEFI.cpp
   ToolChains/VEToolchain.cpp
   ToolChains/WebAssembly.cpp
   ToolChains/XCore.cpp
   ToolChains/PPCLinux.cpp
   ToolChains/PPCFreeBSD.cpp
   ToolChains/InterfaceStubs.cpp
   ToolChains/ZOS.cpp
   Types.cpp

clang/lib/Driver/Driver.cpp

 #include "ToolChains/OpenBSD.h"
 #include "ToolChains/PPCFreeBSD.h"
 #include "ToolChains/PPCLinux.h"
 #include "ToolChains/PS4CPU.h"
 #include "ToolChains/RISCVToolchain.h"
 #include "ToolChains/SPIRV.h"
 #include "ToolChains/Solaris.h"
 #include "ToolChains/TCE.h"
+#include "ToolChains/UEFI.h"
 #include "ToolChains/VEToolchain.h"
 #include "ToolChains/WebAssembly.h"
 #include "ToolChains/XCore.h"
 #include "ToolChains/ZOS.h"
 #include "clang/Basic/TargetID.h"
 #include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Action.h"
case llvm::Triple::CUDA:
       break;
     case llvm::Triple::AMDHSA:
       TC = std::make_unique<toolchains::ROCMToolChain>(*this, Target, Args);
       break;
     case llvm::Triple::AMDPAL:
     case llvm::Triple::Mesa3D:
       TC = std::make_unique<toolchains::AMDGPUToolChain>(*this, Target, Args);
       break;
+    case llvm::Triple::UEFI:
+      TC = std::make_unique<toolchains::UEFI>(*this, Target, Args);
+      break;
     case llvm::Triple::Win32:
       switch (Target.getEnvironment()) {
       default:
         if (Target.isOSBinFormatELF())
           TC = std::make_unique<toolchains::Generic_ELF>(*this, Target, Args);
         else if (Target.isOSBinFormatMachO())
           TC = std::make_unique<toolchains::MachO>(*this, Target, Args);
         else

clang/lib/Driver/ToolChains/UEFI.h

+//===--- UEFI.h - UEFI ToolChain Implementations ----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_UEFI_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_UEFI_H
+
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+
+namespace clang {
+namespace driver {
+namespace tools {
+namespace uefi {
+class LLVM_LIBRARY_VISIBILITY Linker : public Tool {
+public:
+  Linker(const ToolChain &TC) : Tool("uefi::Linker", "lld-link", TC) {}
+
+  bool hasIntegratedCPP() const override { return false; }
+  bool isLinkJob() const override { return true; }
+
+  void ConstructJob(Compilation &C, const JobAction &JA,
+                    const InputInfo &Output, const InputInfoList &Inputs,
+                    const llvm::opt::ArgList &TCArgs,
+                    const char *LinkingOutput) const override;
+};
+} // end namespace uefi
+} // end namespace tools
+
+namespace toolchains {
+
+class LLVM_LIBRARY_VISIBILITY UEFI : public ToolChain {
+public:
+  UEFI(const Driver &D, const llvm::Triple &Triple,
+       const llvm::opt::ArgList &Args);
+
+protected:
+  Tool *buildLinker() const override;
+
+public:
+  bool HasNativeLLVMSupport() const override { return true; }
+  bool IsIntegratedAssemblerDefault() const override { return true; }
+  bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const {
+    return true;
+  }
+  bool isPICDefault() const override { return true; }
+  bool isPIEDefault(const llvm::opt::ArgList &Args) const override {
+    return false;
+  }
+  bool isPICDefaultForced() const override { return true; }
+};
+
+} // namespace toolchains
+} // namespace driver
+} // namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_UEFI_H

clang/lib/Driver/ToolChains/UEFI.cpp

+//===--- UEFI.h - UEFI ToolChain Implementations ----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "UEFI.h"
+#include "CommonArgs.h"
+#include "Darwin.h"
+#include "clang/Basic/CharInfo.h"
+#include "clang/Basic/Version.h"
+#include "clang/Config/config.h"
+#include "clang/Driver/Compilation.h"
+#include "clang/Driver/Driver.h"
+#include "clang/Driver/Options.h"
+#include "clang/Driver/SanitizerArgs.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/Option/Arg.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
+#include "llvm/TargetParser/Host.h"
+
+using namespace clang::driver;
+using namespace clang::driver::toolchains;
+using namespace clang;
+using namespace llvm::opt;
+
+UEFI::UEFI(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
+    : ToolChain(D, Triple, Args) {
+  getProgramPaths().push_back(getDriver().getInstalledDir());
+  if (getDriver().getInstalledDir() != getDriver().Dir)
+    getProgramPaths().push_back(getDriver().Dir);
+}
+
+Tool *UEFI::buildLinker() const { return new tools::uefi::Linker(*this); }
+
+void tools::uefi::Linker::ConstructJob(Compilation &C, const JobAction &JA,
+                                       const InputInfo &Output,
+                                       const InputInfoList &Inputs,
+                                       const ArgList &Args,
+                                       const char *LinkingOutput) const {
+  ArgStringList CmdArgs;
+
+  auto &TC = static_cast<const toolchains::UEFI &>(getToolChain());
+
+  assert((Output.isFilename() || Output.isNothing()) && "invalid output");
+  if (Output.isFilename())
+    CmdArgs.push_back(
+        Args.MakeArgString(std::string("-out:") + Output.getFilename()));
+
+  CmdArgs.push_back("-nologo");
+
+  // Other UEFI binary subsystems that are currently unsupported:
+  // efi_boot_service_driver, efi_rom, efi_runtime_driver.
+  CmdArgs.push_back("-subsystem:efi_application");
+
+  // Default entry function name according to the TianaCore reference
+  // implementation is EfiMain. In the future we will add a flag to override the
+  // entry function name.
+  CmdArgs.push_back("-entry:EfiMain");
+
+  if (Args.hasArg(options::OPT_g_Group, options::OPT__SLASH_Z7))
+    CmdArgs.push_back("-debug");
+
+  Args.AddAllArgValues(CmdArgs, options::OPT__SLASH_link);
+
+  // Add filenames, libraries, and other linker inputs.
+  for (const auto &Input : Inputs) {
+    if (Input.isFilename()) {
+      CmdArgs.push_back(Input.getFilename());
+      continue;
+    }
+
+    const Arg &A = Input.getInputArg();
+    if (A.getOption().matches(options::OPT_l)) {
+      StringRef Lib = A.getValue();
+      const char *LinkLibArg;
+      if (Lib.endswith(".lib"))
+        LinkLibArg = Args.MakeArgString(Lib);
+      else
+        LinkLibArg = Args.MakeArgString(Lib + ".lib");
+      CmdArgs.push_back(LinkLibArg);
+      continue;
+    }
+
+    // Otherwise, this is some other kind of linker input option like -Wl, -z,
+    // or -L.
+    A.renderAsInput(Args, CmdArgs);
+  }
+
+  // This should ideally be handled by ToolChain::GetLinkerPath but we need
+  // to special case some linker paths. In the case of lld, we need to
+  // translate 'lld' into 'lld-link'.
+  StringRef Linker =
+      Args.getLastArgValue(options::OPT_fuse_ld_EQ, CLANG_DEFAULT_LINKER);
+  if (Linker.empty() || Linker.equals_insensitive("lld"))
+    Linker = "lld-link";
+
+  auto LinkerPath = TC.GetProgramPath(Linker.str().c_str());
+  auto LinkCmd = std::make_unique<Command>(
+      JA, *this, ResponseFileSupport::AtFileUTF16(),
+      Args.MakeArgString(LinkerPath), CmdArgs, Inputs, Output);
+  C.addCommand(std::move(LinkCmd));
+}

clang/test/Driver/uefi.c

+// RUN: %clang -### %s --target=x86_64-unknown-uefi \
+// RUN:     --sysroot=%S/platform -fuse-ld=lld 2>&1 \
+// RUN:     | FileCheck -check-prefixes=CHECK %s
+// RUN: %clang -### %s --target=x86_64-uefi \
+// RUN:     --sysroot=%S/platform -fuse-ld=lld 2>&1 \
+// RUN:     | FileCheck -check-prefixes=CHECK %s
+// CHECK: "-cc1"
+// CHECK-SAME: "-triple" "x86_64-unknown-uefi"
+// CHECK-SAME: "-mrelocation-model" "pic" "-pic-level" "2"
+// CHECK-SAME: "-mframe-pointer=all"

llvm/lib/TargetParser/Triple.cpp

static Triple::ObjectFormatType getDefaultFormat(const Triple &T) {
   case Triple::arm:
   case Triple::thumb:
   case Triple::x86:
   case Triple::x86_64:
     if (T.isOSDarwin())
       return Triple::MachO;
     else if (T.isOSWindows())
       return Triple::COFF;
+    else if (T.isUEFI())
+      return Triple::COFF;
     return Triple::ELF;
 
   case Triple::aarch64_be:
   case Triple::amdgcn:
   case Triple::amdil64:
   case Triple::amdil:
   case Triple::arc:
   case Triple::armeb:

llvm/unittests/TargetParser/TripleTest.cpp

TEST(TripleTest, ParsedIDs) {
   EXPECT_EQ(Triple::HermitCore, T.getOS());
   EXPECT_EQ(Triple::UnknownEnvironment, T.getEnvironment());
 
   T = Triple("x86_64-unknown-uefi");
   EXPECT_EQ(Triple::x86_64, T.getArch());
   EXPECT_EQ(Triple::UnknownVendor, T.getVendor());
   EXPECT_EQ(Triple::UEFI, T.getOS());
   EXPECT_EQ(Triple::UnknownEnvironment, T.getEnvironment());
+  EXPECT_EQ(Triple::COFF, T.getObjectFormat());
 
   T = Triple("wasm32-unknown-unknown");
   EXPECT_EQ(Triple::wasm32, T.getArch());
   EXPECT_EQ(Triple::UnknownVendor, T.getVendor());
   EXPECT_EQ(Triple::UnknownOS, T.getOS());
   EXPECT_EQ(Triple::UnknownEnvironment, T.getEnvironment());
 
   T = Triple("wasm32-unknown-wasi");