Author: Matt Arsenault Date: 2026-07-13T15:05:03+02:00 New Revision: dc1273beba1c70cb74d03c760958ee625d8aecf5
URL: https://github.com/llvm/llvm-project/commit/dc1273beba1c70cb74d03c760958ee625d8aecf5 DIFF: https://github.com/llvm/llvm-project/commit/dc1273beba1c70cb74d03c760958ee625d8aecf5.diff LOG: clang-linker-wrapper: Use AMDGPU::TargetID for device-image compatibility (#209135) Previously the link compatibilty was based on a raw string match of the triple + cpu name. Start parsing the triple, and use the compatibility function. Start using AMDGPU::TargetID from TargetParser, as a step towards removing clang's redundant TargetID handling. Co-authored-by: Claude (Opus 4.8) Added: Modified: clang/test/OffloadTools/clang-linker-wrapper/linker-wrapper.c clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp llvm/include/llvm/TargetParser/AMDGPUTargetParser.h llvm/lib/Object/OffloadBinary.cpp llvm/lib/TargetParser/AMDGPUTargetParser.cpp llvm/unittests/TargetParser/TargetParserTest.cpp Removed: ################################################################################ diff --git a/clang/test/OffloadTools/clang-linker-wrapper/linker-wrapper.c b/clang/test/OffloadTools/clang-linker-wrapper/linker-wrapper.c index 0e018dc3af8c9..f4bac21181a9e 100644 --- a/clang/test/OffloadTools/clang-linker-wrapper/linker-wrapper.c +++ b/clang/test/OffloadTools/clang-linker-wrapper/linker-wrapper.c @@ -217,6 +217,63 @@ __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 diff erent 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 12b6cae3ea792..afa84eea41aae 100644 --- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp +++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp @@ -1414,8 +1414,7 @@ getDeviceInput(const ArgList &Args) { OffloadFile::TargetID Target = Binary; SmallVector<OffloadFile::TargetID> CompatibleTargets; for (const auto &[ID, Input] : InputFiles) - if (Target.first == ID.first && - clang::isCompatibleTargetID(Target.second, ID.second)) + if (Target == ID || object::areTargetsCompatible(Target, ID)) 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 4c1f17787e939..0fba0d4956d02 100644 --- a/llvm/include/llvm/TargetParser/AMDGPUTargetParser.h +++ b/llvm/include/llvm/TargetParser/AMDGPUTargetParser.h @@ -166,6 +166,10 @@ 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". @@ -234,6 +238,10 @@ 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 e5c9f4dae131d..d1d01f9f7a667 100644 --- a/llvm/lib/Object/OffloadBinary.cpp +++ b/llvm/lib/Object/OffloadBinary.cpp @@ -22,6 +22,7 @@ #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; @@ -453,31 +454,25 @@ bool object::areTargetsCompatible(const OffloadFile::TargetID &LHS, if (LHS == RHS) return false; - // The triples must match at all times. - if (LHS.first != RHS.first) - return false; + llvm::Triple LHSTT(LHS.first); + llvm::Triple RHSTT(RHS.first); - // If the architecture is "all" we assume it is always compatible. - if (LHS.second == "generic" || RHS.second == "generic") - return true; + // 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); + } - // Only The AMDGPU target requires additional checks. - llvm::Triple T(LHS.first); - if (!T.isAMDGPU()) + // For other targets the triples must be compatible. + if (!LHSTT.isCompatibleWith(RHSTT)) return false; - // The base processor must always match. - if (LHS.second.split(":").first != RHS.second.split(":").first) - return false; + // If the architecture is "generic" we assume it is always compatible. + if (LHS.second == "generic" || RHS.second == "generic") + return true; - // 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; + return LHS.second == RHS.second; } diff --git a/llvm/lib/TargetParser/AMDGPUTargetParser.cpp b/llvm/lib/TargetParser/AMDGPUTargetParser.cpp index bf169c5b5ef42..c5f4247117d1b 100644 --- a/llvm/lib/TargetParser/AMDGPUTargetParser.cpp +++ b/llvm/lib/TargetParser/AMDGPUTargetParser.cpp @@ -11,7 +11,6 @@ //===----------------------------------------------------------------------===// #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" @@ -917,6 +916,28 @@ 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, ':'); @@ -942,40 +963,10 @@ TargetID::parseTargetIDString(StringRef TargetIDDirective) { if (!TT.isAMDGCN()) return std::nullopt; - 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); + // 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]); } void TargetID::print(raw_ostream &StreamRep) const { @@ -1008,3 +999,26 @@ 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 231e3cb306298..1b6216f9ae68f 100644 --- a/llvm/unittests/TargetParser/TargetParserTest.cpp +++ b/llvm/unittests/TargetParser/TargetParserTest.cpp @@ -2828,4 +2828,52 @@ 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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
