phosek added inline comments.
================ Comment at: clang/lib/Driver/ToolChains/UEFI.cpp:41-45 +bool UEFI::IsUnwindTablesDefault(const ArgList &Args) const { + // TODO(prabhukr):: What about x86_32? + return getArch() == llvm::Triple::x86_64 || getArch() == llvm::Triple::arm || + getArch() == llvm::Triple::thumb || getArch() == llvm::Triple::aarch64; +} ---------------- I'd hardcode and inline this for simplicity, this toolchain is only used for x86_64 and arm64 right now so now need to duplicate that here. ================ Comment at: clang/lib/Driver/ToolChains/UEFI.cpp:48-49 +bool UEFI::isPICDefault() const { + return getArch() == llvm::Triple::x86_64 || + getArch() == llvm::Triple::aarch64; +} ---------------- I'd hardcode and inline this for simplicity, this toolchain is only used for x86_64 and arm64 right now so now need to duplicate that here. ================ Comment at: clang/lib/Driver/ToolChains/UEFI.cpp:52 + +bool UEFI::isPIEDefault(const llvm::opt::ArgList &Args) const { return false; } + ---------------- I'd inline this. ================ Comment at: clang/lib/Driver/ToolChains/UEFI.cpp:55-56 +bool UEFI::isPICDefaultForced() const { + return getArch() == llvm::Triple::x86_64 || + getArch() == llvm::Triple::aarch64; +} ---------------- I'd hardcode and inline this for simplicity, this toolchain is only used for x86_64 and arm64 right now so now need to duplicate that here. ================ Comment at: clang/lib/Driver/ToolChains/UEFI.cpp:59 + +bool UEFI::IsIntegratedAssemblerDefault() const { return true; } ---------------- I'd inline this. ================ Comment at: clang/lib/Driver/ToolChains/UEFI.h:22 + const llvm::opt::ArgList &Args); + + bool IsIntegratedAssemblerDefault() const override; ---------------- You also need `HasNativeLLVMSupport`, see https://github.com/llvm/llvm-project/blob/46dee4a3a3dfb372a0eaa0b4490c80be2f421f29/clang/lib/Driver/ToolChains/Fuchsia.h#L43 ================ Comment at: clang/lib/Driver/ToolChains/UEFI.h:28 + bool isPICDefaultForced() const override; +}; +} // namespace toolchains ---------------- You'll also need to override `buildLinker` since the default implementation doesn't do anything, see https://github.com/llvm/llvm-project/blob/46dee4a3a3dfb372a0eaa0b4490c80be2f421f29/clang/lib/Driver/ToolChain.cpp#L319 I'd hardcode it to use `lld-link` for simplicty. You can use Fuchsia's implementation for inspiration (although the flags are going to be different since Fuchsia uses `ld.lld`, not `lld-link`): https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Fuchsia.h#L17-L34 https://github.com/llvm/llvm-project/blob/609b789170625277f631139c790c22d527ff1eed/clang/lib/Driver/ToolChains/Fuchsia.cpp#L31-L191 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131594/new/ https://reviews.llvm.org/D131594 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits