llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-backend-amdgpu Author: Kewen Meng (Kewen12) <details> <summary>Changes</summary> 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 --- Patch is 39.66 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/213824.diff 13 Files Affected: - (modified) clang/include/clang/Basic/TargetID.h (+34-10) - (modified) clang/lib/Basic/TargetID.cpp (+165-29) - (modified) clang/lib/Basic/Targets/AMDGPU.cpp (+18-25) - (modified) clang/lib/Basic/Targets/AMDGPU.h (+14-18) - (modified) clang/lib/Driver/Driver.cpp (+9-18) - (modified) clang/lib/Driver/OffloadBundler.cpp (+43-40) - (modified) clang/lib/Driver/ToolChains/AMDGPU.cpp (+49-47) - (modified) clang/lib/Driver/ToolChains/AMDGPU.h (+13-10) - (modified) clang/test/OffloadTools/clang-offload-bundler/basic.c (-11) - (modified) clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp (+9-14) - (modified) llvm/include/llvm/TargetParser/AMDGPUTargetParser.h (+1-1) - (modified) llvm/lib/TargetParser/AMDGPUTargetParser.cpp (+1-1) - (modified) llvm/unittests/TargetParser/TargetParserTest.cpp (+3-3) ``````````diff 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/Ta... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/213824 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
