Author: Matt Arsenault Date: 2026-06-12T11:22:36+02:00 New Revision: 951c67d4dbed4f87f366f1f911bc061bc6da1912
URL: https://github.com/llvm/llvm-project/commit/951c67d4dbed4f87f366f1f911bc061bc6da1912 DIFF: https://github.com/llvm/llvm-project/commit/951c67d4dbed4f87f366f1f911bc061bc6da1912.diff LOG: clang/AMDGPU: Use TranslateArgs from the base toolchain instead of the host (#198627) This fixes -Xopenmp-target / -Xarch for arbitrary arguments. HIP and OpenMP had cargo-cult broken implementations of TranslateArgs, which called the host toolchain's implementation, and then special case transferred either -march or -mcpu to the device argument list. The respective device forwarding flags should work for any argument, not just this one. The main feature that needs to be preserved is the shared filtering of unsupported sanitizers to degrade them into warnings. Most of the changes here are dealing with fallout observed when the host target is darwin. The darwin toolchain happens to have some hacky statefulness tracking the compile target version, which gets written and rewritten on argument parsing. To maintain this hack, there are a few unused calls to getArgsForToolChain; start passing OFK_Host to these so the offload toolchains don't get confused and think they're in a non-offload context. Added: clang/test/Driver/offload-darwin-host.hip clang/test/Driver/openmp-Xopenmp-target-forward-args.c Modified: clang/lib/Driver/Driver.cpp clang/lib/Driver/ToolChain.cpp clang/lib/Driver/ToolChains/AMDGPU.cpp clang/lib/Driver/ToolChains/AMDGPU.h clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp clang/lib/Driver/ToolChains/AMDGPUOpenMP.h clang/lib/Driver/ToolChains/Gnu.cpp clang/lib/Driver/ToolChains/HIPAMD.cpp clang/lib/Driver/ToolChains/HIPAMD.h Removed: ################################################################################ diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 5cbf98fc074b6..165a1e2a6c29f 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -2734,7 +2734,7 @@ bool Driver::HandleImmediateArgs(Compilation &C) { // FIXME: Remove when darwin's toolchain is initialized during construction. // FIXME: For some more esoteric targets the default toolchain is not the // correct one. - C.getArgsForToolChain(&TC, Triple.getArchName(), Action::OFK_None); + C.getArgsForToolChain(&TC, Triple.getArchName(), Action::OFK_Host); RegisterEffectiveTriple TripleRAII(TC, Triple); switch (RLT) { case ToolChain::RLT_CompilerRT: @@ -6060,7 +6060,7 @@ InputInfoList Driver::BuildJobsForActionNoCache( // computed. Get the default arguments for OFK_None to ensure that // initialization is performed before processing the offload action. // FIXME: Remove when darwin's toolchain is initialized during construction. - C.getArgsForToolChain(TC, BoundArch, Action::OFK_None); + C.getArgsForToolChain(TC, BoundArch, Action::OFK_Host); // The offload action is expected to be used in four diff erent situations. // diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp index fc4bcfc2fe865..8db726cedfe90 100644 --- a/clang/lib/Driver/ToolChain.cpp +++ b/clang/lib/Driver/ToolChain.cpp @@ -520,11 +520,20 @@ SanitizerArgs ToolChain::getSanitizerArgs(const llvm::opt::ArgList &JobArgs, StringRef BoundArch, Action::OffloadKind DeviceOffloadKind) const { + // When -fno-gpu-sanitize is specified for GPU targets, don't emit + // diagnostics about unsupported sanitizers for specific GPU arches, + // since sanitizers are disabled for the GPU anyway. + bool DiagnoseBoundArchErrors = + BoundArchSanitizerArgsChecked.insert(BoundArch).second; + if (!BoundArch.empty() && getTriple().isGPU() && + !JobArgs.hasFlag(options::OPT_fgpu_sanitize, + options::OPT_fno_gpu_sanitize, true)) { + DiagnoseBoundArchErrors = false; + } + SanitizerArgs SanArgs(*this, JobArgs, /*DiagnoseErrors=*/!SanitizerArgsChecked, - /*DiagnoseBoundArchErrors=*/ - BoundArchSanitizerArgsChecked.insert(BoundArch).second, - BoundArch, DeviceOffloadKind); + DiagnoseBoundArchErrors, BoundArch, DeviceOffloadKind); SanitizerArgsChecked = true; return SanArgs; diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp index b51ad4e66edb7..b0421f795bf41 100644 --- a/clang/lib/Driver/ToolChains/AMDGPU.cpp +++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp @@ -714,23 +714,24 @@ Tool *AMDGPUToolChain::buildLinker() const { DerivedArgList * AMDGPUToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch, Action::OffloadKind DeviceOffloadKind) const { - DerivedArgList *DAL = Generic_ELF::TranslateArgs(Args, BoundArch, DeviceOffloadKind); - - const OptTable &Opts = getDriver().getOpts(); - - if (!DAL) + if (!DAL) { DAL = new DerivedArgList(Args.getBaseArgs()); + for (Arg *A : Args) + DAL->append(A); + } - for (Arg *A : Args) - DAL->append(A); + const OptTable &Opts = getDriver().getOpts(); - // AMDGPU is intended to use `-mcpu` but we accept `-march` for legacy. - if (Arg *A = DAL->getLastArg(options::OPT_march_EQ)) { - DAL->eraseArg(options::OPT_march_EQ); - if (!DAL->hasArg(options::OPT_mcpu_EQ)) - DAL->AddJoinedArg(A, Opts.getOption(options::OPT_mcpu_EQ), A->getValue()); + if (DeviceOffloadKind == Action::OFK_None) { + // AMDGPU is intended to use `-mcpu` but we accept `-march` for legacy. + if (Arg *A = DAL->getLastArg(options::OPT_march_EQ)) { + DAL->eraseArg(options::OPT_march_EQ); + if (!DAL->hasArg(options::OPT_mcpu_EQ)) + DAL->AddJoinedArg(A, Opts.getOption(options::OPT_mcpu_EQ), + A->getValue()); + } } // Replace -mcpu=native with detected GPU. @@ -751,7 +752,13 @@ AMDGPUToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch, } } - checkTargetID(*DAL); + if (!BoundArch.empty()) { + DAL->eraseArg(options::OPT_mcpu_EQ); + DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_mcpu_EQ), BoundArch); + checkTargetID(*DAL); + } else if (DeviceOffloadKind == Action::OFK_None) { + checkTargetID(*DAL); + } if (Args.getLastArgValue(options::OPT_x) != "cl") return DAL; @@ -838,6 +845,29 @@ ROCMToolChain::ROCMToolChain(const Driver &D, const llvm::Triple &Triple, RocmInstallation->detectDeviceLibrary(); } +DerivedArgList * +ROCMToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch, + Action::OffloadKind DeviceOffloadKind) const { + DerivedArgList *DAL = + AMDGPUToolChain::TranslateArgs(Args, BoundArch, DeviceOffloadKind); + + // Filter out sanitizer coverage options that are not supported for AMDGPU. + for (Arg *A : Args) { + // Sanitizer coverage is currently not supported for AMDGPU. + if (A->getOption().matches(options::OPT_fsan_cov_Group)) { + // Upgrade to error if the option was explicitly specified for device + bool IsExplicitDevice = + A->getBaseArg().getOption().matches(options::OPT_Xarch_device); + getDriver().Diag(IsExplicitDevice + ? diag::err_drv_unsupported_option_for_target + : diag::warn_drv_unsupported_option_for_target) + << A->getAsString(Args) << getTriple().str(); + } + } + + return DAL; +} + void AMDGPUToolChain::addClangTargetOptions( const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args, llvm::StringRef BoundArch, Action::OffloadKind DeviceOffloadingKind) const { diff --git a/clang/lib/Driver/ToolChains/AMDGPU.h b/clang/lib/Driver/ToolChains/AMDGPU.h index 2c3465a162dfc..66b269ca8aa05 100644 --- a/clang/lib/Driver/ToolChains/AMDGPU.h +++ b/clang/lib/Driver/ToolChains/AMDGPU.h @@ -141,6 +141,11 @@ class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain { public: ROCMToolChain(const Driver &D, const llvm::Triple &Triple, const llvm::opt::ArgList &Args); + + llvm::opt::DerivedArgList * + TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch, + Action::OffloadKind DeviceOffloadKind) const override; + void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args, @@ -152,31 +157,6 @@ class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain { getCommonDeviceLibNames(const llvm::opt::ArgList &DriverArgs, llvm::StringRef TargetID, llvm::StringRef GPUArch, Action::OffloadKind DeviceOffloadingKind) const; - - bool diagnoseUnsupportedOption(const llvm::opt::Arg *A, - const llvm::opt::DerivedArgList &DAL, - const llvm::opt::ArgList &DriverArgs, - const char *Value = nullptr) const { - auto &Diags = getDriver().getDiags(); - bool IsExplicitDevice = - A->getBaseArg().getOption().matches(options::OPT_Xarch_device); - - if (Value) { - unsigned DiagID = - IsExplicitDevice - ? clang::diag::err_drv_unsupported_option_part_for_target - : clang::diag::warn_drv_unsupported_option_part_for_target; - Diags.Report(DiagID) << Value << A->getAsString(DriverArgs) - << getTriple().str(); - } else { - unsigned DiagID = - IsExplicitDevice - ? clang::diag::err_drv_unsupported_option_for_target - : clang::diag::warn_drv_unsupported_option_for_target; - Diags.Report(DiagID) << A->getAsString(DAL) << getTriple().str(); - } - return true; - } }; } // end namespace toolchains diff --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp index e7a169a374464..9a90e491bb27d 100644 --- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp +++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp @@ -33,9 +33,6 @@ AMDGPUOpenMPToolChain::AMDGPUOpenMPToolChain(const Driver &D, void AMDGPUOpenMPToolChain::addClangTargetOptions( const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args, llvm::StringRef BoundArch, Action::OffloadKind DeviceOffloadingKind) const { - HostTC.addClangTargetOptions(DriverArgs, CC1Args, BoundArch, - DeviceOffloadingKind); - assert(DeviceOffloadingKind == Action::OFK_OpenMP && "Only OpenMP offloading kinds are supported."); @@ -55,46 +52,6 @@ void AMDGPUOpenMPToolChain::addClangTargetOptions( return; } -llvm::opt::DerivedArgList *AMDGPUOpenMPToolChain::TranslateArgs( - const llvm::opt::DerivedArgList &Args, StringRef BoundArch, - Action::OffloadKind DeviceOffloadKind) const { - DerivedArgList *DAL = - HostTC.TranslateArgs(Args, BoundArch, DeviceOffloadKind); - - if (!DAL) - DAL = new DerivedArgList(Args.getBaseArgs()); - - const OptTable &Opts = getDriver().getOpts(); - - for (Arg *A : Args) { - // Sanitizer coverage is currently not supported for AMDGPU. - if (A->getOption().matches(options::OPT_fsan_cov_Group)) { - diagnoseUnsupportedOption(A, *DAL, Args); - continue; - } - - if (A->getOption().matches(options::OPT_fsanitize_EQ) && - !Args.hasFlag(options::OPT_fgpu_sanitize, options::OPT_fno_gpu_sanitize, - true)) - continue; - - DAL->append(A); - } - - if (!BoundArch.empty()) { - DAL->eraseArg(options::OPT_mcpu_EQ); - DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_mcpu_EQ), BoundArch); - } - - return DAL; -} - -void AMDGPUOpenMPToolChain::addClangWarningOptions( - ArgStringList &CC1Args) const { - AMDGPUToolChain::addClangWarningOptions(CC1Args); - HostTC.addClangWarningOptions(CC1Args); -} - ToolChain::CXXStdlibType AMDGPUOpenMPToolChain::GetCXXStdlibType(const ArgList &Args) const { return HostTC.GetCXXStdlibType(Args); diff --git a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.h b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.h index 7e212f15a9ebc..bc8b195383c6f 100644 --- a/clang/lib/Driver/ToolChains/AMDGPUOpenMP.h +++ b/clang/lib/Driver/ToolChains/AMDGPUOpenMP.h @@ -33,16 +33,11 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUOpenMPToolChain final return &HostTC.getTriple(); } - llvm::opt::DerivedArgList * - TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch, - Action::OffloadKind DeviceOffloadKind) const override; - void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args, llvm::StringRef BoundArch, Action::OffloadKind DeviceOffloadKind) const override; - void addClangWarningOptions(llvm::opt::ArgStringList &CC1Args) const override; CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList &Args) const override; void AddClangCXXStdlibIncludeArgs( const llvm::opt::ArgList &Args, diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp index c489a1bddcb26..0bb51de630c99 100644 --- a/clang/lib/Driver/ToolChains/Gnu.cpp +++ b/clang/lib/Driver/ToolChains/Gnu.cpp @@ -3457,8 +3457,8 @@ llvm::opt::DerivedArgList * Generic_GCC::TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch, Action::OffloadKind DeviceOffloadKind) const { - if (DeviceOffloadKind != Action::OFK_SYCL && - DeviceOffloadKind != Action::OFK_OpenMP) + if (DeviceOffloadKind == Action::OFK_None || + DeviceOffloadKind == Action::OFK_Host) return nullptr; DerivedArgList *DAL = new DerivedArgList(Args.getBaseArgs()); @@ -3484,13 +3484,14 @@ Generic_GCC::TranslateArgs(const llvm::opt::DerivedArgList &Args, // Add the bound architecture to the arguments list if present. if (!BoundArch.empty()) { - options::ID Opt = - getTriple().isARM() || getTriple().isPPC() || getTriple().isAArch64() - ? options::OPT_mcpu_EQ - : options::OPT_march_EQ; + options::ID Opt = getTriple().isARM() || getTriple().isPPC() || + getTriple().isAArch64() || getTriple().isAMDGPU() + ? options::OPT_mcpu_EQ + : options::OPT_march_EQ; DAL->eraseArg(Opt); DAL->AddJoinedArg(nullptr, Opts.getOption(Opt), BoundArch); } + return DAL; } diff --git a/clang/lib/Driver/ToolChains/HIPAMD.cpp b/clang/lib/Driver/ToolChains/HIPAMD.cpp index 4adb84f353d25..01cb23d0aa230 100644 --- a/clang/lib/Driver/ToolChains/HIPAMD.cpp +++ b/clang/lib/Driver/ToolChains/HIPAMD.cpp @@ -235,9 +235,6 @@ HIPAMDToolChain::HIPAMDToolChain(const Driver &D, const llvm::Triple &Triple, void HIPAMDToolChain::addClangTargetOptions( const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args, llvm::StringRef BoundArch, Action::OffloadKind DeviceOffloadingKind) const { - HostTC.addClangTargetOptions(DriverArgs, CC1Args, BoundArch, - DeviceOffloadingKind); - assert(DeviceOffloadingKind == Action::OFK_HIP && "Only HIP offloading kinds are supported for GPUs."); @@ -294,37 +291,14 @@ llvm::opt::DerivedArgList * HIPAMDToolChain::TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch, Action::OffloadKind DeviceOffloadKind) const { - DerivedArgList *DAL = - HostTC.TranslateArgs(Args, BoundArch, DeviceOffloadKind); - if (!DAL) - DAL = new DerivedArgList(Args.getBaseArgs()); - - const OptTable &Opts = getDriver().getOpts(); - - for (Arg *A : Args) { - // Sanitizer coverage is currently not supported for AMDGPU. - if (A->getOption().matches(options::OPT_fsan_cov_Group)) { - diagnoseUnsupportedOption(A, *DAL, Args); - continue; - } - - if (A->getOption().matches(options::OPT_fsanitize_EQ) && - !Args.hasFlag(options::OPT_fgpu_sanitize, options::OPT_fno_gpu_sanitize, - true)) - continue; - - DAL->append(A); - } + llvm::opt::DerivedArgList *DAL = + ROCMToolChain::TranslateArgs(Args, BoundArch, DeviceOffloadKind); - if (!BoundArch.empty()) { - DAL->eraseArg(options::OPT_mcpu_EQ); - DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_mcpu_EQ), BoundArch); - checkTargetID(*DAL); - } - - if (!Args.hasArg(options::OPT_flto_partitions_EQ)) + if (!Args.hasArg(options::OPT_flto_partitions_EQ)) { + const OptTable &Opts = getDriver().getOpts(); DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_flto_partitions_EQ), "8"); + } return DAL; } @@ -335,11 +309,6 @@ Tool *HIPAMDToolChain::buildLinker() const { return new tools::AMDGCN::Linker(*this); } -void HIPAMDToolChain::addClangWarningOptions(ArgStringList &CC1Args) const { - AMDGPUToolChain::addClangWarningOptions(CC1Args); - HostTC.addClangWarningOptions(CC1Args); -} - ToolChain::CXXStdlibType HIPAMDToolChain::GetCXXStdlibType(const ArgList &Args) const { return HostTC.GetCXXStdlibType(Args); diff --git a/clang/lib/Driver/ToolChains/HIPAMD.h b/clang/lib/Driver/ToolChains/HIPAMD.h index 8277119bf9348..628124f4bc71c 100644 --- a/clang/lib/Driver/ToolChains/HIPAMD.h +++ b/clang/lib/Driver/ToolChains/HIPAMD.h @@ -70,7 +70,6 @@ class LLVM_LIBRARY_VISIBILITY HIPAMDToolChain final : public ROCMToolChain { llvm::opt::ArgStringList &CC1Args, llvm::StringRef BoundArch, Action::OffloadKind DeviceOffloadKind) const override; - void addClangWarningOptions(llvm::opt::ArgStringList &CC1Args) const override; CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList &Args) const override; void AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs, diff --git a/clang/test/Driver/offload-darwin-host.hip b/clang/test/Driver/offload-darwin-host.hip new file mode 100644 index 0000000000000..3a3ab7267bd70 --- /dev/null +++ b/clang/test/Driver/offload-darwin-host.hip @@ -0,0 +1,8 @@ +// Make sure the darwin host toolchain does not assert with offload +// languages. The darwin toolchain has a platform initialization hack which was +// broken by offload toolchains trying to perform extra host argument handling. + +// RUN: %clang -### --target=arm64-apple-darwin24.6.0 -mmacos-version-min=10.9 --offload-arch=gfx900 -nogpulib -nogpuinc %s 2>&1 | FileCheck %s + +// CHECK: "-cc1" "-triple" "amdgcn-amd-amdhsa" "-aux-triple" "arm64-apple-darwin24.6.0" +// CHECK: "-cc1" "-triple" "arm64-apple-macosx10.9.0" "-aux-triple" "amdgcn-amd-amdhsa" diff --git a/clang/test/Driver/openmp-Xopenmp-target-forward-args.c b/clang/test/Driver/openmp-Xopenmp-target-forward-args.c new file mode 100644 index 0000000000000..5749df973ed78 --- /dev/null +++ b/clang/test/Driver/openmp-Xopenmp-target-forward-args.c @@ -0,0 +1,10 @@ +// Check that -Xopenmp-target forwards arbitrary arguments, not just -march + +// RUN: %clang -### --target=x86_64-pc-linux -no-canonical-prefixes -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx900 -Xopenmp-target=amdgcn-amd-amdhsa -ffinite-math-only -nogpulib %s 2>&1 \ +// RUN: | FileCheck %s + +// Flag should only apply to device, not the host. + +// CHECK-NOT: -ffinite-math-only +// CHECK: "-triple" "amdgcn-amd-amdhsa" {{.*}} "-ffinite-math-only" +// CHECK-NOT: -ffinite-math-only _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
