https://github.com/arsenm updated https://github.com/llvm/llvm-project/pull/209845
>From c8849237f34276d6d66a72a1a5a0ab59bc1a5445 Mon Sep 17 00:00:00 2001 From: Matt Arsenault <[email protected]> Date: Wed, 17 Jun 2026 13:30:41 +0200 Subject: [PATCH 1/3] clang: Use TargetID parsing from AMDGPUTargetParser We had grown 2 parallel parsing implementations for triple+gpu name+feature flag target ID strings. Mostly eliminate the redundant clang version. Co-authored-by: Claude (Opus 4.8) --- clang/include/clang/Basic/TargetID.h | 40 +--- clang/lib/Basic/TargetID.cpp | 194 +++--------------- clang/lib/Basic/Targets/AMDGPU.cpp | 31 +-- clang/lib/Basic/Targets/AMDGPU.h | 32 +-- clang/lib/Driver/Driver.cpp | 16 +- clang/lib/Driver/OffloadBundler.cpp | 70 +++---- clang/lib/Driver/ToolChains/AMDGPU.cpp | 98 +++++---- clang/lib/Driver/ToolChains/AMDGPU.h | 23 +-- .../llvm/TargetParser/AMDGPUTargetParser.h | 2 +- llvm/lib/TargetParser/AMDGPUTargetParser.cpp | 2 +- .../TargetParser/TargetParserTest.cpp | 6 +- 11 files changed, 172 insertions(+), 342 deletions(-) diff --git a/clang/include/clang/Basic/TargetID.h b/clang/include/clang/Basic/TargetID.h index 902151d76556d..5625f1df45142 100644 --- a/clang/include/clang/Basic/TargetID.h +++ b/clang/include/clang/Basic/TargetID.h @@ -9,53 +9,27 @@ #ifndef LLVM_CLANG_BASIC_TARGETID_H #define LLVM_CLANG_BASIC_TARGETID_H -#include "llvm/ADT/SmallVector.h" -#include "llvm/ADT/StringMap.h" -#include "llvm/TargetParser/Triple.h" #include <optional> #include <set> +#include <string> -namespace clang { +namespace llvm { +class Triple; +} -/// Get all feature strings that can be used in target ID for \p Processor. -/// Target ID is a processor name with optional feature strings -/// postfixed by a plus or minus sign delimited by colons, e.g. -/// gfx908:xnack+:sramecc-. Each processor have a limited -/// number of predefined features when showing up in a target ID. -llvm::SmallVector<llvm::StringRef, 4> -getAllPossibleTargetIDFeatures(const llvm::Triple &T, - llvm::StringRef Processor); +namespace clang { /// Get processor name from target ID. /// Returns canonical processor name or empty if the processor name is invalid. llvm::StringRef getProcessorFromTargetID(const llvm::Triple &T, llvm::StringRef OffloadArch); -/// Parse a target ID to get processor and feature map. -/// Returns canonicalized processor name or std::nullopt if the target ID is -/// invalid. Returns target ID features in \p FeatureMap if it is not null -/// pointer. This function assumes \p OffloadArch is a valid target ID. -/// If the target ID contains feature+, map it to true. -/// If the target ID contains feature-, map it to false. -/// If the target ID does not contain a feature (default), do not map it. -std::optional<llvm::StringRef> parseTargetID(const llvm::Triple &T, - llvm::StringRef OffloadArch, - llvm::StringMap<bool> *FeatureMap); - -/// Returns canonical target ID, assuming \p Processor is canonical and all -/// entries in \p Features are valid. -std::string getCanonicalTargetID(llvm::StringRef Processor, - const llvm::StringMap<bool> &Features); - /// Get the conflicted pair of target IDs for a compilation or a bundled code /// object, assuming \p TargetIDs are canonicalized. If there is no conflicts, /// returns std::nullopt. std::optional<std::pair<llvm::StringRef, llvm::StringRef>> -getConflictTargetIDCombination(const std::set<llvm::StringRef> &TargetIDs); - -/// Check whether the provided target ID is compatible with the requested -/// target ID. -bool isCompatibleTargetID(llvm::StringRef Provided, llvm::StringRef Requested); +getConflictTargetIDCombination(const llvm::Triple &T, + const std::set<llvm::StringRef> &TargetIDs); /// Sanitize a target ID string for use in a file name. /// Replaces invalid characters (like ':') with safe characters (like '@'). diff --git a/clang/lib/Basic/TargetID.cpp b/clang/lib/Basic/TargetID.cpp index 67f429607ef27..68064558f64c9 100644 --- a/clang/lib/Basic/TargetID.cpp +++ b/clang/lib/Basic/TargetID.cpp @@ -7,186 +7,50 @@ //===----------------------------------------------------------------------===// #include "clang/Basic/TargetID.h" -#include "llvm/ADT/STLExtras.h" -#include "llvm/ADT/SmallSet.h" -#include "llvm/ADT/SmallVector.h" #include "llvm/Support/Path.h" #include "llvm/TargetParser/AMDGPUTargetParser.h" -#include "llvm/TargetParser/Triple.h" -#include <map> -#include <optional> -#include <string> namespace clang { -static llvm::SmallVector<llvm::StringRef, 4> -getAllPossibleAMDGPUTargetIDFeatures(const llvm::Triple &T, - llvm::StringRef Proc) { - // Entries in returned vector should be in alphabetical order. - llvm::SmallVector<llvm::StringRef, 4> Ret; - auto ProcKind = T.isAMDGCN() ? llvm::AMDGPU::parseArchAMDGCN(Proc) - : llvm::AMDGPU::parseArchR600(Proc); - if (ProcKind == llvm::AMDGPU::GK_NONE) - return Ret; - auto Features = T.isAMDGCN() ? llvm::AMDGPU::getArchAttrAMDGCN(ProcKind) - : llvm::AMDGPU::getArchAttrR600(ProcKind); - if (Features & llvm::AMDGPU::FEATURE_SRAMECC) - Ret.push_back("sramecc"); - // Only allow xnack in target ID if the processor supports on/off modes. - if (Features & llvm::AMDGPU::FEATURE_XNACK_ON_OFF_MODES) - Ret.push_back("xnack"); - return Ret; -} - -llvm::SmallVector<llvm::StringRef, 4> -getAllPossibleTargetIDFeatures(const llvm::Triple &T, - llvm::StringRef Processor) { - llvm::SmallVector<llvm::StringRef, 4> Ret; - if (T.isAMDGPU()) - return getAllPossibleAMDGPUTargetIDFeatures(T, Processor); - return Ret; -} - -/// Returns canonical processor name or empty string if \p Processor is invalid. -static llvm::StringRef getCanonicalProcessorName(const llvm::Triple &T, - llvm::StringRef Processor) { - if (T.isAMDGPU()) - return llvm::AMDGPU::getCanonicalArchName(T, Processor); - return Processor; -} - llvm::StringRef getProcessorFromTargetID(const llvm::Triple &T, - llvm::StringRef TargetID) { - auto Split = TargetID.split(':'); - return getCanonicalProcessorName(T, Split.first); -} - -// Parse a target ID with format checking only. Do not check whether processor -// name or features are valid for the processor. -// -// A target ID is a processor name followed by a list of target features -// delimited by colon. Each target feature is a string post-fixed by a plus -// or minus sign, e.g. gfx908:sramecc+:xnack-. -static std::optional<llvm::StringRef> -parseTargetIDWithFormatCheckingOnly(llvm::StringRef TargetID, - llvm::StringMap<bool> *FeatureMap) { - llvm::StringRef Processor; - - if (TargetID.empty()) - return llvm::StringRef(); - - auto Split = TargetID.split(':'); - Processor = Split.first; - if (Processor.empty()) - return std::nullopt; - - auto Features = Split.second; - if (Features.empty()) - return Processor; - - llvm::StringMap<bool> LocalFeatureMap; - if (!FeatureMap) - FeatureMap = &LocalFeatureMap; - - while (!Features.empty()) { - auto Splits = Features.split(':'); - if (Splits.first.empty()) - return std::nullopt; - auto Sign = Splits.first.back(); - auto Feature = Splits.first.drop_back(); - if (Sign != '+' && Sign != '-') - return std::nullopt; - bool IsOn = Sign == '+'; - // Each feature can only show up at most once in target ID. - if (!FeatureMap->try_emplace(Feature, IsOn).second) - return std::nullopt; - Features = Splits.second; - } - return Processor; -} - -std::optional<llvm::StringRef> -parseTargetID(const llvm::Triple &T, llvm::StringRef TargetID, - llvm::StringMap<bool> *FeatureMap) { - auto OptionalProcessor = - parseTargetIDWithFormatCheckingOnly(TargetID, FeatureMap); - - if (!OptionalProcessor) - return std::nullopt; - - llvm::StringRef Processor = getCanonicalProcessorName(T, *OptionalProcessor); - if (Processor.empty()) - return std::nullopt; - - llvm::SmallSet<llvm::StringRef, 4> AllFeatures( - llvm::from_range, getAllPossibleTargetIDFeatures(T, Processor)); - - for (auto &&F : *FeatureMap) - if (!AllFeatures.count(F.first())) - return std::nullopt; - - return Processor; -} - -// A canonical target ID is a target ID containing a canonical processor name -// and features in alphabetical order. -std::string getCanonicalTargetID(llvm::StringRef Processor, - const llvm::StringMap<bool> &Features) { - std::string TargetID = Processor.str(); - std::map<const llvm::StringRef, bool> OrderedMap; - for (const auto &F : Features) - OrderedMap[F.first()] = F.second; - for (const auto &F : OrderedMap) - TargetID = TargetID + ':' + F.first.str() + (F.second ? "+" : "-"); - return TargetID; + llvm::StringRef OffloadArch) { + auto Split = OffloadArch.split(':'); + if (T.isAMDGPU()) + return llvm::AMDGPU::getCanonicalArchName(T, Split.first); + return Split.first; } // For a specific processor, a feature either shows up in all target IDs, or -// does not show up in any target IDs. Otherwise the target ID combination -// is invalid. +// does not show up in any target IDs. Otherwise the target ID combination is +// invalid. std::optional<std::pair<llvm::StringRef, llvm::StringRef>> -getConflictTargetIDCombination(const std::set<llvm::StringRef> &TargetIDs) { +getConflictTargetIDCombination(const llvm::Triple &T, + const std::set<llvm::StringRef> &TargetIDs) { struct Info { llvm::StringRef TargetID; - llvm::StringMap<bool> Features; - Info(llvm::StringRef TargetID, const llvm::StringMap<bool> &Features) - : TargetID(TargetID), Features(Features) {} + bool HasXnack; + bool HasSramEcc; }; - llvm::StringMap<Info> FeatureMap; - for (auto &&ID : TargetIDs) { - llvm::StringMap<bool> Features; - llvm::StringRef Proc = *parseTargetIDWithFormatCheckingOnly(ID, &Features); - auto [Loc, Inserted] = FeatureMap.try_emplace(Proc, ID, Features); - if (!Inserted) { - auto &ExistingFeatures = Loc->second.Features; - if (llvm::any_of(Features, [&](auto &F) { - return ExistingFeatures.count(F.first()) == 0; - })) - return std::make_pair(Loc->second.TargetID, ID); - } - } - return std::nullopt; -} -bool isCompatibleTargetID(llvm::StringRef Provided, llvm::StringRef Requested) { - llvm::StringMap<bool> ProvidedFeatures, RequestedFeatures; - llvm::StringRef ProvidedProc = - *parseTargetIDWithFormatCheckingOnly(Provided, &ProvidedFeatures); - llvm::StringRef RequestedProc = - *parseTargetIDWithFormatCheckingOnly(Requested, &RequestedFeatures); - if (ProvidedProc != RequestedProc) - return false; - for (const auto &F : ProvidedFeatures) { - auto Loc = RequestedFeatures.find(F.first()); - // The default (unspecified) value of a feature is 'All', which can match - // either 'On' or 'Off'. - if (Loc == RequestedFeatures.end()) - return false; - // If a feature is specified, it must have exact match. - if (Loc->second != F.second) - return false; + llvm::SmallDenseMap<llvm::AMDGPU::GPUKind, Info> Seen; + for (llvm::StringRef ID : TargetIDs) { + std::optional<llvm::AMDGPU::TargetID> Parsed = + llvm::AMDGPU::TargetID::parse(T, ID); + if (!Parsed) + continue; + + // A feature is present in a target ID only when an explicit '+'/'-' + // modifier is given, not when it is left unspecified. + Info Cur{ID, Parsed->isXnackOnOrOff(), Parsed->isSramEccOnOrOff()}; + auto [Loc, Inserted] = Seen.try_emplace(Parsed->getGPUKind(), Cur); + if (Inserted) + continue; + + const Info &Prev = Loc->second; + if (Cur.HasXnack != Prev.HasXnack || Cur.HasSramEcc != Prev.HasSramEcc) + return std::make_pair(Prev.TargetID, ID); } - return true; + return std::nullopt; } std::string sanitizeTargetIDInFileName(llvm::StringRef TargetID) { diff --git a/clang/lib/Basic/Targets/AMDGPU.cpp b/clang/lib/Basic/Targets/AMDGPU.cpp index 3fd9643373383..49bc99a34a5d5 100644 --- a/clang/lib/Basic/Targets/AMDGPU.cpp +++ b/clang/lib/Basic/Targets/AMDGPU.cpp @@ -295,22 +295,25 @@ void AMDGPUTargetInfo::getTargetDefines(const LangOptions &Opts, Twine("__")); Builder.defineMacro("__amdgcn_processor__", Twine("\"") + Twine(CanonName) + Twine("\"")); - Builder.defineMacro( - "__amdgcn_target_id__", - Twine("\"") + - Twine(getCanonicalTargetID(getArchNameAMDGCN(GPUKind), - OffloadArchFeatures)) + - Twine("\"")); - for (auto F : getAllPossibleTargetIDFeatures(getTriple(), CanonName)) { - auto Loc = OffloadArchFeatures.find(F); - if (Loc != OffloadArchFeatures.end()) { - std::string NewF = F.str(); + llvm::AMDGPU::TargetID TargetID(GPUKind, getTriple(), XnackSetting, + SramEccSetting); + Builder.defineMacro("__amdgcn_target_id__", + Twine("\"") + + Twine(TargetID.getCanonicalTargetIDString()) + + Twine("\"")); + auto DefineFeatureMacro = [&](StringRef Feature, + llvm::AMDGPU::TargetIDSetting Setting) { + if (Setting == llvm::AMDGPU::TargetIDSetting::On || + Setting == llvm::AMDGPU::TargetIDSetting::Off) { + std::string NewF = Feature.str(); llvm::replace(NewF, '-', '_'); - Builder.defineMacro(Twine("__amdgcn_feature_") + Twine(NewF) + - Twine("__"), - Loc->second ? "1" : "0"); + Builder.defineMacro( + Twine("__amdgcn_feature_") + Twine(NewF) + Twine("__"), + Setting == llvm::AMDGPU::TargetIDSetting::On ? "1" : "0"); } - } + }; + DefineFeatureMacro("xnack", XnackSetting); + DefineFeatureMacro("sramecc", SramEccSetting); } if (Opts.AtomicIgnoreDenormalMode) diff --git a/clang/lib/Basic/Targets/AMDGPU.h b/clang/lib/Basic/Targets/AMDGPU.h index 89ba561ef302d..b2117542c2edd 100644 --- a/clang/lib/Basic/Targets/AMDGPU.h +++ b/clang/lib/Basic/Targets/AMDGPU.h @@ -42,13 +42,13 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : public TargetInfo { /// Whether having image instructions. bool HasImage = false; - /// Target ID is device name followed by optional feature name postfixed - /// by plus or minus sign delimitted by colon, e.g. gfx908:xnack+:sramecc-. - /// If the target ID contains feature+, map it to true. - /// If the target ID contains feature-, map it to false. - /// If the target ID does not contain a feature (default), do not map it. - llvm::StringMap<bool> OffloadArchFeatures; - std::string TargetID; + /// Explicit xnack/sramecc target-id feature settings from the command line, + /// e.g. gfx908:xnack+:sramecc-. "Unsupported" means the feature was not + /// specified (or is not a valid target-id modifier for the processor). + llvm::AMDGPU::TargetIDSetting XnackSetting = + llvm::AMDGPU::TargetIDSetting::Unsupported; + llvm::AMDGPU::TargetIDSetting SramEccSetting = + llvm::AMDGPU::TargetIDSetting::Unsupported; bool hasFP64() const { return getTriple().isAMDGCN() || @@ -462,8 +462,7 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : public TargetInfo { bool handleTargetFeatures(std::vector<std::string> &Features, DiagnosticsEngine &Diags) override { HasFullBFloat16 = true; - auto TargetIDFeatures = - getAllPossibleTargetIDFeatures(getTriple(), getArchNameAMDGCN(GPUKind)); + unsigned ArchAttr = llvm::AMDGPU::getArchAttrAMDGCN(GPUKind); for (const auto &F : Features) { assert(F.front() == '+' || F.front() == '-'); if (F == "+wavefrontsize64") @@ -474,12 +473,17 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : public TargetInfo { CUMode = false; else if (F == "+image-insts") HasImage = true; - bool IsOn = F.front() == '+'; + llvm::AMDGPU::TargetIDSetting Setting = + F.front() == '+' ? llvm::AMDGPU::TargetIDSetting::On + : llvm::AMDGPU::TargetIDSetting::Off; StringRef Name = StringRef(F).drop_front(); - if (!llvm::is_contained(TargetIDFeatures, Name)) - continue; - assert(!OffloadArchFeatures.contains(Name)); - OffloadArchFeatures[Name] = IsOn; + // xnack is a valid target-id modifier only when the processor supports + // on/off modes; sramecc when the processor supports sramecc. + if (Name == "xnack" && + (ArchAttr & llvm::AMDGPU::FEATURE_XNACK_ON_OFF_MODES)) + XnackSetting = Setting; + else if (Name == "sramecc" && (ArchAttr & llvm::AMDGPU::FEATURE_SRAMECC)) + SramEccSetting = Setting; } return true; } diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index e606cdc4c1cf8..f876adfe8646f 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -108,6 +108,7 @@ #include "llvm/Support/TarWriter.h" #include "llvm/Support/VirtualFileSystem.h" #include "llvm/Support/raw_ostream.h" +#include "llvm/TargetParser/AMDGPUTargetParser.h" #include "llvm/TargetParser/Host.h" #include "llvm/TargetParser/RISCVISAInfo.h" #include <cstdlib> // ::getenv @@ -4851,14 +4852,17 @@ static StringRef getCanonicalArchString(Compilation &C, if (IsNVIDIAOffloadArch(Arch)) return Args.MakeArgStringRef(OffloadArchToString(Arch)); - if (IsAMDOffloadArch(Arch)) { - llvm::StringMap<bool> Features; - std::optional<StringRef> Arch = parseTargetID(Triple, ArchStr, &Features); - if (!Arch) { + // AMDGCN target IDs carry a processor and xnack/sramecc modifiers to + // canonicalize. Other AMD offload arches (e.g. the amdgcnspirv pseudo-arch on + // a SPIR-V triple) have no target-id features and pass through unchanged. + if (IsAMDOffloadArch(Arch) && Triple.isAMDGCN()) { + std::optional<llvm::AMDGPU::TargetID> ID = + llvm::AMDGPU::TargetID::parse(Triple, ArchStr); + if (!ID) { C.getDriver().Diag(clang::diag::err_drv_bad_target_id) << ArchStr; return StringRef(); } - return Args.MakeArgStringRef(getCanonicalTargetID(*Arch, Features)); + return Args.MakeArgStringRef(ID->getCanonicalTargetIDString()); } // If the input isn't CUDA or HIP just return the architecture. @@ -4875,7 +4879,7 @@ getConflictOffloadArchCombination(const llvm::DenseSet<StringRef> &Archs, std::set<StringRef> ArchSet; llvm::copy(Archs, std::inserter(ArchSet, ArchSet.begin())); - return getConflictTargetIDCombination(ArchSet); + return getConflictTargetIDCombination(Triple, ArchSet); } llvm::SmallVector<BoundArch> diff --git a/clang/lib/Driver/OffloadBundler.cpp b/clang/lib/Driver/OffloadBundler.cpp index 8e4d44071ef55..6307b1c4a9704 100644 --- a/clang/lib/Driver/OffloadBundler.cpp +++ b/clang/lib/Driver/OffloadBundler.cpp @@ -16,7 +16,6 @@ #include "clang/Driver/OffloadBundler.h" #include "clang/Basic/OffloadArch.h" -#include "clang/Basic/TargetID.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" @@ -48,6 +47,7 @@ #include "llvm/Support/Timer.h" #include "llvm/Support/WithColor.h" #include "llvm/Support/raw_ostream.h" +#include "llvm/TargetParser/AMDGPUTargetParser.h" #include "llvm/TargetParser/Host.h" #include "llvm/TargetParser/Triple.h" #include <algorithm> @@ -1115,15 +1115,15 @@ bool isCodeObjectCompatible(const OffloadTargetInfo &CodeObjectInfo, } // Incompatible if Processors mismatch. - llvm::StringMap<bool> CodeObjectFeatureMap, TargetFeatureMap; - std::optional<StringRef> CodeObjectProc = clang::parseTargetID( - CodeObjectInfo.Triple, CodeObjectInfo.TargetID, &CodeObjectFeatureMap); - std::optional<StringRef> TargetProc = clang::parseTargetID( - TargetInfo.Triple, TargetInfo.TargetID, &TargetFeatureMap); - - // Both TargetProc and CodeObjectProc can't be empty here. - if (!TargetProc || !CodeObjectProc || - CodeObjectProc.value() != TargetProc.value()) { + std::optional<llvm::AMDGPU::TargetID> CodeObjectID = + llvm::AMDGPU::TargetID::parse(CodeObjectInfo.Triple, + CodeObjectInfo.TargetID); + std::optional<llvm::AMDGPU::TargetID> TargetID = + llvm::AMDGPU::TargetID::parse(TargetInfo.Triple, TargetInfo.TargetID); + + // Both target IDs must be valid and name the same processor. + if (!CodeObjectID || !TargetID || + CodeObjectID->getGPUKind() != TargetID->getGPUKind()) { DEBUG_WITH_TYPE("CodeObjectCompatibility", dbgs() << "Incompatible: Processor mismatch \t[CodeObject: " << CodeObjectInfo.str() @@ -1131,44 +1131,30 @@ bool isCodeObjectCompatible(const OffloadTargetInfo &CodeObjectInfo, return false; } - // Incompatible if CodeObject has more features than Target, irrespective of - // type or sign of features. - if (CodeObjectFeatureMap.getNumItems() > TargetFeatureMap.getNumItems()) { + // A feature (xnack/sramecc) is compatible if the code object leaves it + // unspecified ("Any"), or specifies it with the same value the target does. + // A feature the code object specifies but the target leaves unspecified is + // incompatible, as is a differing explicit value. + auto FeatureCompatible = [&](llvm::AMDGPU::TargetIDSetting CodeObject, + llvm::AMDGPU::TargetIDSetting Target) { + bool CodeObjectExplicit = CodeObject == llvm::AMDGPU::TargetIDSetting::On || + CodeObject == llvm::AMDGPU::TargetIDSetting::Off; + if (!CodeObjectExplicit) + return true; + return CodeObject == Target; + }; + + if (!FeatureCompatible(CodeObjectID->getXnackSetting(), + TargetID->getXnackSetting()) || + !FeatureCompatible(CodeObjectID->getSramEccSetting(), + TargetID->getSramEccSetting())) { DEBUG_WITH_TYPE("CodeObjectCompatibility", - dbgs() << "Incompatible: CodeObject has more features " - "than target \t[CodeObject: " + dbgs() << "Incompatible: Feature mismatch \t[CodeObject: " << CodeObjectInfo.str() << "]\t:\t[Target: " << TargetInfo.str() << "]\n"); return false; } - // Compatible if each target feature specified by target is compatible with - // target feature of code object. The target feature is compatible if the - // code object does not specify it (meaning Any), or if it specifies it - // with the same value (meaning On or Off). - for (const auto &CodeObjectFeature : CodeObjectFeatureMap) { - auto TargetFeature = TargetFeatureMap.find(CodeObjectFeature.getKey()); - if (TargetFeature == TargetFeatureMap.end()) { - DEBUG_WITH_TYPE( - "CodeObjectCompatibility", - dbgs() - << "Incompatible: Value of CodeObject's non-ANY feature is " - "not matching with Target feature's ANY value \t[CodeObject: " - << CodeObjectInfo.str() << "]\t:\t[Target: " << TargetInfo.str() - << "]\n"); - return false; - } else if (TargetFeature->getValue() != CodeObjectFeature.getValue()) { - DEBUG_WITH_TYPE( - "CodeObjectCompatibility", - dbgs() << "Incompatible: Value of CodeObject's non-ANY feature is " - "not matching with Target feature's non-ANY value " - "\t[CodeObject: " - << CodeObjectInfo.str() - << "]\t:\t[Target: " << TargetInfo.str() << "]\n"); - return false; - } - } - // CodeObject is compatible if all features of Target are: // - either, present in the Code Object's features map with the same sign, // - or, the feature is missing from CodeObjects's features map i.e. it is diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp index 5893f6f6b2915..9ab3dafd5c8b7 100644 --- a/clang/lib/Driver/ToolChains/AMDGPU.cpp +++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp @@ -754,26 +754,24 @@ AMDGPUToolChain::TranslateArgs(const DerivedArgList &Args, BoundArch BA, } if (!getTriple().isSPIRV()) { - AMDGPUToolChain::ParsedTargetIDType PTID = checkTargetID(*DAL); - - // Synthesize feature flags for target ID modifiers (xnack, sramecc). - if (PTID.OptionalFeatureMap) { - const llvm::StringMap<bool> &FeatureMap = *PTID.OptionalFeatureMap; - - auto XnackIt = FeatureMap.find("xnack"); - if (XnackIt != FeatureMap.end()) { - DAL->AddFlagArg(nullptr, Opts.getOption(XnackIt->second + std::optional<llvm::AMDGPU::TargetID> PTID = checkTargetID(*DAL); + + // Synthesize feature flags for explicit target ID modifiers (xnack, + // sramecc). + if (PTID) { + using llvm::AMDGPU::TargetIDSetting; + if (PTID->isXnackOnOrOff()) + DAL->AddFlagArg(nullptr, Opts.getOption(PTID->getXnackSetting() == + TargetIDSetting::On ? options::OPT_mxnack : options::OPT_mno_xnack)); - } - auto SrameccIt = FeatureMap.find("sramecc"); - if (SrameccIt != FeatureMap.end()) { - DAL->AddFlagArg(nullptr, - Opts.getOption(SrameccIt->second - ? options::OPT_msramecc - : options::OPT_mno_sramecc)); - } + if (PTID->isSramEccOnOrOff()) + DAL->AddFlagArg( + nullptr, + Opts.getOption(PTID->getSramEccSetting() == TargetIDSetting::On + ? options::OPT_msramecc + : options::OPT_mno_sramecc)); } } @@ -987,28 +985,33 @@ AMDGPUToolChain::getGPUArch(const llvm::opt::ArgList &DriverArgs) const { getTriple(), DriverArgs.getLastArgValue(options::OPT_mcpu_EQ)); } -AMDGPUToolChain::ParsedTargetIDType +StringRef +AMDGPUToolChain::getTargetIDArg(const llvm::opt::ArgList &DriverArgs) const { + // Target IDs are only meaningful for AMDGCN targets. + if (!getTriple().isAMDGCN()) + return StringRef(); + return DriverArgs.getLastArgValue(options::OPT_mcpu_EQ); +} + +std::optional<llvm::AMDGPU::TargetID> AMDGPUToolChain::getParsedTargetID(const llvm::opt::ArgList &DriverArgs) const { - StringRef TargetID = DriverArgs.getLastArgValue(options::OPT_mcpu_EQ); + StringRef TargetID = getTargetIDArg(DriverArgs); if (TargetID.empty()) - return {}; - - llvm::StringMap<bool> FeatureMap; - auto OptionalGpuArch = parseTargetID(getTriple(), TargetID, &FeatureMap); - if (!OptionalGpuArch) - return {TargetID.str(), std::nullopt, std::nullopt}; + return std::nullopt; - return {TargetID.str(), OptionalGpuArch->str(), FeatureMap}; + return llvm::AMDGPU::TargetID::parse(getTriple(), TargetID); } -AMDGPUToolChain::ParsedTargetIDType +std::optional<llvm::AMDGPU::TargetID> AMDGPUToolChain::checkTargetID(const llvm::opt::ArgList &DriverArgs) const { - auto PTID = getParsedTargetID(DriverArgs); - if (PTID.OptionalTargetID && !PTID.OptionalGPUArch) { - getDriver().Diag(clang::diag::err_drv_bad_target_id) - << *PTID.OptionalTargetID; + std::optional<llvm::AMDGPU::TargetID> ID = getParsedTargetID(DriverArgs); + // Diagnose a non-empty but invalid target ID. + if (!ID) { + StringRef TargetID = getTargetIDArg(DriverArgs); + if (!TargetID.empty()) + getDriver().Diag(clang::diag::err_drv_bad_target_id) << TargetID; } - return PTID; + return ID; } Expected<SmallVector<std::string>> @@ -1288,26 +1291,21 @@ LTOKind AMDGPUToolChain::getLTOMode(const ArgList &Args, } static bool isXnackAvailable(const llvm::Triple &TT, llvm::StringRef TargetID) { - // Arch-specific check - only report as supported if arch has xnack+ - llvm::StringRef Processor = getProcessorFromTargetID(TT, TargetID); - auto ProcKind = TT.isAMDGCN() ? llvm::AMDGPU::parseArchAMDGCN(Processor) - : llvm::AMDGPU::parseArchR600(Processor); - auto Features = TT.isAMDGCN() ? llvm::AMDGPU::getArchAttrAMDGCN(ProcKind) - : llvm::AMDGPU::getArchAttrR600(ProcKind); - - // If processor has xnack but doesn't support on/off modes, xnack is always on - bool XnackAlwaysOn = (Features & llvm::AMDGPU::FEATURE_XNACK) && - !(Features & llvm::AMDGPU::FEATURE_XNACK_ON_OFF_MODES); - if (XnackAlwaysOn) + std::optional<llvm::AMDGPU::TargetID> ID = + llvm::AMDGPU::TargetID::parse(TT, TargetID); + if (!ID) + return false; + + unsigned Features = llvm::AMDGPU::getArchAttrAMDGCN(ID->getGPUKind()); + + // If the processor has xnack but doesn't support on/off modes, xnack is + // always on. + if ((Features & llvm::AMDGPU::FEATURE_XNACK) && + !(Features & llvm::AMDGPU::FEATURE_XNACK_ON_OFF_MODES)) return true; - // Otherwise, check if xnack+ is explicitly enabled in the target ID - llvm::StringMap<bool> FeatureMap; - auto OptionalGpuArch = parseTargetID(TT, TargetID, &FeatureMap); - if (!OptionalGpuArch) - return false; - auto Loc = FeatureMap.find("xnack"); - return (Loc != FeatureMap.end() && Loc->second); + // Otherwise, it is available only if the target ID explicitly enables it. + return ID->getXnackSetting() == llvm::AMDGPU::TargetIDSetting::On; } SanitizerMask AMDGPUToolChain::getSupportedSanitizers( diff --git a/clang/lib/Driver/ToolChains/AMDGPU.h b/clang/lib/Driver/ToolChains/AMDGPU.h index 229c077a3943f..cb0d4e7096b8e 100644 --- a/clang/lib/Driver/ToolChains/AMDGPU.h +++ b/clang/lib/Driver/ToolChains/AMDGPU.h @@ -160,23 +160,20 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUToolChain : public Generic_ELF { Action::OffloadKind DeviceOffloadingKind) const; protected: - /// The struct type returned by getParsedTargetID. - struct ParsedTargetIDType { - std::optional<std::string> OptionalTargetID; - std::optional<std::string> OptionalGPUArch; - std::optional<llvm::StringMap<bool>> OptionalFeatureMap; - }; - - /// Check and diagnose invalid target ID specified by -mcpu. - /// Returns the parsed target ID. - virtual ParsedTargetIDType + /// Check and diagnose an invalid target ID specified by -mcpu. Returns the + /// parsed target ID, or std::nullopt if -mcpu is absent or invalid + virtual std::optional<llvm::AMDGPU::TargetID> checkTargetID(const llvm::opt::ArgList &DriverArgs) const; - /// Get target ID, GPU arch, and target ID features if the target ID is - /// specified and valid. - ParsedTargetIDType + /// Parse the target ID specified by -mcpu. Returns the parsed target ID, or + /// std::nullopt if -mcpu is absent or invalid. + std::optional<llvm::AMDGPU::TargetID> getParsedTargetID(const llvm::opt::ArgList &DriverArgs) const; + /// Get the raw target ID string from -mcpu, or an empty string if -mcpu is + /// absent or the target is not AMDGCN. + StringRef getTargetIDArg(const llvm::opt::ArgList &DriverArgs) const; + /// Get GPU arch from -mcpu without checking. StringRef getGPUArch(const llvm::opt::ArgList &DriverArgs) const; diff --git a/llvm/include/llvm/TargetParser/AMDGPUTargetParser.h b/llvm/include/llvm/TargetParser/AMDGPUTargetParser.h index 9a7ee2e03ae4c..d1e68fbbdc215 100644 --- a/llvm/include/llvm/TargetParser/AMDGPUTargetParser.h +++ b/llvm/include/llvm/TargetParser/AMDGPUTargetParser.h @@ -268,7 +268,7 @@ class LLVM_ABI TargetID { /// \returns the canonical processor name followed by any explicit xnack and /// sramecc feature modifiers order (e.g. "gfx908:sramecc-:xnack+"), without /// the triple prefix. - std::string getCanonicalFeatureString() const; + std::string getCanonicalTargetIDString() const; bool operator==(const TargetID &Other) const; bool operator!=(const TargetID &Other) const { return !(*this == Other); } diff --git a/llvm/lib/TargetParser/AMDGPUTargetParser.cpp b/llvm/lib/TargetParser/AMDGPUTargetParser.cpp index 630a466fa3edc..170125afa76e2 100644 --- a/llvm/lib/TargetParser/AMDGPUTargetParser.cpp +++ b/llvm/lib/TargetParser/AMDGPUTargetParser.cpp @@ -1099,7 +1099,7 @@ void TargetID::printCanonicalTargetIDString(raw_ostream &OS) const { printFeatureModifiers(OS, getSramEccSetting(), getXnackSetting()); } -std::string TargetID::getCanonicalFeatureString() const { +std::string TargetID::getCanonicalTargetIDString() const { std::string Str; raw_string_ostream OS(Str); printCanonicalTargetIDString(OS); diff --git a/llvm/unittests/TargetParser/TargetParserTest.cpp b/llvm/unittests/TargetParser/TargetParserTest.cpp index 2f5436a1d7f9e..2b7fe26fe8bf3 100644 --- a/llvm/unittests/TargetParser/TargetParserTest.cpp +++ b/llvm/unittests/TargetParser/TargetParserTest.cpp @@ -2921,12 +2921,12 @@ TEST(TargetParserTest, testAMDGPUParseTargetIDString) { } EXPECT_EQ(TargetID::parse(AMDHSA, "gfx908:xnack+:sramecc-") - ->getCanonicalFeatureString(), + ->getCanonicalTargetIDString(), "gfx908:sramecc-:xnack+"); - EXPECT_EQ(TargetID::parse(AMDHSA, "gfx908")->getCanonicalFeatureString(), + EXPECT_EQ(TargetID::parse(AMDHSA, "gfx908")->getCanonicalTargetIDString(), "gfx908"); EXPECT_EQ(TargetID::parse(Triple("amdgcn-amd-amdpal"), "gfx908:xnack-") - ->getCanonicalFeatureString(), + ->getCanonicalTargetIDString(), "gfx908:xnack-"); EXPECT_TRUE(TargetID::parse(AMDHSA, "").has_value()); EXPECT_FALSE(TargetID::parse(AMDHSA, "gfxbogus").has_value()); >From 41cde563fc7521d42882b5c09dbc7427d2297f66 Mon Sep 17 00:00:00 2001 From: Matt Arsenault <[email protected]> Date: Wed, 15 Jul 2026 18:08:04 +0200 Subject: [PATCH 2/3] Fix offload bundler usage of getConflictTargetIDCombination --- clang/include/clang/Basic/TargetID.h | 20 ++++++++-------- clang/lib/Basic/TargetID.cpp | 5 ++-- clang/lib/Driver/Driver.cpp | 13 +++++++---- clang/lib/Driver/OffloadBundler.cpp | 14 +++++++++-- .../ClangOffloadBundler.cpp | 23 +++++++++++-------- 5 files changed, 48 insertions(+), 27 deletions(-) diff --git a/clang/include/clang/Basic/TargetID.h b/clang/include/clang/Basic/TargetID.h index 5625f1df45142..8871b76859fd7 100644 --- a/clang/include/clang/Basic/TargetID.h +++ b/clang/include/clang/Basic/TargetID.h @@ -9,13 +9,11 @@ #ifndef LLVM_CLANG_BASIC_TARGETID_H #define LLVM_CLANG_BASIC_TARGETID_H +#include "llvm/ADT/ArrayRef.h" +#include "llvm/TargetParser/Triple.h" #include <optional> -#include <set> #include <string> - -namespace llvm { -class Triple; -} +#include <utility> namespace clang { @@ -24,12 +22,16 @@ namespace clang { llvm::StringRef getProcessorFromTargetID(const llvm::Triple &T, llvm::StringRef OffloadArch); +/// A device triple paired with a target ID (processor and feature modifiers) +/// for that triple, e.g. {amdgcn-amd-amdhsa, "gfx906:xnack+"}. +using TargetIDEntry = std::pair<const llvm::Triple &, llvm::StringRef>; + /// Get the conflicted pair of target IDs for a compilation or a bundled code -/// object, assuming \p TargetIDs are canonicalized. If there is no conflicts, -/// returns std::nullopt. +/// object. Two entries conflict when they resolve to the same processor but +/// disagree on whether a feature (xnack/sramecc) is explicitly specified. If +/// there is no conflict, returns std::nullopt. std::optional<std::pair<llvm::StringRef, llvm::StringRef>> -getConflictTargetIDCombination(const llvm::Triple &T, - const std::set<llvm::StringRef> &TargetIDs); +getConflictTargetIDCombination(llvm::ArrayRef<TargetIDEntry> Entries); /// Sanitize a target ID string for use in a file name. /// Replaces invalid characters (like ':') with safe characters (like '@'). diff --git a/clang/lib/Basic/TargetID.cpp b/clang/lib/Basic/TargetID.cpp index 68064558f64c9..a179d26645c5b 100644 --- a/clang/lib/Basic/TargetID.cpp +++ b/clang/lib/Basic/TargetID.cpp @@ -24,8 +24,7 @@ llvm::StringRef getProcessorFromTargetID(const llvm::Triple &T, // does not show up in any target IDs. Otherwise the target ID combination is // invalid. std::optional<std::pair<llvm::StringRef, llvm::StringRef>> -getConflictTargetIDCombination(const llvm::Triple &T, - const std::set<llvm::StringRef> &TargetIDs) { +getConflictTargetIDCombination(llvm::ArrayRef<TargetIDEntry> Entries) { struct Info { llvm::StringRef TargetID; bool HasXnack; @@ -33,7 +32,7 @@ getConflictTargetIDCombination(const llvm::Triple &T, }; llvm::SmallDenseMap<llvm::AMDGPU::GPUKind, Info> Seen; - for (llvm::StringRef ID : TargetIDs) { + for (const auto &[T, ID] : Entries) { std::optional<llvm::AMDGPU::TargetID> Parsed = llvm::AMDGPU::TargetID::parse(T, ID); if (!Parsed) diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index f876adfe8646f..b0ebfc9e99c53 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -4873,13 +4873,18 @@ static StringRef getCanonicalArchString(Compilation &C, /// incompatible pair if a conflict occurs. static std::optional<std::pair<llvm::StringRef, llvm::StringRef>> getConflictOffloadArchCombination(const llvm::DenseSet<StringRef> &Archs, - llvm::Triple Triple) { + const llvm::Triple &Triple) { if (!Triple.isAMDGPU()) return std::nullopt; - std::set<StringRef> ArchSet; - llvm::copy(Archs, std::inserter(ArchSet, ArchSet.begin())); - return getConflictTargetIDCombination(Triple, ArchSet); + // Sort for a deterministic conflicting pair in the diagnostic. + llvm::SmallVector<StringRef> ArchList(Archs.begin(), Archs.end()); + llvm::sort(ArchList); + + llvm::SmallVector<clang::TargetIDEntry> Entries; + for (StringRef Arch : ArchList) + Entries.emplace_back(Triple, Arch); + return getConflictTargetIDCombination(Entries); } llvm::SmallVector<BoundArch> diff --git a/clang/lib/Driver/OffloadBundler.cpp b/clang/lib/Driver/OffloadBundler.cpp index 6307b1c4a9704..41c09a6dec5ed 100644 --- a/clang/lib/Driver/OffloadBundler.cpp +++ b/clang/lib/Driver/OffloadBundler.cpp @@ -16,6 +16,7 @@ #include "clang/Driver/OffloadBundler.h" #include "clang/Basic/OffloadArch.h" +#include "clang/Basic/TargetID.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" @@ -1523,8 +1524,17 @@ CheckHeterogeneousArchive(StringRef ArchiveName, if (CodeObjectFileError) return CodeObjectFileError; - auto &&ConflictingArchs = clang::getConflictTargetIDCombination(BundleIds); - if (ConflictingArchs) { + // A single bundle may contain several triples. Pair each target ID with its + // own triple; the conflict check groups by resolved processor, which is + // spelling-independent. + llvm::SmallVector<clang::TargetIDEntry> Entries; + for (StringRef BundleId : BundleIds) { + OffloadTargetInfo Info(BundleId, BundlerConfig); + Entries.emplace_back(Info.Triple, Info.TargetID); + } + + if (auto &&ConflictingArchs = + clang::getConflictTargetIDCombination(Entries)) { std::string ErrMsg = Twine("conflicting TargetIDs [" + ConflictingArchs.value().first + ", " + ConflictingArchs.value().second + "] found in " + diff --git a/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp b/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp index 40d77abe2ef7c..72ead7c0b34db 100644 --- a/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp +++ b/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp @@ -349,8 +349,8 @@ int main(int argc, const char **argv) { unsigned HostTargetNum = 0u; bool HIPOnly = true; llvm::DenseSet<StringRef> ParsedTargets; - // Map {offload-kind}-{triple} to target IDs. - std::map<std::string, std::set<StringRef>> TargetIDs; + // Map {offload-kind}-{triple} to its device triple and target IDs. + std::map<std::string, std::pair<llvm::Triple, std::set<StringRef>>> TargetIDs; // Standardize target names to include env field std::vector<std::string> StandardizedTargetNames; for (StringRef Target : TargetNames) { @@ -385,8 +385,10 @@ int main(int argc, const char **argv) { return reportError(createStringError(errc::invalid_argument, Msg.str())); } - TargetIDs[OffloadInfo.OffloadKind.str() + "-" + OffloadInfo.Triple.str()] - .insert(OffloadInfo.TargetID); + auto &Entry = TargetIDs[OffloadInfo.OffloadKind.str() + "-" + + OffloadInfo.Triple.str()]; + Entry.first = OffloadInfo.Triple; + Entry.second.insert(OffloadInfo.TargetID); if (KindIsValid && OffloadInfo.hasHostKind()) { ++HostTargetNum; // Save the index of the input that refers to the host. @@ -402,14 +404,17 @@ int main(int argc, const char **argv) { BundlerConfig.TargetNames.assign(StandardizedTargetNames.begin(), StandardizedTargetNames.end()); - for (const auto &TargetID : TargetIDs) { - if (auto ConflictingTID = - clang::getConflictTargetIDCombination(TargetID.second)) { + for (const auto &[Key, TripleAndIDs] : TargetIDs) { + const auto &[Triple, IDs] = TripleAndIDs; + llvm::SmallVector<clang::TargetIDEntry> Entries; + for (StringRef ID : IDs) + Entries.emplace_back(Triple, ID); + if (auto ConflictingTID = clang::getConflictTargetIDCombination(Entries)) { SmallVector<char, 128u> Buf; raw_svector_ostream Msg(Buf); Msg << "Cannot bundle inputs with conflicting targets: '" - << TargetID.first + "-" + ConflictingTID->first << "' and '" - << TargetID.first + "-" + ConflictingTID->second << "'"; + << Key + "-" + ConflictingTID->first << "' and '" + << Key + "-" + ConflictingTID->second << "'"; return reportError(createStringError(errc::invalid_argument, Msg.str())); } } >From aaa9236cac2bab745f5accb0d042c4646ee8c5ba Mon Sep 17 00:00:00 2001 From: Matt Arsenault <[email protected]> Date: Fri, 17 Jul 2026 13:55:50 +0200 Subject: [PATCH 3/3] clang: Add missing DenseMap.h include to TargetID.cpp Co-authored-by: Claude (Claude Opus 4.8) <[email protected]> --- clang/lib/Basic/TargetID.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/lib/Basic/TargetID.cpp b/clang/lib/Basic/TargetID.cpp index a179d26645c5b..d2e1228897ceb 100644 --- a/clang/lib/Basic/TargetID.cpp +++ b/clang/lib/Basic/TargetID.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "clang/Basic/TargetID.h" +#include "llvm/ADT/DenseMap.h" #include "llvm/Support/Path.h" #include "llvm/TargetParser/AMDGPUTargetParser.h" _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
