llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-backend-amdgpu Author: Matt Arsenault (arsenm) <details> <summary>Changes</summary> This reverts commit 6327754ad4eb21b3c27034bc170e8b9944e5e318. Resolve conflicts with the addition of FEAT_SRAMECC_ON_OFF_MODES. Co-authored-by: Claude Opus 5.5 <noreply@<!-- -->anthropic.com> --- Patch is 44.02 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/231420.diff 13 Files Affected: - (modified) clang/include/clang/Basic/TargetID.h (+10-34) - (modified) clang/lib/Basic/TargetID.cpp (+30-167) - (modified) clang/lib/Basic/Targets/AMDGPU.cpp (+25-18) - (modified) clang/lib/Basic/Targets/AMDGPU.h (+20-14) - (modified) clang/lib/Driver/Driver.cpp (+22-9) - (modified) clang/lib/Driver/OffloadBundler.cpp (+40-43) - (modified) clang/lib/Driver/ToolChains/AMDGPU.cpp (+58-61) - (modified) clang/lib/Driver/ToolChains/AMDGPU.h (+10-13) - (modified) clang/test/OffloadTools/clang-offload-bundler/basic.c (+11) - (modified) clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp (+14-9) - (modified) llvm/include/llvm/TargetParser/AMDGPUTargetParser.h (+1-1) - (modified) llvm/lib/TargetParser/AMDGPUTargetParser.cpp (+6-3) - (modified) llvm/unittests/TargetParser/TargetParserTest.cpp (+9-9) ``````````diff diff --git a/clang/include/clang/Basic/TargetID.h b/clang/include/clang/Basic/TargetID.h index 7e791424df4b8..6321c7df4a185 100644 --- a/clang/include/clang/Basic/TargetID.h +++ b/clang/include/clang/Basic/TargetID.h @@ -9,53 +9,29 @@ #ifndef LLVM_CLANG_BASIC_TARGETID_H #define LLVM_CLANG_BASIC_TARGETID_H -#include "llvm/ADT/SmallVector.h" -#include "llvm/ADT/StringMap.h" +#include "llvm/ADT/ArrayRef.h" #include "llvm/TargetParser/Triple.h" #include <optional> -#include <set> +#include <string> +#include <utility> 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); -/// 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); +/// 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 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(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 423f550ae2fb0..841b2a59672b0 100644 --- a/clang/lib/Basic/TargetID.cpp +++ b/clang/lib/Basic/TargetID.cpp @@ -8,198 +8,61 @@ #include "clang/Basic/TargetID.h" #include "clang/Basic/OffloadArch.h" -#include "llvm/ADT/STLExtras.h" -#include "llvm/ADT/SmallSet.h" -#include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/DenseMap.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; - if (!T.isAMDGCN()) - return Ret; - llvm::AMDGPU::GPUKind ProcKind = llvm::AMDGPU::parseArchAMDGCN(Proc); - if (ProcKind == llvm::AMDGPU::GK_NONE) - return Ret; - const llvm::AMDGPU::AMDGPUFeatureBitset &Features = - llvm::AMDGPU::getFeatureBitset(ProcKind); - // Only allow features in target IDs if the processor supports on/off modes. - if (Features.test(llvm::AMDGPU::FEAT_SRAMECC_ON_OFF_MODES)) - Ret.push_back("sramecc"); - if (Features.test(llvm::AMDGPU::FEAT_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) { +llvm::StringRef getProcessorFromTargetID(const llvm::Triple &T, + llvm::StringRef ArchName) { + auto Split = ArchName.split(':'); if (!T.isAMDGPU()) - return Processor; + return Split.first; - if (llvm::StringRef Name = llvm::AMDGPU::getCanonicalArchName(T, Processor); + if (llvm::StringRef Name = llvm::AMDGPU::getCanonicalArchName(T, Split.first); !Name.empty()) return Name; // Accept the AMDGPU subarch triple spelling (e.g. "amdgpu9.00") as an alias // for the corresponding gfx processor. OffloadArch Arch = - getSubArchOffloadArch(llvm::Triple::parseSubArch(Processor)); + getSubArchOffloadArch(llvm::Triple::parseSubArch(Split.first)); if (Arch.isUnknown()) return {}; return OffloadArchToString(Arch); } -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(const std::set<llvm::StringRef> &TargetIDs) { +getConflictTargetIDCombination(llvm::ArrayRef<TargetIDEntry> Entries) { 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 (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); } - 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 c3e6f8bbcafa8..68c36a3b88d41 100644 --- a/clang/lib/Basic/Targets/AMDGPU.cpp +++ b/clang/lib/Basic/Targets/AMDGPU.cpp @@ -245,13 +245,17 @@ AMDGPUTargetInfo::AMDGPUTargetInfo(const llvm::Triple &Triple, HalfArgsAndReturns = true; if (Opts.AMDGPUXnackState != TargetOptions::AMDGPUFeatureState::Any) { - OffloadArchFeatures["xnack"] = - Opts.AMDGPUXnackState == TargetOptions::AMDGPUFeatureState::Enabled; + XnackSetting = + Opts.AMDGPUXnackState == TargetOptions::AMDGPUFeatureState::Enabled + ? llvm::AMDGPU::TargetIDSetting::On + : llvm::AMDGPU::TargetIDSetting::Off; } if (Opts.AMDGPUSramEccState != TargetOptions::AMDGPUFeatureState::Any) { - OffloadArchFeatures["sramecc"] = - Opts.AMDGPUSramEccState == TargetOptions::AMDGPUFeatureState::Enabled; + SramEccSetting = + Opts.AMDGPUSramEccState == TargetOptions::AMDGPUFeatureState::Enabled + ? llvm::AMDGPU::TargetIDSetting::On + : llvm::AMDGPU::TargetIDSetting::Off; } } @@ -316,22 +320,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 bf4ad80b291ec..6c09ed5f6456b 100644 --- a/clang/lib/Basic/Targets/AMDGPU.h +++ b/clang/lib/Basic/Targets/AMDGPU.h @@ -41,13 +41,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(); } @@ -461,8 +461,8 @@ 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)); + const llvm::AMDGPU::AMDGPUFeatureBitset &ArchFeatures = + llvm::AMDGPU::getFeatureBitset(GPUKind); for (const auto &F : Features) { assert(F.front() == '+' || F.front() == '-'); if (F == "+wavefrontsize64") @@ -473,12 +473,18 @@ 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" && + ArchFeatures.test(llvm::AMDGPU::FEAT_XNACK_ON_OFF_MODES)) + XnackSetting = Setting; + else if (Name == "sramecc" && + ArchFeatures.test(llvm::AMDGPU::FEAT_SRAMECC_SUPPORT)) + SramEccSetting = Setting; } return true; } diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index b2dbc32f1b89f..a9a21175af9c5 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 @@ -3933,14 +3934,21 @@ static StringRef getCanonicalArchString(Compilation &C, if (Arch.isNVPTX()) return Args.MakeArgStringRef(OffloadArchToString(Arch)); - if (Arch.isAMDGPU() || Arch.isAMDGCNSPIRV()) { - 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 (Arch.isAMDGPU() && Triple.isAMDGCN()) { + StringRef TargetIDStr = ArchStr; + if (llvm::Triple::parseSubArch(ArchStr) != llvm::Triple::NoSubArch) + TargetIDStr = getProcessorFromTargetID(Triple, ArchStr); + + std::optional<llvm::AMDGPU::TargetID> ID = + llvm::AMDGPU::TargetID::parse(Triple, TargetIDStr); + 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. @@ -3951,13 +3959,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(ArchSet); + // Sort for a deterministic conflicting pair in the diagnostic. + llvm::SmallVector<StringRef> ArchList(Archs.begin(), Archs.end()); + llvm::sort(ArchList); + + llvm:... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/231420 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
