[clang] [llvm] Revert "clang-linker-wrapper: Use AMDGPU::TargetID for device-image compatibility" (PR #209407)
Matt Arsenault via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 14 02:07:29 PDT 2026
https://github.com/arsenm created https://github.com/llvm/llvm-project/pull/209407
Reverts llvm/llvm-project#209135
This shouldn't be doing logical linking compatibility
>From cca525d50974b13c0797ca95c322611dbffd60c5 Mon Sep 17 00:00:00 2001
From: Matt Arsenault <arsenm2 at gmail.com>
Date: Tue, 14 Jul 2026 11:07:04 +0200
Subject: [PATCH] =?UTF-8?q?Revert=20"clang-linker-wrapper:=20Use=20AMDGPU:?=
=?UTF-8?q?:TargetID=20for=20device-image=20compatibi=E2=80=A6"?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This reverts commit dc1273beba1c70cb74d03c760958ee625d8aecf5.
---
.../clang-linker-wrapper/linker-wrapper.c | 57 -------------
.../ClangLinkerWrapper.cpp | 3 +-
.../llvm/TargetParser/AMDGPUTargetParser.h | 8 --
llvm/lib/Object/OffloadBinary.cpp | 39 +++++----
llvm/lib/TargetParser/AMDGPUTargetParser.cpp | 84 ++++++++-----------
.../TargetParser/TargetParserTest.cpp | 48 -----------
6 files changed, 59 insertions(+), 180 deletions(-)
diff --git a/clang/test/OffloadTools/clang-linker-wrapper/linker-wrapper.c b/clang/test/OffloadTools/clang-linker-wrapper/linker-wrapper.c
index f4bac21181a9e..0e018dc3af8c9 100644
--- a/clang/test/OffloadTools/clang-linker-wrapper/linker-wrapper.c
+++ b/clang/test/OffloadTools/clang-linker-wrapper/linker-wrapper.c
@@ -217,63 +217,6 @@ __attribute__((visibility("protected"), used)) int x;
// ARCH-ALL: clang{{.*}} -o {{.*}}.img -dumpdir a.out.amdgcn.gfx90a.img. --target=amdgcn-amd-amdhsa -mcpu=gfx90a -Wl,--no-undefined {{.*}}.o {{.*}}.o
// ARCH-ALL: clang{{.*}} -o {{.*}}.img -dumpdir a.out.amdgcn.gfx908.img. --target=amdgcn-amd-amdhsa -mcpu=gfx908 -Wl,--no-undefined {{.*}}.o {{.*}}.o
-// Two images with the same ISA but different triple spellings (the legacy
-// "amdgcn" alias and the canonical "amdgpu") must merge into a single device
-// link, not produce two separate device images.
-// RUN: llvm-offload-binary -o %t-legacy.out \
-// RUN: --image=file=%t.elf.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx90a
-// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t-legacy.o -fembed-offload-object=%t-legacy.out
-// RUN: llvm-offload-binary -o %t-canon.out \
-// RUN: --image=file=%t.elf.o,kind=openmp,triple=amdgpu-amd-amdhsa,arch=gfx90a
-// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t-canon.o -fembed-offload-object=%t-canon.out
-// RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
-// RUN: --linker-path=/usr/bin/ld %t-legacy.o %t-canon.o -o a.out 2>&1 | FileCheck %s --check-prefix=AMDGPU-TRIPLE-MERGE
-
-// AMDGPU-TRIPLE-MERGE: clang{{.*}} --target=amdgcn-amd-amdhsa -mcpu=gfx90a -Wl,--no-undefined {{.*}}.o {{.*}}.o
-// AMDGPU-TRIPLE-MERGE-NOT: clang{{.*}} --target=amdgcn-amd-amdhsa -mcpu=gfx90a -Wl,--no-undefined
-
-// Two images whose triples encode the ISA in the subarch (amdgpu9.0a-amd-amdhsa)
-// and carry no separate arch must merge into a single device link.
-// RUN: llvm-offload-binary -o %t-sub1.out \
-// RUN: --image=file=%t.elf.o,kind=openmp,triple=amdgpu9.0a-amd-amdhsa
-// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t-sub1.o -fembed-offload-object=%t-sub1.out
-// RUN: llvm-offload-binary -o %t-sub2.out \
-// RUN: --image=file=%t.elf.o,kind=openmp,triple=amdgpu9.0a-amd-amdhsa
-// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t-sub2.o -fembed-offload-object=%t-sub2.out
-// RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
-// RUN: --linker-path=/usr/bin/ld %t-sub1.o %t-sub2.o -o a.out 2>&1 | FileCheck %s --check-prefix=AMDGPU-SUBARCH-MERGE
-
-// AMDGPU-SUBARCH-MERGE: clang{{.*}} --target=amdgpu9.0a-amd-amdhsa -Wl,--no-undefined {{.*}}.o {{.*}}.o
-// AMDGPU-SUBARCH-MERGE-NOT: clang{{.*}} --target=amdgpu9.0a-amd-amdhsa -Wl,--no-undefined
-
-// A major-family subarch triple (amdgpu9-amd-amdhsa) acts as a wildcard that
-// merges into a specific member of that family (amdgpu9.00-amd-amdhsa).
-// RUN: llvm-offload-binary -o %t-specific.out \
-// RUN: --image=file=%t.elf.o,kind=openmp,triple=amdgpu9.00-amd-amdhsa
-// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t-specific.o -fembed-offload-object=%t-specific.out
-// RUN: llvm-offload-binary -o %t-major.out \
-// RUN: --image=file=%t.elf.o,kind=openmp,triple=amdgpu9-amd-amdhsa
-// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t-major.o -fembed-offload-object=%t-major.out
-// RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
-// RUN: --linker-path=/usr/bin/ld %t-specific.o %t-major.o -o a.out 2>&1 | FileCheck %s --check-prefix=AMDGPU-MAJOR-SUBARCH-MERGE
-
-// AMDGPU-MAJOR-SUBARCH-MERGE: clang{{.*}} --target=amdgpu9.00-amd-amdhsa -Wl,--no-undefined {{.*}}.o {{.*}}.o
-// AMDGPU-MAJOR-SUBARCH-MERGE-NOT: clang{{.*}} --target={{.*}}-amd-amdhsa -Wl,--no-undefined
-
-// Two distinct GPUs in the same major family are not interchangeable and must
-// not merge.
-// RUN: llvm-offload-binary -o %t-gfx900.out \
-// RUN: --image=file=%t.elf.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx900
-// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t-gfx900.o -fembed-offload-object=%t-gfx900.out
-// RUN: llvm-offload-binary -o %t-gfx906.out \
-// RUN: --image=file=%t.elf.o,kind=openmp,triple=amdgcn-amd-amdhsa,arch=gfx906
-// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t-gfx906.o -fembed-offload-object=%t-gfx906.out
-// RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
-// RUN: --linker-path=/usr/bin/ld %t-gfx900.o %t-gfx906.o -o a.out 2>&1 | FileCheck %s --check-prefix=AMDGPU-SAME-MAJOR-NO-MERGE
-
-// AMDGPU-SAME-MAJOR-NO-MERGE-DAG: clang{{.*}} -mcpu=gfx900 -Wl,--no-undefined {{.*}}.o
-// AMDGPU-SAME-MAJOR-NO-MERGE-DAG: clang{{.*}} -mcpu=gfx906 -Wl,--no-undefined {{.*}}.o
-
// RUN: llvm-offload-binary -o %t.out \
// RUN: --image=file=%t.elf.o,kind=openmp,triple=x86_64-unknown-linux-gnu \
// RUN: --image=file=%t.elf.o,kind=openmp,triple=x86_64-unknown-linux-gnu
diff --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index afa84eea41aae..12b6cae3ea792 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -1414,7 +1414,8 @@ getDeviceInput(const ArgList &Args) {
OffloadFile::TargetID Target = Binary;
SmallVector<OffloadFile::TargetID> CompatibleTargets;
for (const auto &[ID, Input] : InputFiles)
- if (Target == ID || object::areTargetsCompatible(Target, ID))
+ if (Target.first == ID.first &&
+ clang::isCompatibleTargetID(Target.second, ID.second))
CompatibleTargets.emplace_back(ID);
// Seed a new image when no existing target can provide for this input.
diff --git a/llvm/include/llvm/TargetParser/AMDGPUTargetParser.h b/llvm/include/llvm/TargetParser/AMDGPUTargetParser.h
index d5e2609d20f25..ab8cb05ed80f6 100644
--- a/llvm/include/llvm/TargetParser/AMDGPUTargetParser.h
+++ b/llvm/include/llvm/TargetParser/AMDGPUTargetParser.h
@@ -167,10 +167,6 @@ class LLVM_ABI TargetID {
TargetID(GPUKind Arch, const Triple &TT, TargetIDSetting XnackSetting,
TargetIDSetting SramEccSetting);
- /// Construct a TargetID from a triple \p TT and the processor+features string
- /// e.g. "gfx90a", "gfx90a:xnack+:sramecc-", "".
- TargetID(const Triple &TT, StringRef TargetIDStr);
-
~TargetID() = default;
/// \return True if the current xnack setting is not "Unsupported".
@@ -239,10 +235,6 @@ class LLVM_ABI TargetID {
static std::optional<TargetID>
parseTargetIDString(StringRef TargetIDDirective);
- /// Returns true if a device image built for *this can satisfy a request for
- /// \p Other (i.e. they are compatible and can be grouped together).
- bool isCompatibleWith(const TargetID &Other) const;
-
void print(raw_ostream &OS) const;
std::string toString() const;
diff --git a/llvm/lib/Object/OffloadBinary.cpp b/llvm/lib/Object/OffloadBinary.cpp
index d1d01f9f7a667..e5c9f4dae131d 100644
--- a/llvm/lib/Object/OffloadBinary.cpp
+++ b/llvm/lib/Object/OffloadBinary.cpp
@@ -22,7 +22,6 @@
#include "llvm/Object/ObjectFile.h"
#include "llvm/Support/Alignment.h"
#include "llvm/Support/SourceMgr.h"
-#include "llvm/TargetParser/AMDGPUTargetParser.h"
using namespace llvm;
using namespace llvm::object;
@@ -454,25 +453,31 @@ bool object::areTargetsCompatible(const OffloadFile::TargetID &LHS,
if (LHS == RHS)
return false;
- llvm::Triple LHSTT(LHS.first);
- llvm::Triple RHSTT(RHS.first);
-
- // The AMDGPU target requires target-id aware checks (base processor plus
- // xnack/sramecc features), which also handle the generic wildcard and the
- // legacy "amdgcn"/"amdgpu" triple spellings.
- if (LHSTT.isAMDGPU()) {
- AMDGPU::TargetID LTID(LHSTT, LHS.second);
- AMDGPU::TargetID RTID(RHSTT, RHS.second);
- return LTID.isCompatibleWith(RTID);
- }
-
- // For other targets the triples must be compatible.
- if (!LHSTT.isCompatibleWith(RHSTT))
+ // The triples must match at all times.
+ if (LHS.first != RHS.first)
return false;
- // If the architecture is "generic" we assume it is always compatible.
+ // If the architecture is "all" we assume it is always compatible.
if (LHS.second == "generic" || RHS.second == "generic")
return true;
- return LHS.second == RHS.second;
+ // Only The AMDGPU target requires additional checks.
+ llvm::Triple T(LHS.first);
+ if (!T.isAMDGPU())
+ return false;
+
+ // The base processor must always match.
+ if (LHS.second.split(":").first != RHS.second.split(":").first)
+ return false;
+
+ // Check combintions of on / off features that must match.
+ if (LHS.second.contains("xnack+") && RHS.second.contains("xnack-"))
+ return false;
+ if (LHS.second.contains("xnack-") && RHS.second.contains("xnack+"))
+ return false;
+ if (LHS.second.contains("sramecc-") && RHS.second.contains("sramecc+"))
+ return false;
+ if (LHS.second.contains("sramecc+") && RHS.second.contains("sramecc-"))
+ return false;
+ return true;
}
diff --git a/llvm/lib/TargetParser/AMDGPUTargetParser.cpp b/llvm/lib/TargetParser/AMDGPUTargetParser.cpp
index c5f4247117d1b..bf169c5b5ef42 100644
--- a/llvm/lib/TargetParser/AMDGPUTargetParser.cpp
+++ b/llvm/lib/TargetParser/AMDGPUTargetParser.cpp
@@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//
#include "llvm/TargetParser/AMDGPUTargetParser.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/ADT/Twine.h"
@@ -916,28 +917,6 @@ getTargetIDSettingFromFeatureString(StringRef FeatureString) {
llvm_unreachable("Malformed feature string");
}
-// Derive the architecture from the processor name in \p TargetIDStr. "generic"
-// and the empty processor name act as a wildcard.
-static GPUKind getGPUKindFromTargetID(const Triple &TT, StringRef TargetIDStr) {
- StringRef CPUName = TargetIDStr.split(':').first;
- return (CPUName.empty() || CPUName == "generic")
- ? getGPUKindFromSubArch(TT.getSubArch())
- : parseArchAMDGCN(CPUName);
-}
-
-TargetID::TargetID(const Triple &TT, StringRef TargetIDStr)
- : TargetID(getGPUKindFromTargetID(TT, TargetIDStr), TT,
- TargetIDSetting::Unsupported, TargetIDSetting::Unsupported) {
- // Default xnack/sramecc to the "Any" wildcard when the architecture supports
- // them, then apply any explicit feature overrides from the target-id string.
- unsigned ArchAttr = getArchAttrAMDGCN(Arch);
- if (ArchAttr & FEATURE_XNACK)
- XnackSetting = TargetIDSetting::Any;
- if (ArchAttr & FEATURE_SRAMECC)
- SramEccSetting = TargetIDSetting::Any;
- setTargetIDFromTargetIDStream(TargetIDStr);
-}
-
void TargetID::setTargetIDFromTargetIDStream(StringRef TargetID) {
SmallVector<StringRef, 3> TargetIDSplit;
TargetID.split(TargetIDSplit, ':');
@@ -963,10 +942,40 @@ TargetID::parseTargetIDString(StringRef TargetIDDirective) {
if (!TT.isAMDGCN())
return std::nullopt;
- // The processor+features field must be present, even if empty (the ISA can
- // be encoded in the triple's subarch, e.g.
- // "amdgpu12.50-amd-amdhsa-unknown-").
- return TargetID(TT, Parts[4]);
+ SmallVector<StringRef, 3> FeatureSplit;
+ Parts[4].split(FeatureSplit, ':');
+ if (FeatureSplit.empty())
+ return std::nullopt;
+
+ StringRef CPUName = FeatureSplit[0];
+
+ // Prefer the explicitly named processor so the parsed target id reflects it
+ // (e.g. for validation against the triple subarch). The processor field may
+ // be empty when the ISA is already encoded in the triple's subarch
+ // (e.g. "amdgpu12.50-amd-amdhsa-unknown-"), in which case derive the arch
+ // from the subarch.
+ GPUKind Arch = CPUName.empty() ? getGPUKindFromSubArch(TT.getSubArch())
+ : parseArchAMDGCN(CPUName);
+
+ unsigned ArchAttr = getArchAttrAMDGCN(Arch);
+
+ // Determine xnack/sramecc support based on the architecture attributes.
+ TargetIDSetting XnackSetting = (ArchAttr & FEATURE_XNACK)
+ ? TargetIDSetting::Any
+ : TargetIDSetting::Unsupported;
+ TargetIDSetting SramEccSetting = (ArchAttr & FEATURE_SRAMECC)
+ ? TargetIDSetting::Any
+ : TargetIDSetting::Unsupported;
+
+ for (StringRef FeatureString :
+ ArrayRef<StringRef>(FeatureSplit).drop_front(1)) {
+ if (FeatureString.starts_with("xnack"))
+ XnackSetting = getTargetIDSettingFromFeatureString(FeatureString);
+ else if (FeatureString.starts_with("sramecc"))
+ SramEccSetting = getTargetIDSettingFromFeatureString(FeatureString);
+ }
+
+ return TargetID(Arch, TT, XnackSetting, SramEccSetting);
}
void TargetID::print(raw_ostream &StreamRep) const {
@@ -999,26 +1008,3 @@ bool TargetID::operator==(const TargetID &Other) const {
SramEccSetting == Other.SramEccSetting && IsAMDHSA == Other.IsAMDHSA &&
TargetTripleString == Other.TargetTripleString;
}
-
-static bool areFeatureSettingsCompatible(TargetIDSetting A, TargetIDSetting B) {
- return A == TargetIDSetting::Any || B == TargetIDSetting::Any || A == B;
-}
-
-bool TargetID::isCompatibleWith(const TargetID &Other) const {
- // The triples must be compatible.
- if (!Triple(getTargetTripleString())
- .isCompatibleWith(Triple(Other.getTargetTripleString())))
- return false;
-
- // The processors must be compatible. A generic/major-family image (its
- // subarch is the major-family subarch, or the GPU is unknown) acts as a
- // wildcard that merges into a specific image group.
- Triple::SubArchType SubA = getSubArch(Arch);
- Triple::SubArchType SubB = getSubArch(Other.Arch);
- if (!isSubArchCompatible(SubA, SubB))
- return false;
-
- // The xnack/sramecc settings must not conflict.
- return areFeatureSettingsCompatible(XnackSetting, Other.XnackSetting) &&
- areFeatureSettingsCompatible(SramEccSetting, Other.SramEccSetting);
-}
diff --git a/llvm/unittests/TargetParser/TargetParserTest.cpp b/llvm/unittests/TargetParser/TargetParserTest.cpp
index 1b6216f9ae68f..231e3cb306298 100644
--- a/llvm/unittests/TargetParser/TargetParserTest.cpp
+++ b/llvm/unittests/TargetParser/TargetParserTest.cpp
@@ -2828,52 +2828,4 @@ TEST(TargetParserTest, testAMDGPUgetIsaVersionFromSubArch) {
(AMDGPU::IsaVersion{0, 0, 0}));
}
-TEST(TargetParserTest, testAMDGPUTargetIDCompat) {
- using AMDGPU::TargetID;
- Triple AMDHSA("amdgcn-amd-amdhsa");
- Triple AMDHSACanon("amdgpu-amd-amdhsa");
-
- auto Compatible = [](const TargetID &A, const TargetID &B) {
- // Compatibility is symmetric for the cases exercised here.
- EXPECT_EQ(A.isCompatibleWith(B), B.isCompatibleWith(A));
- return A.isCompatibleWith(B);
- };
-
- // Exact match is compatible.
- EXPECT_TRUE(
- Compatible(TargetID(AMDHSA, "gfx90a"), TargetID(AMDHSA, "gfx90a")));
-
- // Different processors are not compatible.
- EXPECT_FALSE(
- Compatible(TargetID(AMDHSA, "gfx90a"), TargetID(AMDHSA, "gfx908")));
-
- // "generic" and empty act as a wildcard that merges into a specific image,
- // in both directions.
- EXPECT_TRUE(
- Compatible(TargetID(AMDHSA, "generic"), TargetID(AMDHSA, "gfx90a")));
- EXPECT_TRUE(Compatible(TargetID(AMDHSA, ""), TargetID(AMDHSA, "gfx908")));
-
- // Major-subarch wildcard triple merges into a specific processor.
- Triple AMDHSAMajor9("amdgpu9-amd-amdhsa");
- EXPECT_TRUE(
- Compatible(TargetID(AMDHSAMajor9, ""), TargetID(AMDHSA, "gfx900")));
-
- // Explicit On vs Off features must not match.
- EXPECT_FALSE(Compatible(TargetID(AMDHSA, "gfx90a:xnack+"),
- TargetID(AMDHSA, "gfx90a:xnack-")));
- EXPECT_FALSE(Compatible(TargetID(AMDHSA, "gfx90a:sramecc+"),
- TargetID(AMDHSA, "gfx90a:sramecc-")));
-
- // "Any"/"Unsupported" settings act as wildcards: a plain gfx90a (xnack "Any")
- // merges with an explicit xnack+ or xnack- request.
- EXPECT_TRUE(Compatible(TargetID(AMDHSA, "gfx90a"),
- TargetID(AMDHSA, "gfx90a:xnack+")));
- EXPECT_TRUE(Compatible(TargetID(AMDHSA, "gfx90a"),
- TargetID(AMDHSA, "gfx90a:xnack-")));
-
- // The legacy "amdgcn" and canonical "amdgpu" triple spellings merge.
- EXPECT_TRUE(
- Compatible(TargetID(AMDHSA, "gfx90a"), TargetID(AMDHSACanon, "gfx90a")));
-}
-
} // namespace
More information about the cfe-commits
mailing list