https://github.com/Kewen12 created https://github.com/llvm/llvm-project/pull/213824
Reverts llvm/llvm-project#209845 verified by local reverting unblock bots: https://lab.llvm.org/buildbot/#/builders/234/builds/1391 https://lab.llvm.org/buildbot/#/builders/10/builds/33193 >From 5f0650a82c36858ca43bc36b8278a3a10ea9af70 Mon Sep 17 00:00:00 2001 From: Kewen Meng <[email protected]> Date: Mon, 3 Aug 2026 20:00:19 -0700 Subject: [PATCH] Revert "clang: Use TargetID parsing from AMDGPUTargetParser (#209845)" This reverts commit a56d758e794208f029f88d274f12a5750c877a6f. --- clang/include/clang/Basic/TargetID.h | 44 +++- clang/lib/Basic/TargetID.cpp | 194 +++++++++++++++--- clang/lib/Basic/Targets/AMDGPU.cpp | 43 ++-- clang/lib/Basic/Targets/AMDGPU.h | 32 ++- clang/lib/Driver/Driver.cpp | 27 +-- clang/lib/Driver/OffloadBundler.cpp | 83 ++++---- clang/lib/Driver/ToolChains/AMDGPU.cpp | 96 ++++----- clang/lib/Driver/ToolChains/AMDGPU.h | 23 ++- .../clang-offload-bundler/basic.c | 11 - .../ClangOffloadBundler.cpp | 23 +-- .../llvm/TargetParser/AMDGPUTargetParser.h | 2 +- llvm/lib/TargetParser/AMDGPUTargetParser.cpp | 2 +- .../TargetParser/TargetParserTest.cpp | 6 +- 13 files changed, 359 insertions(+), 227 deletions(-) diff --git a/clang/include/clang/Basic/TargetID.h b/clang/include/clang/Basic/TargetID.h index 8871b76859fd7..902151d76556d 100644 --- a/clang/include/clang/Basic/TargetID.h +++ b/clang/include/clang/Basic/TargetID.h @@ -9,29 +9,53 @@ #ifndef LLVM_CLANG_BASIC_TARGETID_H #define LLVM_CLANG_BASIC_TARGETID_H -#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/StringMap.h" #include "llvm/TargetParser/Triple.h" #include <optional> -#include <string> -#include <utility> +#include <set> namespace clang { +/// 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); + /// 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); -/// 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>; +/// 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. 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. +/// object, assuming \p TargetIDs are canonicalized. If there is no conflicts, +/// returns std::nullopt. std::optional<std::pair<llvm::StringRef, llvm::StringRef>> -getConflictTargetIDCombination(llvm::ArrayRef<TargetIDEntry> Entries); +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); /// 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 d2e1228897ceb..29d5d4a5d2996 100644 --- a/clang/lib/Basic/TargetID.cpp +++ b/clang/lib/Basic/TargetID.cpp @@ -7,52 +7,188 @@ //===----------------------------------------------------------------------===// #include "clang/Basic/TargetID.h" -#include "llvm/ADT/DenseMap.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 { -llvm::StringRef getProcessorFromTargetID(const llvm::Triple &T, - llvm::StringRef OffloadArch) { - auto Split = OffloadArch.split(':'); +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; + if (!T.isAMDGCN()) + return Ret; + llvm::AMDGPU::GPUKind ProcKind = llvm::AMDGPU::parseArchAMDGCN(Proc); + if (ProcKind == llvm::AMDGPU::GK_NONE) + return Ret; + unsigned Features = llvm::AMDGPU::getArchAttrAMDGCN(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, Split.first); - return Split.first; + 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; } // 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(llvm::ArrayRef<TargetIDEntry> Entries) { +getConflictTargetIDCombination(const std::set<llvm::StringRef> &TargetIDs) { struct Info { llvm::StringRef TargetID; - bool HasXnack; - bool HasSramEcc; + llvm::StringMap<bool> Features; + Info(llvm::StringRef TargetID, const llvm::StringMap<bool> &Features) + : TargetID(TargetID), Features(Features) {} }; - - llvm::SmallDenseMap<llvm::AMDGPU::GPUKind, Info> Seen; - for (const auto &[T, ID] : Entries) { - 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); + 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; + } + return true; +} + std::string sanitizeTargetIDInFileName(llvm::StringRef TargetID) { std::string FileName = TargetID.str(); if (llvm::sys::path::is_style_windows(llvm::sys::path::Style::native)) diff --git a/clang/lib/Basic/Targets/AMDGPU.cpp b/clang/lib/Basic/Targets/AMDGPU.cpp index c6efd02d912e9..4109066ec910e 100644 --- a/clang/lib/Basic/Targets/AMDGPU.cpp +++ b/clang/lib/Basic/Targets/AMDGPU.cpp @@ -236,17 +236,13 @@ AMDGPUTargetInfo::AMDGPUTargetInfo(const llvm::Triple &Triple, HalfArgsAndReturns = true; if (Opts.AMDGPUXnackState != TargetOptions::AMDGPUFeatureState::Any) { - XnackSetting = - Opts.AMDGPUXnackState == TargetOptions::AMDGPUFeatureState::Enabled - ? llvm::AMDGPU::TargetIDSetting::On - : llvm::AMDGPU::TargetIDSetting::Off; + OffloadArchFeatures["xnack"] = + Opts.AMDGPUXnackState == TargetOptions::AMDGPUFeatureState::Enabled; } if (Opts.AMDGPUSramEccState != TargetOptions::AMDGPUFeatureState::Any) { - SramEccSetting = - Opts.AMDGPUSramEccState == TargetOptions::AMDGPUFeatureState::Enabled - ? llvm::AMDGPU::TargetIDSetting::On - : llvm::AMDGPU::TargetIDSetting::Off; + OffloadArchFeatures["sramecc"] = + Opts.AMDGPUSramEccState == TargetOptions::AMDGPUFeatureState::Enabled; } } @@ -311,25 +307,22 @@ void AMDGPUTargetInfo::getTargetDefines(const LangOptions &Opts, Twine("__")); Builder.defineMacro("__amdgcn_processor__", Twine("\"") + Twine(CanonName) + Twine("\"")); - 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(); + 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::replace(NewF, '-', '_'); - Builder.defineMacro( - Twine("__amdgcn_feature_") + Twine(NewF) + Twine("__"), - Setting == llvm::AMDGPU::TargetIDSetting::On ? "1" : "0"); + Builder.defineMacro(Twine("__amdgcn_feature_") + Twine(NewF) + + Twine("__"), + Loc->second ? "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 61c87456f581f..f8933ebee8ffd 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; - /// 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; + /// 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; bool hasFP64() const { return getTriple().isAMDGCN(); } @@ -461,7 +461,8 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : public TargetInfo { bool handleTargetFeatures(std::vector<std::string> &Features, DiagnosticsEngine &Diags) override { HasFullBFloat16 = true; - unsigned ArchAttr = llvm::AMDGPU::getArchAttrAMDGCN(GPUKind); + auto TargetIDFeatures = + getAllPossibleTargetIDFeatures(getTriple(), getArchNameAMDGCN(GPUKind)); for (const auto &F : Features) { assert(F.front() == '+' || F.front() == '-'); if (F == "+wavefrontsize64") @@ -472,17 +473,12 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : public TargetInfo { CUMode = false; else if (F == "+image-insts") HasImage = true; - llvm::AMDGPU::TargetIDSetting Setting = - F.front() == '+' ? llvm::AMDGPU::TargetIDSetting::On - : llvm::AMDGPU::TargetIDSetting::Off; + bool IsOn = F.front() == '+'; StringRef Name = StringRef(F).drop_front(); - // 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; + if (!llvm::is_contained(TargetIDFeatures, Name)) + continue; + assert(!OffloadArchFeatures.contains(Name)); + OffloadArchFeatures[Name] = IsOn; } return true; } diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index b4f114b5b9f7a..5bd46db170d96 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -108,7 +108,6 @@ #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 @@ -4896,17 +4895,14 @@ static StringRef getCanonicalArchString(Compilation &C, if (IsNVIDIAOffloadArch(Arch)) return Args.MakeArgStringRef(OffloadArchToString(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) { + if (IsAMDOffloadArch(Arch)) { + llvm::StringMap<bool> Features; + std::optional<StringRef> Arch = parseTargetID(Triple, ArchStr, &Features); + if (!Arch) { C.getDriver().Diag(clang::diag::err_drv_bad_target_id) << ArchStr; return StringRef(); } - return Args.MakeArgStringRef(ID->getCanonicalTargetIDString()); + return Args.MakeArgStringRef(getCanonicalTargetID(*Arch, Features)); } // If the input isn't CUDA or HIP just return the architecture. @@ -4917,18 +4913,13 @@ 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, - const llvm::Triple &Triple) { + llvm::Triple Triple) { if (!Triple.isAMDGPU()) return std::nullopt; - // 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); + std::set<StringRef> ArchSet; + llvm::copy(Archs, std::inserter(ArchSet, ArchSet.begin())); + return getConflictTargetIDCombination(ArchSet); } llvm::SmallVector<BoundArch> diff --git a/clang/lib/Driver/OffloadBundler.cpp b/clang/lib/Driver/OffloadBundler.cpp index a356b5f84c51e..b397ee4c5b075 100644 --- a/clang/lib/Driver/OffloadBundler.cpp +++ b/clang/lib/Driver/OffloadBundler.cpp @@ -48,7 +48,6 @@ #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 +1114,15 @@ bool isCodeObjectCompatible(const OffloadTargetInfo &CodeObjectInfo, } // Incompatible if Processors mismatch. - 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()) { + 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()) { DEBUG_WITH_TYPE("CodeObjectCompatibility", dbgs() << "Incompatible: Processor mismatch \t[CodeObject: " << CodeObjectInfo.str() @@ -1131,30 +1130,44 @@ bool isCodeObjectCompatible(const OffloadTargetInfo &CodeObjectInfo, return false; } - // 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())) { + // Incompatible if CodeObject has more features than Target, irrespective of + // type or sign of features. + if (CodeObjectFeatureMap.getNumItems() > TargetFeatureMap.getNumItems()) { DEBUG_WITH_TYPE("CodeObjectCompatibility", - dbgs() << "Incompatible: Feature mismatch \t[CodeObject: " + dbgs() << "Incompatible: CodeObject has more features " + "than target \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 @@ -1523,18 +1536,8 @@ CheckHeterogeneousArchive(StringRef ArchiveName, if (CodeObjectFileError) return CodeObjectFileError; - // 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<OffloadTargetInfo> Infos; - for (StringRef BundleId : BundleIds) - Infos.emplace_back(BundleId, BundlerConfig); - llvm::SmallVector<clang::TargetIDEntry> Entries; - for (const OffloadTargetInfo &Info : Infos) - Entries.emplace_back(Info.Triple, Info.TargetID); - - if (auto &&ConflictingArchs = - clang::getConflictTargetIDCombination(Entries)) { + auto &&ConflictingArchs = clang::getConflictTargetIDCombination(BundleIds); + if (ConflictingArchs) { std::string ErrMsg = Twine("conflicting TargetIDs [" + ConflictingArchs.value().first + ", " + ConflictingArchs.value().second + "] found in " + diff --git a/clang/lib/Driver/ToolChains/AMDGPU.cpp b/clang/lib/Driver/ToolChains/AMDGPU.cpp index 4f2e6278a10b0..7bce060de0596 100644 --- a/clang/lib/Driver/ToolChains/AMDGPU.cpp +++ b/clang/lib/Driver/ToolChains/AMDGPU.cpp @@ -771,24 +771,26 @@ AMDGPUToolChain::TranslateArgs(const DerivedArgList &Args, BoundArch BA, } if (!getTriple().isSPIRV()) { - 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 + 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 ? options::OPT_mxnack : options::OPT_mno_xnack)); + } - if (PTID->isSramEccOnOrOff()) - DAL->AddFlagArg( - nullptr, - Opts.getOption(PTID->getSramEccSetting() == TargetIDSetting::On - ? options::OPT_msramecc - : options::OPT_mno_sramecc)); + auto SrameccIt = FeatureMap.find("sramecc"); + if (SrameccIt != FeatureMap.end()) { + DAL->AddFlagArg(nullptr, + Opts.getOption(SrameccIt->second + ? options::OPT_msramecc + : options::OPT_mno_sramecc)); + } } } @@ -1002,33 +1004,28 @@ AMDGPUToolChain::getGPUArch(const llvm::opt::ArgList &DriverArgs) const { getTriple(), DriverArgs.getLastArgValue(options::OPT_mcpu_EQ)); } -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::ParsedTargetIDType AMDGPUToolChain::getParsedTargetID(const llvm::opt::ArgList &DriverArgs) const { - StringRef TargetID = getTargetIDArg(DriverArgs); + StringRef TargetID = DriverArgs.getLastArgValue(options::OPT_mcpu_EQ); if (TargetID.empty()) - return std::nullopt; + return {}; + + llvm::StringMap<bool> FeatureMap; + auto OptionalGpuArch = parseTargetID(getTriple(), TargetID, &FeatureMap); + if (!OptionalGpuArch) + return {TargetID.str(), std::nullopt, std::nullopt}; - return llvm::AMDGPU::TargetID::parse(getTriple(), TargetID); + return {TargetID.str(), OptionalGpuArch->str(), FeatureMap}; } -std::optional<llvm::AMDGPU::TargetID> +AMDGPUToolChain::ParsedTargetIDType AMDGPUToolChain::checkTargetID(const llvm::opt::ArgList &DriverArgs) const { - 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; + auto PTID = getParsedTargetID(DriverArgs); + if (PTID.OptionalTargetID && !PTID.OptionalGPUArch) { + getDriver().Diag(clang::diag::err_drv_bad_target_id) + << *PTID.OptionalTargetID; } - return ID; + return PTID; } Expected<SmallVector<std::string>> @@ -1308,21 +1305,26 @@ LTOKind AMDGPUToolChain::getLTOMode(const ArgList &Args, } static bool isXnackAvailable(const llvm::Triple &TT, llvm::StringRef TargetID) { - std::optional<llvm::AMDGPU::TargetID> ID = - llvm::AMDGPU::TargetID::parse(TT, TargetID); - if (!ID) + // Arch-specific check - only report as supported if arch has xnack+ + if (!TT.isAMDGCN()) 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)) + llvm::StringRef Processor = getProcessorFromTargetID(TT, TargetID); + llvm::AMDGPU::GPUKind ProcKind = llvm::AMDGPU::parseArchAMDGCN(Processor); + unsigned Features = llvm::AMDGPU::getArchAttrAMDGCN(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) return true; - // Otherwise, it is available only if the target ID explicitly enables it. - return ID->getXnackSetting() == llvm::AMDGPU::TargetIDSetting::On; + // 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); } SanitizerMask AMDGPUToolChain::getSupportedSanitizers( diff --git a/clang/lib/Driver/ToolChains/AMDGPU.h b/clang/lib/Driver/ToolChains/AMDGPU.h index fd71b53064d3e..027a6e3b47dca 100644 --- a/clang/lib/Driver/ToolChains/AMDGPU.h +++ b/clang/lib/Driver/ToolChains/AMDGPU.h @@ -161,20 +161,23 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUToolChain : public Generic_ELF { Action::OffloadKind DeviceOffloadingKind) const; protected: - /// 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> + /// 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 checkTargetID(const llvm::opt::ArgList &DriverArgs) const; - /// 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> + /// Get target ID, GPU arch, and target ID features if the target ID is + /// specified and valid. + ParsedTargetIDType 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/clang/test/OffloadTools/clang-offload-bundler/basic.c b/clang/test/OffloadTools/clang-offload-bundler/basic.c index bd2ca595c4a7e..b10c9cde08921 100644 --- a/clang/test/OffloadTools/clang-offload-bundler/basic.c +++ b/clang/test/OffloadTools/clang-offload-bundler/basic.c @@ -535,17 +535,6 @@ // RUN: not clang-offload-bundler -type=o -targets=host-x86_64-unknown-linux-gnu,openmp-amdgpu9.06-amd-amdhsa--gfx906,openmp-amdgpu9.06-amd-amdhsa--gfx906:sramecc+ -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.bad.bundle 2>&1 | FileCheck %s -check-prefix=BADTARGETS // BADTARGETS: error: Cannot bundle inputs with conflicting targets: 'openmp-amdgpu9.06-amd-amdhsa--gfx906' and 'openmp-amdgpu9.06-amd-amdhsa--gfx906:sramecc+' -// Check the per-member TargetID conflict detection performed by -// -check-input-archive. The bundle-time conflict check groups by offload kind -// and triple, so "gfx906" and "gfx906:xnack+" placed under different offload -// kinds (hip vs hipv4) bundle successfully. The archive check instead groups by -// resolved processor and must flag them as conflicting for the same gfx906. - -// RUN: clang-offload-bundler -type=o -targets=host-x86_64-unknown-linux-gnu,hip-amdgcn-amd-amdhsa--gfx906,hipv4-amdgcn-amd-amdhsa--gfx906:xnack+ -input=%t.o -input=%t.tgt1 -input=%t.tgt2 -output=%t.conflict.bundle -// RUN: llvm-ar cr %t.conflict-archive.a %t.conflict.bundle -// RUN: not clang-offload-bundler -unbundle -type=a -check-input-archive -targets=hip-amdgcn-amd-amdhsa--gfx906 -input=%t.conflict-archive.a -output=%t.conflict-out.a 2>&1 | FileCheck %s -check-prefix=CONFLICTARCHIVE -// CONFLICTARCHIVE: error: conflicting TargetIDs [gfx906, gfx906:xnack+] found in {{.*}}conflict.bundle of {{.*}}conflict-archive.a - // Check for error if no compatible code object is found in the heterogeneous archive library // RUN: not clang-offload-bundler -unbundle -type=a -targets=openmp-amdgpu8.03-amd-amdhsa--gfx803 -input=%t.input-archive.a -output=%t-archive-gfx803-incompatible.a 2>&1 | FileCheck %s -check-prefix=INCOMPATIBLEARCHIVE // INCOMPATIBLEARCHIVE: error: no compatible code object found for the target 'openmp-amdgpu8.03-amd-amdhsa--gfx803' in heterogeneous archive library diff --git a/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp b/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp index 72ead7c0b34db..40d77abe2ef7c 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 its device triple and target IDs. - std::map<std::string, std::pair<llvm::Triple, std::set<StringRef>>> TargetIDs; + // Map {offload-kind}-{triple} to target IDs. + std::map<std::string, std::set<StringRef>> TargetIDs; // Standardize target names to include env field std::vector<std::string> StandardizedTargetNames; for (StringRef Target : TargetNames) { @@ -385,10 +385,8 @@ int main(int argc, const char **argv) { return reportError(createStringError(errc::invalid_argument, Msg.str())); } - auto &Entry = TargetIDs[OffloadInfo.OffloadKind.str() + "-" + - OffloadInfo.Triple.str()]; - Entry.first = OffloadInfo.Triple; - Entry.second.insert(OffloadInfo.TargetID); + TargetIDs[OffloadInfo.OffloadKind.str() + "-" + OffloadInfo.Triple.str()] + .insert(OffloadInfo.TargetID); if (KindIsValid && OffloadInfo.hasHostKind()) { ++HostTargetNum; // Save the index of the input that refers to the host. @@ -404,17 +402,14 @@ int main(int argc, const char **argv) { BundlerConfig.TargetNames.assign(StandardizedTargetNames.begin(), StandardizedTargetNames.end()); - 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)) { + for (const auto &TargetID : TargetIDs) { + if (auto ConflictingTID = + clang::getConflictTargetIDCombination(TargetID.second)) { SmallVector<char, 128u> Buf; raw_svector_ostream Msg(Buf); Msg << "Cannot bundle inputs with conflicting targets: '" - << Key + "-" + ConflictingTID->first << "' and '" - << Key + "-" + ConflictingTID->second << "'"; + << TargetID.first + "-" + ConflictingTID->first << "' and '" + << TargetID.first + "-" + ConflictingTID->second << "'"; return reportError(createStringError(errc::invalid_argument, Msg.str())); } } diff --git a/llvm/include/llvm/TargetParser/AMDGPUTargetParser.h b/llvm/include/llvm/TargetParser/AMDGPUTargetParser.h index f5e5137b027b4..c2e394d82292f 100644 --- a/llvm/include/llvm/TargetParser/AMDGPUTargetParser.h +++ b/llvm/include/llvm/TargetParser/AMDGPUTargetParser.h @@ -299,7 +299,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 getCanonicalTargetIDString() const; + std::string getCanonicalFeatureString() 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 bd87e635a00ee..61addf60b284c 100644 --- a/llvm/lib/TargetParser/AMDGPUTargetParser.cpp +++ b/llvm/lib/TargetParser/AMDGPUTargetParser.cpp @@ -716,7 +716,7 @@ void TargetID::printCanonicalTargetIDString(raw_ostream &OS) const { printFeatureModifiers(OS, getSramEccSetting(), getXnackSetting()); } -std::string TargetID::getCanonicalTargetIDString() const { +std::string TargetID::getCanonicalFeatureString() 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 377a3304231c8..f13704b138d88 100644 --- a/llvm/unittests/TargetParser/TargetParserTest.cpp +++ b/llvm/unittests/TargetParser/TargetParserTest.cpp @@ -3120,12 +3120,12 @@ TEST(TargetParserTest, testAMDGPUParseTargetIDString) { } EXPECT_EQ(TargetID::parse(AMDHSA, "gfx908:xnack+:sramecc-") - ->getCanonicalTargetIDString(), + ->getCanonicalFeatureString(), "gfx908:sramecc-:xnack+"); - EXPECT_EQ(TargetID::parse(AMDHSA, "gfx908")->getCanonicalTargetIDString(), + EXPECT_EQ(TargetID::parse(AMDHSA, "gfx908")->getCanonicalFeatureString(), "gfx908"); EXPECT_EQ(TargetID::parse(Triple("amdgcn-amd-amdpal"), "gfx908:xnack-") - ->getCanonicalTargetIDString(), + ->getCanonicalFeatureString(), "gfx908:xnack-"); EXPECT_TRUE(TargetID::parse(AMDHSA, "").has_value()); EXPECT_FALSE(TargetID::parse(AMDHSA, "gfxbogus").has_value()); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
