https://github.com/srividya-sundaram updated https://github.com/llvm/llvm-project/pull/232148
>From 8bf280443889135fd10afb41cb247a3856d25c5e Mon Sep 17 00:00:00 2001 From: srividya sundaram <[email protected]> Date: Wed, 14 Oct 2026 17:58:02 -0700 Subject: [PATCH 1/5] [Clang][Offload] Request canonical architectures in --should-extract The linker wrapper extracts device code from a static library only when a --should-extract value matches the image's arch label exactly. The label is the canonical architecture, but --should-extract was built from the raw --offload-arch values, so --offload-arch=gfx908:xnack+:sramecc- labeled the image gfx908:sramecc-:xnack+ yet requested gfx908:xnack+:sramecc-, and an architecture removed with --no-offload-arch was still requested. Take the values from getOffloadArchs, which canonicalizes and applies --no-offload-arch. Assisted-by: Claude Opus 5.5 --- clang/lib/Driver/ToolChains/Clang.cpp | 11 ++++++++--- clang/test/Driver/offload-should-extract.c | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) create mode 100644 clang/test/Driver/offload-should-extract.c diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index eb7bd3ad1ad5e2..a67f6a761d0e6b 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -9909,9 +9909,14 @@ void LinkerWrapper::ConstructJob(Compilation &C, const JobAction &JA, } // If the user explicitly requested it via `--offload-arch` we should - // extract it from any static libraries if present. - for (StringRef Arg : ToolChainArgs.getAllArgValues(OPT_offload_arch_EQ)) - CmdArgs.emplace_back(Args.MakeArgString("--should-extract=" + Arg)); + // extract it from any static libraries if present. Use the canonical + // name, which device images are labeled with. + if (ToolChainArgs.hasArg(OPT_offload_arch_EQ)) + for (const BoundArch &BA : + C.getDriver().getOffloadArchs(C, ToolChainArgs, Kind, *TC)) + if (!BA.ArchName.empty()) + CmdArgs.emplace_back( + Args.MakeArgString("--should-extract=" + BA.ArchName)); // If this is OpenMP the device linker will need `-lompdevice`. if (Kind == Action::OFK_OpenMP && !Args.hasArg(OPT_no_offloadlib) && diff --git a/clang/test/Driver/offload-should-extract.c b/clang/test/Driver/offload-should-extract.c new file mode 100644 index 00000000000000..f610466e9a562b --- /dev/null +++ b/clang/test/Driver/offload-should-extract.c @@ -0,0 +1,17 @@ +// The linker wrapper extracts device code from static libraries by the +// architecture its images are labeled with, so request the canonical name. + +// RUN: %clang -### --target=x86_64-unknown-linux-gnu -x hip -nogpulib -nogpuinc \ +// RUN: --offload-arch=gfx908:xnack+:sramecc- %s 2>&1 \ +// RUN: | FileCheck %s --check-prefix=CANONICAL +// CANONICAL: "--image=file={{.*}},triple={{[^,]*}},arch=gfx908:sramecc-:xnack+,kind=hip" +// CANONICAL: clang-linker-wrapper{{.*}} "--should-extract=gfx908:sramecc-:xnack+" + +/// An architecture removed with --no-offload-arch is not requested. +// RUN: %clang -### --target=x86_64-unknown-linux-gnu -x hip -nogpulib -nogpuinc \ +// RUN: --offload-arch=gfx906 --offload-arch=gfx90a --no-offload-arch=gfx906 %s 2>&1 \ +// RUN: | FileCheck %s --check-prefix=REMOVED +// REMOVED: clang-linker-wrapper +// REMOVED-NOT: "--should-extract=gfx906" +// REMOVED-SAME: "--should-extract=gfx90a" +// REMOVED-NOT: "--should-extract=gfx906" >From 46bad95e0f032998abef7a52d936ae1575f3edd1 Mon Sep 17 00:00:00 2001 From: srividya sundaram <[email protected]> Date: Tue, 20 Oct 2026 16:19:08 -0700 Subject: [PATCH 2/5] [Clang][Offload] Name the target in the --no-offload-arch test Without --offload-targets, the driver drops a removed architecture before the linker wrapper's job is built, so the check passed without the fix. Assisted-by: Claude Opus 5.5 --- clang/test/Driver/offload-should-extract.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/test/Driver/offload-should-extract.c b/clang/test/Driver/offload-should-extract.c index f610466e9a562b..0900fb8936ab21 100644 --- a/clang/test/Driver/offload-should-extract.c +++ b/clang/test/Driver/offload-should-extract.c @@ -7,9 +7,11 @@ // CANONICAL: "--image=file={{.*}},triple={{[^,]*}},arch=gfx908:sramecc-:xnack+,kind=hip" // CANONICAL: clang-linker-wrapper{{.*}} "--should-extract=gfx908:sramecc-:xnack+" -/// An architecture removed with --no-offload-arch is not requested. +/// An architecture removed with --no-offload-arch is not requested. Name the +/// target, so that every option reaches the linker wrapper's toolchain. // RUN: %clang -### --target=x86_64-unknown-linux-gnu -x hip -nogpulib -nogpuinc \ -// RUN: --offload-arch=gfx906 --offload-arch=gfx90a --no-offload-arch=gfx906 %s 2>&1 \ +// RUN: --offload-targets=amdgcn-amd-amdhsa --offload-arch=gfx906 \ +// RUN: --offload-arch=gfx90a --no-offload-arch=gfx906 %s 2>&1 \ // RUN: | FileCheck %s --check-prefix=REMOVED // REMOVED: clang-linker-wrapper // REMOVED-NOT: "--should-extract=gfx906" >From 4f4841be1f2fabfe35795077c89811e074091338 Mon Sep 17 00:00:00 2001 From: srividya sundaram <[email protected]> Date: Fri, 23 Oct 2026 14:23:29 -0700 Subject: [PATCH 3/5] Address review comments. Fold the --should-extract tests into hip-link-offload-binary.c and clarify the comment. --- clang/lib/Driver/ToolChains/Clang.cpp | 5 +++-- clang/test/Driver/hip-link-offload-binary.c | 21 +++++++++++++++++++++ clang/test/Driver/offload-should-extract.c | 19 ------------------- 3 files changed, 24 insertions(+), 21 deletions(-) delete mode 100644 clang/test/Driver/offload-should-extract.c diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index a67f6a761d0e6b..7af98823cb40e0 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -9909,8 +9909,9 @@ void LinkerWrapper::ConstructJob(Compilation &C, const JobAction &JA, } // If the user explicitly requested it via `--offload-arch` we should - // extract it from any static libraries if present. Use the canonical - // name, which device images are labeled with. + // extract it from any static libraries if present. Use the arch names + // from getOffloadArchs so target-ID features are sorted to match the + // image labels and --no-offload-arch is applied. if (ToolChainArgs.hasArg(OPT_offload_arch_EQ)) for (const BoundArch &BA : C.getDriver().getOffloadArchs(C, ToolChainArgs, Kind, *TC)) diff --git a/clang/test/Driver/hip-link-offload-binary.c b/clang/test/Driver/hip-link-offload-binary.c index 37756f66295937..aff2a970af6bef 100644 --- a/clang/test/Driver/hip-link-offload-binary.c +++ b/clang/test/Driver/hip-link-offload-binary.c @@ -21,6 +21,27 @@ // DRIVER-SAME: "--emit-fatbin-only" "-o" "{{.*}}.hipfb" // DRIVER-SAME: "{{.*}}.x.bundle.bc" "{{.*}}.y.bundle.bc" +/// Request the arch names that device images are labeled with: target-ID +/// features are sorted and --no-offload-arch is applied. +// RUN: %clang -### --target=x86_64-unknown-linux-gnu \ +// RUN: -fgpu-rdc --hip-link --cuda-device-only \ +// RUN: --offload-arch=gfx908:xnack+:sramecc- \ +// RUN: %t.x.bundle.bc -o %t.hipfb 2>&1 \ +// RUN: | FileCheck %s --check-prefix=TARGET-ID +// TARGET-ID: "{{.*}}clang-linker-wrapper" +// TARGET-ID-SAME: "--should-extract=gfx908:sramecc-:xnack+" + +// RUN: %clang -### --target=x86_64-unknown-linux-gnu \ +// RUN: -fgpu-rdc --hip-link --cuda-device-only \ +// RUN: --offload-targets=amdgcn-amd-amdhsa --offload-arch=gfx906 \ +// RUN: --offload-arch=gfx942 --no-offload-arch=gfx906 \ +// RUN: %t.x.bundle.bc -o %t.hipfb 2>&1 \ +// RUN: | FileCheck %s --check-prefix=REMOVED +// REMOVED: "{{.*}}clang-linker-wrapper" +// REMOVED-NOT: "--should-extract=gfx906" +// REMOVED-SAME: "--should-extract=gfx942" +// REMOVED-NOT: "--should-extract=gfx906" + // RUN: %clang -### -v --target=x86_64-unknown-linux-gnu \ // RUN: -fgpu-rdc --hip-link --cuda-device-only \ // RUN: --offload-arch=gfx906 %t.x.bundle.bc -o %t.hipfb 2>&1 \ diff --git a/clang/test/Driver/offload-should-extract.c b/clang/test/Driver/offload-should-extract.c deleted file mode 100644 index 0900fb8936ab21..00000000000000 --- a/clang/test/Driver/offload-should-extract.c +++ /dev/null @@ -1,19 +0,0 @@ -// The linker wrapper extracts device code from static libraries by the -// architecture its images are labeled with, so request the canonical name. - -// RUN: %clang -### --target=x86_64-unknown-linux-gnu -x hip -nogpulib -nogpuinc \ -// RUN: --offload-arch=gfx908:xnack+:sramecc- %s 2>&1 \ -// RUN: | FileCheck %s --check-prefix=CANONICAL -// CANONICAL: "--image=file={{.*}},triple={{[^,]*}},arch=gfx908:sramecc-:xnack+,kind=hip" -// CANONICAL: clang-linker-wrapper{{.*}} "--should-extract=gfx908:sramecc-:xnack+" - -/// An architecture removed with --no-offload-arch is not requested. Name the -/// target, so that every option reaches the linker wrapper's toolchain. -// RUN: %clang -### --target=x86_64-unknown-linux-gnu -x hip -nogpulib -nogpuinc \ -// RUN: --offload-targets=amdgcn-amd-amdhsa --offload-arch=gfx906 \ -// RUN: --offload-arch=gfx90a --no-offload-arch=gfx906 %s 2>&1 \ -// RUN: | FileCheck %s --check-prefix=REMOVED -// REMOVED: clang-linker-wrapper -// REMOVED-NOT: "--should-extract=gfx906" -// REMOVED-SAME: "--should-extract=gfx90a" -// REMOVED-NOT: "--should-extract=gfx906" >From d27f254702a73d7a454bf57830765e2f0884faaa Mon Sep 17 00:00:00 2001 From: srividya sundaram <[email protected]> Date: Thu, 29 Oct 2026 16:42:21 -0700 Subject: [PATCH 4/5] Cache getOffloadArchs per tool chain and offload kind. Compute the bound architectures once and reuse them, so the linker wrapper does not parse --offload-arch again. This also runs native GPU detection and reports its diagnostics only once. --- clang/include/clang/Driver/Compilation.h | 15 +++++++++++++++ clang/include/clang/Driver/Driver.h | 3 ++- clang/lib/Driver/Driver.cpp | 18 +++++++++++++++--- clang/lib/Driver/ToolChains/Clang.cpp | 2 +- clang/test/Driver/amdgpu-hip-system-arch.c | 22 ++++++++++++++++++++++ 5 files changed, 55 insertions(+), 5 deletions(-) diff --git a/clang/include/clang/Driver/Compilation.h b/clang/include/clang/Driver/Compilation.h index 825806b6cfe332..b7aafdb4e81662 100644 --- a/clang/include/clang/Driver/Compilation.h +++ b/clang/include/clang/Driver/Compilation.h @@ -16,6 +16,7 @@ #include "clang/Driver/Util.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Option/Option.h" #include <cassert> @@ -97,6 +98,12 @@ class Compilation { }; std::map<TCArgsKey, llvm::opt::DerivedArgList *> TCArgs; + /// Cache of the bound architectures for a particular tool chain and device + /// offload kind, filled by Driver::getOffloadArchs. + std::map<std::pair<const ToolChain *, Action::OffloadKind>, + std::optional<llvm::SmallVector<BoundArch>>> + OffloadArchs; + /// Temporary files which should be removed on exit. llvm::opt::ArgStringList TempFiles; @@ -256,6 +263,14 @@ class Compilation { getArgsForToolChain(const ToolChain *TC, BoundArch BA, Action::OffloadKind DeviceOffloadKind); + /// Return the cached bound architectures for tool chain \p TC and device + /// offload kind \p Kind. The entry holds no value until + /// Driver::getOffloadArchs computes it. + std::optional<llvm::SmallVector<BoundArch>> & + getCachedOffloadArchs(const ToolChain *TC, Action::OffloadKind Kind) { + return OffloadArchs[{TC, Kind}]; + } + /// addTempFile - Add a file to remove on exit, and returns its /// argument. const char *addTempFile(const char *Name) { diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index 914ef62cd62fa9..c1bb35a48b499a 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -522,7 +522,8 @@ class Driver { /// Returns the set of bound architectures active for this offload kind. /// If there are no bound architctures we return a set containing only the - /// empty string. + /// empty string. The result is computed from \p Args on the first call for + /// a tool chain and offload kind and cached in \p C. llvm::SmallVector<BoundArch> getOffloadArchs(Compilation &C, const llvm::opt::DerivedArgList &Args, Action::OffloadKind Kind, const ToolChain &TC) const; diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 707d6847caf446..dd392b87898aa5 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -3981,9 +3981,9 @@ getConflictOffloadArchCombination(const llvm::DenseSet<StringRef> &Archs, return getConflictTargetIDCombination(ArchSet); } -llvm::SmallVector<BoundArch> -Driver::getOffloadArchs(Compilation &C, const llvm::opt::DerivedArgList &Args, - Action::OffloadKind Kind, const ToolChain &TC) const { +static llvm::SmallVector<BoundArch> +computeOffloadArchs(Compilation &C, const llvm::opt::DerivedArgList &Args, + Action::OffloadKind Kind, const ToolChain &TC) { // --offload and --offload-arch options are mutually exclusive. if (Args.hasArgNoClaim(options::OPT_offload_EQ) && Args.hasArgNoClaim(options::OPT_offload_arch_EQ, @@ -4108,6 +4108,18 @@ Driver::getOffloadArchs(Compilation &C, const llvm::opt::DerivedArgList &Args, return Result; } +llvm::SmallVector<BoundArch> +Driver::getOffloadArchs(Compilation &C, const llvm::opt::DerivedArgList &Args, + Action::OffloadKind Kind, const ToolChain &TC) const { + // Compute the architectures once per tool chain and offload kind, so native + // GPU detection runs and its diagnostics are emitted only once. + std::optional<llvm::SmallVector<BoundArch>> &Cached = + C.getCachedOffloadArchs(&TC, Kind); + if (!Cached) + Cached = computeOffloadArchs(C, Args, Kind, TC); + return *Cached; +} + Action * Driver::BuildOffloadingActions(Compilation &C, llvm::opt::DerivedArgList &Args, const InputTy &Input, StringRef CUID, diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index d0bdd673cc5c33..07e9d14b00a75b 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -9929,7 +9929,7 @@ void LinkerWrapper::ConstructJob(Compilation &C, const JobAction &JA, // image labels and --no-offload-arch is applied. if (ToolChainArgs.hasArg(OPT_offload_arch_EQ)) for (const BoundArch &BA : - C.getDriver().getOffloadArchs(C, ToolChainArgs, Kind, *TC)) + C.getDriver().getOffloadArchs(C, C.getArgs(), Kind, *TC)) if (!BA.ArchName.empty()) CmdArgs.emplace_back( Args.MakeArgString("--should-extract=" + BA.ArchName)); diff --git a/clang/test/Driver/amdgpu-hip-system-arch.c b/clang/test/Driver/amdgpu-hip-system-arch.c index 0949159d460923..9cfb34828a2932 100644 --- a/clang/test/Driver/amdgpu-hip-system-arch.c +++ b/clang/test/Driver/amdgpu-hip-system-arch.c @@ -32,3 +32,25 @@ // RUN: -x hip %s 2>&1 | \ // RUN: FileCheck %s --check-prefix=BAD-TIMEOUT // BAD-TIMEOUT: clang: error: cannot determine hip architecture: CLANG_TOOLCHAIN_PROGRAM_TIMEOUT expected an integer, got 'foo'; consider passing it via '--offload-arch'; environment variable CLANG_TOOLCHAIN_PROGRAM_TIMEOUT specifies the tool timeout (integer secs, <=0 is infinite) + +/// With an explicit offload target, '--offload-arch=native' reaches +/// getOffloadArchs. The tool runs once and a failure is reported once. +// RUN: echo '#!/bin/sh' > %t/amdgpu_arch_count +// RUN: echo 'echo run >> "$(dirname "$0")/runs"' >> %t/amdgpu_arch_count +// RUN: echo 'echo gfx906' >> %t/amdgpu_arch_count +// RUN: chmod +x %t/amdgpu_arch_count +// RUN: rm -f %t/runs +// RUN: %clang -### --target=x86_64-unknown-linux-gnu -nogpuinc -nogpulib \ +// RUN: --offload-targets=amdgcn-amd-amdhsa --offload-arch=native \ +// RUN: --amdgpu-arch-tool=%t/amdgpu_arch_count -fgpu-rdc -x hip %s 2>&1 \ +// RUN: | FileCheck %s --check-prefix=NATIVE-ONCE +// RUN: count 1 < %t/runs +// NATIVE-ONCE: "-cc1" "-triple" "amdgpu9.06-amd-amdhsa" +// NATIVE-ONCE: clang-linker-wrapper{{.*}}"--should-extract=gfx906" + +// RUN: not %clang -### --target=x86_64-unknown-linux-gnu -nogpuinc -nogpulib \ +// RUN: --offload-targets=amdgcn-amd-amdhsa --offload-arch=native \ +// RUN: --amdgpu-arch-tool=%t/amdgpu_arch_fail -x hip %s 2>&1 \ +// RUN: | FileCheck %s --check-prefix=FAIL-ONCE +// FAIL-ONCE: error: cannot determine amdgpu architecture +// FAIL-ONCE-NOT: error: cannot determine amdgpu architecture >From b1215f36e52315bce7f98ee1bb8286d5b7acf26d Mon Sep 17 00:00:00 2001 From: srividya sundaram <[email protected]> Date: Fri, 30 Oct 2026 10:46:07 -0700 Subject: [PATCH 5/5] Copy cached offload arch names into compilation storage. A crash reproducer frees the toolchain args, which the cached names may point into. --- clang/lib/Driver/Driver.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index dd392b87898aa5..2d0f3666735c06 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -4115,8 +4115,14 @@ Driver::getOffloadArchs(Compilation &C, const llvm::opt::DerivedArgList &Args, // GPU detection runs and its diagnostics are emitted only once. std::optional<llvm::SmallVector<BoundArch>> &Cached = C.getCachedOffloadArchs(&TC, Kind); - if (!Cached) + if (!Cached) { Cached = computeOffloadArchs(C, Args, Kind, TC); + // The names may point into toolchain args that a crash reproducer frees, + // so copy them into storage that lives as long as the compilation. + for (BoundArch &BA : *Cached) + if (!BA.empty()) + BA.ArchName = C.getArgs().MakeArgStringRef(BA.ArchName); + } return *Cached; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
