https://github.com/aengelke created https://github.com/llvm/llvm-project/pull/206234
Build failures in C++20 mode due to deleted constructor preventing brace initialization. Reverts llvm/llvm-project#206178 >From 46b70b9ec8b085131a711754b75978ec96a619a4 Mon Sep 17 00:00:00 2001 From: Alexis Engelke <[email protected]> Date: Sat, 27 Jun 2026 11:15:29 +0200 Subject: [PATCH] Revert "[MC][NFC] Make FeatureKV/SubtargetKV pointers private (#206178)" This reverts commit 0b3d2ae22d4adbdb45a1ed0348ff574b8eeb80a4. --- clang/tools/driver/cc1_main.cpp | 12 +++--- llvm/include/llvm/MC/MCSubtargetInfo.h | 42 +++++++------------ llvm/lib/MC/MCSubtargetInfo.cpp | 23 +++++----- .../AMDGPURemoveIncompatibleFunctions.cpp | 4 +- .../Target/RISCV/AsmParser/RISCVAsmParser.cpp | 29 +++++++------ .../RISCV/MCTargetDesc/RISCVBaseInfo.cpp | 6 +-- llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp | 8 ++-- .../WebAssembly/WebAssemblyAsmPrinter.cpp | 2 +- .../WebAssembly/WebAssemblyTargetMachine.cpp | 6 +-- .../Transforms/TargetToTargetFeatures.cpp | 6 +-- 10 files changed, 61 insertions(+), 77 deletions(-) diff --git a/clang/tools/driver/cc1_main.cpp b/clang/tools/driver/cc1_main.cpp index 89b0a340e6672..9e2e3f9c645e9 100644 --- a/clang/tools/driver/cc1_main.cpp +++ b/clang/tools/driver/cc1_main.cpp @@ -150,7 +150,7 @@ static int PrintSupportedExtensions(std::string TargetStr) { llvm::StringMap<llvm::StringRef> DescMap; for (const llvm::SubtargetFeatureKV &feature : Features) - DescMap.insert({feature.key(), feature.desc()}); + DescMap.insert({feature.Key, feature.Desc}); if (MachineTriple.isRISCV()) llvm::RISCVISAInfo::printSupportedExtensions(DescMap); @@ -192,18 +192,18 @@ static int PrintEnabledExtensions(const TargetOptions& TargetOpts) { // Extract the feature names that are enabled for the given target. // We do that by capturing the key from the set of SubtargetFeatureKV entries // provided by MCSubtargetInfo, which match the '-target-feature' values. - const std::vector<const llvm::SubtargetFeatureKV *> Features = + const std::vector<llvm::SubtargetFeatureKV> Features = MCInfo.getEnabledProcessorFeatures(); std::set<llvm::StringRef> EnabledFeatureNames; - for (const llvm::SubtargetFeatureKV *feature : Features) - EnabledFeatureNames.insert(feature->key()); + for (const llvm::SubtargetFeatureKV &feature : Features) + EnabledFeatureNames.insert(feature.Key); if (MachineTriple.isAArch64()) llvm::AArch64::printEnabledExtensions(EnabledFeatureNames); else if (MachineTriple.isRISCV()) { llvm::StringMap<llvm::StringRef> DescMap; - for (const llvm::SubtargetFeatureKV *feature : Features) - DescMap.insert({feature->key(), feature->desc()}); + for (const llvm::SubtargetFeatureKV &feature : Features) + DescMap.insert({feature.Key, feature.Desc}); llvm::RISCVISAInfo::printEnabledExtensions(MachineTriple.isArch64Bit(), EnabledFeatureNames, DescMap); } else { diff --git a/llvm/include/llvm/MC/MCSubtargetInfo.h b/llvm/include/llvm/MC/MCSubtargetInfo.h index 0d690586f246b..708de6d98f40b 100644 --- a/llvm/include/llvm/MC/MCSubtargetInfo.h +++ b/llvm/include/llvm/MC/MCSubtargetInfo.h @@ -34,26 +34,19 @@ class MCInst; /// Used to provide key value pairs for feature and CPU bit flags. struct SubtargetFeatureKV { - // Note: PrivKey/PrivDesc should not be accessed and will be removed. They are - // not private to keep this struct POD. - const char *PrivKey; ///< K-V key string - const char *PrivDesc; ///< Help descriptor + const char *Key; ///< K-V key string + const char *Desc; ///< Help descriptor unsigned Value; ///< K-V integer value FeatureBitArray Implies; ///< K-V bit mask - // Because of relative string offsets, this type is not copyable. - SubtargetFeatureKV(const SubtargetFeatureKV &) = delete; - SubtargetFeatureKV &operator=(const SubtargetFeatureKV &) = delete; - - const char *key() const { return PrivKey; } - const char *desc() const { return PrivDesc; } - /// Compare routine for std::lower_bound - bool operator<(StringRef S) const { return StringRef(key()) < S; } + bool operator<(StringRef S) const { + return StringRef(Key) < S; + } /// Compare routine for std::is_sorted. bool operator<(const SubtargetFeatureKV &Other) const { - return StringRef(key()) < StringRef(Other.key()); + return StringRef(Key) < StringRef(Other.Key); } }; @@ -61,26 +54,19 @@ struct SubtargetFeatureKV { /// Used to provide key value pairs for feature and CPU bit flags. struct SubtargetSubTypeKV { - // Note: PrivKey/PrivSchedModel should not be accessed and will be removed. - // They are not private to keep this struct POD. - const char *PrivKey; ///< K-V key string + const char *Key; ///< K-V key string FeatureBitArray Implies; ///< K-V bit mask FeatureBitArray TuneImplies; ///< K-V bit mask - const MCSchedModel *PrivSchedModel; - - // Because of relative string offsets, this type is not copyable. - SubtargetSubTypeKV(const SubtargetSubTypeKV &) = delete; - SubtargetSubTypeKV &operator=(const SubtargetSubTypeKV &) = delete; - - const char *key() const { return PrivKey; } - const MCSchedModel *schedModel() const { return PrivSchedModel; } + const MCSchedModel *SchedModel; /// Compare routine for std::lower_bound - bool operator<(StringRef S) const { return StringRef(key()) < S; } + bool operator<(StringRef S) const { + return StringRef(Key) < S; + } /// Compare routine for std::is_sorted. bool operator<(const SubtargetSubTypeKV &Other) const { - return StringRef(key()) < StringRef(Other.key()); + return StringRef(Key) < StringRef(Other.Key); } }; @@ -244,7 +230,7 @@ class LLVM_ABI MCSubtargetInfo { /// Check whether the CPU string is valid. virtual bool isCPUStringValid(StringRef CPU) const { auto Found = llvm::lower_bound(ProcDesc, CPU); - return Found != ProcDesc.end() && StringRef(Found->key()) == CPU; + return Found != ProcDesc.end() && StringRef(Found->Key) == CPU; } /// Return processor descriptions. @@ -258,7 +244,7 @@ class LLVM_ABI MCSubtargetInfo { } /// Return the list of processor features currently enabled. - std::vector<const SubtargetFeatureKV *> getEnabledProcessorFeatures() const; + std::vector<SubtargetFeatureKV> getEnabledProcessorFeatures() const; /// HwMode IDs are stored and accessed in a bit set format, enabling /// users to efficiently retrieve specific IDs, such as the RegInfo diff --git a/llvm/lib/MC/MCSubtargetInfo.cpp b/llvm/lib/MC/MCSubtargetInfo.cpp index 2cae4643641a3..ed263a2b4817f 100644 --- a/llvm/lib/MC/MCSubtargetInfo.cpp +++ b/llvm/lib/MC/MCSubtargetInfo.cpp @@ -28,8 +28,7 @@ static const T *Find(StringRef S, ArrayRef<T> A) { // Binary search the array auto F = llvm::lower_bound(A, S); // If not found then return NULL - if (F == A.end() || StringRef(F->key()) != S) - return nullptr; + if (F == A.end() || StringRef(F->Key) != S) return nullptr; // Return the found array item return F; } @@ -98,7 +97,7 @@ static void ApplyFeatureFlag(FeatureBitset &Bits, StringRef Feature, static size_t getLongestEntryLength(ArrayRef<SubtargetFeatureKV> Table) { size_t MaxLen = 0; for (auto &I : Table) - MaxLen = std::max(MaxLen, std::strlen(I.key())); + MaxLen = std::max(MaxLen, std::strlen(I.Key)); return MaxLen; } @@ -139,8 +138,7 @@ static void Help(ArrayRef<StringRef> CPUNames, // Print the Feature table. errs() << "Available features for this target:\n\n"; for (auto &Feature : FeatTable) - errs() << format(" %-*s - %s.\n", MaxFeatLen, Feature.key(), - Feature.desc()); + errs() << format(" %-*s - %s.\n", MaxFeatLen, Feature.Key, Feature.Desc); errs() << '\n'; errs() << "Use +feature to enable a feature, or -feature to disable it.\n" @@ -356,8 +354,8 @@ const MCSchedModel &MCSubtargetInfo::getSchedModelForCPU(StringRef CPU) const { << " (ignoring processor)\n"; return MCSchedModel::Default; } - assert(CPUEntry->schedModel() && "Missing processor SchedModel value"); - return *CPUEntry->schedModel(); + assert(CPUEntry->SchedModel && "Missing processor SchedModel value"); + return *CPUEntry->SchedModel; } InstrItineraryData @@ -371,12 +369,13 @@ void MCSubtargetInfo::initInstrItins(InstrItineraryData &InstrItins) const { ForwardingPaths); } -std::vector<const SubtargetFeatureKV *> +std::vector<SubtargetFeatureKV> MCSubtargetInfo::getEnabledProcessorFeatures() const { - std::vector<const SubtargetFeatureKV *> EnabledFeatures; - for (const SubtargetFeatureKV &FeatureKV : ProcFeatures) - if (FeatureBits.test(FeatureKV.Value)) - EnabledFeatures.push_back(&FeatureKV); + std::vector<SubtargetFeatureKV> EnabledFeatures; + auto IsEnabled = [&](const SubtargetFeatureKV &FeatureKV) { + return FeatureBits.test(FeatureKV.Value); + }; + llvm::copy_if(ProcFeatures, std::back_inserter(EnabledFeatures), IsEnabled); return EnabledFeatures; } diff --git a/llvm/lib/Target/AMDGPU/AMDGPURemoveIncompatibleFunctions.cpp b/llvm/lib/Target/AMDGPU/AMDGPURemoveIncompatibleFunctions.cpp index 6ee58aaba6e5b..246d2a9738d88 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPURemoveIncompatibleFunctions.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPURemoveIncompatibleFunctions.cpp @@ -88,7 +88,7 @@ class AMDGPURemoveIncompatibleFunctionsLegacy : public ModulePass { StringRef getFeatureName(unsigned Feature) { for (const SubtargetFeatureKV &KV : AMDGPUFeatureKV) if (Feature == KV.Value) - return KV.key(); + return KV.Key; llvm_unreachable("Unknown Target feature"); } @@ -96,7 +96,7 @@ StringRef getFeatureName(unsigned Feature) { const SubtargetSubTypeKV *getGPUInfo(const GCNSubtarget &ST, StringRef GPUName) { for (const SubtargetSubTypeKV &KV : ST.getAllProcessorDescriptions()) - if (StringRef(KV.key()) == GPUName) + if (StringRef(KV.Key) == GPUName) return &KV; return nullptr; diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp index 41e83210ed356..c7a9999851319 100644 --- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp +++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp @@ -2084,10 +2084,9 @@ ParseStatus RISCVAsmParser::parseCSRSystemRegister(OperandVector &Operands) { // Accept a named Sys Reg if the required features are present. const auto &FeatureBits = getSTI().getFeatureBits(); if (!SysReg->haveRequiredFeatures(FeatureBits)) { - const auto *Feature = - llvm::find_if(RISCVFeatureKV, [&](const auto &Feature) { - return SysReg->FeaturesRequired[Feature.Value]; - }); + const auto *Feature = llvm::find_if(RISCVFeatureKV, [&](auto Feature) { + return SysReg->FeaturesRequired[Feature.Value]; + }); auto ErrorMsg = std::string("system register '") + SysReg->Name + "' "; if (SysReg->IsRV32Only && FeatureBits[RISCV::Feature64Bit]) { ErrorMsg += "is RV32 only"; @@ -2096,7 +2095,7 @@ ParseStatus RISCVAsmParser::parseCSRSystemRegister(OperandVector &Operands) { } if (Feature != std::end(RISCVFeatureKV)) { ErrorMsg += - "requires '" + std::string(Feature->key()) + "' to be enabled"; + "requires '" + std::string(Feature->Key) + "' to be enabled"; } return Error(S, ErrorMsg); @@ -3132,8 +3131,8 @@ ParseStatus RISCVAsmParser::parseDirective(AsmToken DirectiveID) { bool RISCVAsmParser::resetToArch(StringRef Arch, SMLoc Loc, std::string &Result, bool FromOptionDirective) { for (auto &Feature : RISCVFeatureKV) - if (llvm::RISCVISAInfo::isSupportedExtensionFeature(Feature.key())) - clearFeatureBits(Feature.Value, Feature.key()); + if (llvm::RISCVISAInfo::isSupportedExtensionFeature(Feature.Key)) + clearFeatureBits(Feature.Value, Feature.Key); auto ParseResult = llvm::RISCVISAInfo::parseArchString( Arch, /*EnableExperimentalExtension=*/true, @@ -3151,8 +3150,8 @@ bool RISCVAsmParser::resetToArch(StringRef Arch, SMLoc Loc, std::string &Result, auto &ISAInfo = *ParseResult; for (auto &Feature : RISCVFeatureKV) - if (ISAInfo->hasExtension(Feature.key())) - setFeatureBits(Feature.Value, Feature.key()); + if (ISAInfo->hasExtension(Feature.Key)) + setFeatureBits(Feature.Value, Feature.Key); if (FromOptionDirective) { if (ISAInfo->getXLen() == 32 && isRV64()) @@ -3247,7 +3246,7 @@ bool RISCVAsmParser::parseDirectiveOption() { StringRef(Feature).starts_with("experimental-")) return Error(Loc, "unexpected experimental extensions"); auto Ext = llvm::lower_bound(RISCVFeatureKV, Feature); - if (Ext == std::end(RISCVFeatureKV) || StringRef(Ext->key()) != Feature) + if (Ext == std::end(RISCVFeatureKV) || StringRef(Ext->Key) != Feature) return Error(Loc, "unknown extension feature"); Args.emplace_back(Type, Arch.str()); @@ -3255,7 +3254,7 @@ bool RISCVAsmParser::parseDirectiveOption() { if (Type == RISCVOptionArchArgType::Plus) { FeatureBitset OldFeatureBits = STI->getFeatureBits(); - setFeatureBits(Ext->Value, Ext->key()); + setFeatureBits(Ext->Value, Ext->Key); auto ParseResult = RISCVFeatures::parseFeatureBits(isRV64(), STI->getFeatureBits()); if (!ParseResult) { copySTI().setFeatureBits(OldFeatureBits); @@ -3277,13 +3276,13 @@ bool RISCVAsmParser::parseDirectiveOption() { for (auto &Feature : RISCVFeatureKV) { if (getSTI().hasFeature(Feature.Value) && Feature.Implies.test(Ext->Value)) - return Error(Loc, Twine("can't disable ") + Ext->key() + - " extension; " + Feature.key() + - " extension requires " + Ext->key() + + return Error(Loc, Twine("can't disable ") + Ext->Key + + " extension; " + Feature.Key + + " extension requires " + Ext->Key + " extension"); } - clearFeatureBits(Ext->Value, Ext->key()); + clearFeatureBits(Ext->Value, Ext->Key); } } while (Parser.getTok().isNot(AsmToken::EndOfStatement)); diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp index 526ee0f1efd27..364f3601766b8 100644 --- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp +++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp @@ -157,10 +157,10 @@ parseFeatureBits(bool IsRV64, const FeatureBitset &FeatureBits) { unsigned XLen = IsRV64 ? 64 : 32; std::vector<std::string> FeatureVector; // Convert FeatureBitset to FeatureVector. - for (const auto &Feature : RISCVFeatureKV) { + for (auto Feature : RISCVFeatureKV) { if (FeatureBits[Feature.Value] && - llvm::RISCVISAInfo::isSupportedExtensionFeature(Feature.key())) - FeatureVector.push_back(std::string("+") + Feature.key()); + llvm::RISCVISAInfo::isSupportedExtensionFeature(Feature.Key)) + FeatureVector.push_back(std::string("+") + Feature.Key); } return llvm::RISCVISAInfo::parseFeatures(XLen, FeatureVector); } diff --git a/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp b/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp index 8ad3297a44fbb..7b4e2e7390058 100644 --- a/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp +++ b/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp @@ -535,12 +535,12 @@ bool RISCVAsmPrinter::emitDirectiveOptionArch() { if (STI->hasFeature(Feature.Value) == MCSTI.hasFeature(Feature.Value)) continue; - if (!llvm::RISCVISAInfo::isSupportedExtensionFeature(Feature.key())) + if (!llvm::RISCVISAInfo::isSupportedExtensionFeature(Feature.Key)) continue; auto Delta = STI->hasFeature(Feature.Value) ? RISCVOptionArchArgType::Plus : RISCVOptionArchArgType::Minus; - StringRef ExtName = Feature.key(); + StringRef ExtName = Feature.Key; ExtName.consume_front("experimental-"); NeedEmitStdOptionArgs.emplace_back(Delta, ExtName.str()); } @@ -643,9 +643,9 @@ void RISCVAsmPrinter::emitStartOfAsmFile(Module &M) { if (!errorToBool(ParseResult.takeError())) { auto &ISAInfo = *ParseResult; for (const auto &Feature : RISCVFeatureKV) { - if (ISAInfo->hasExtension(Feature.key()) && + if (ISAInfo->hasExtension(Feature.Key) && !SubtargetInfo.hasFeature(Feature.Value)) - SubtargetInfo.ToggleFeature(Feature.key()); + SubtargetInfo.ToggleFeature(Feature.Key); } } } diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp index 57669e45d9f78..29472dcd57136 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp @@ -533,7 +533,7 @@ void WebAssemblyAsmPrinter::EmitTargetFeatures(Module &M) { }; for (const SubtargetFeatureKV &KV : WebAssemblyFeatureKV) { - EmitFeature(KV.key()); + EmitFeature(KV.Key); } // This pseudo-feature tells the linker whether shared memory would be safe EmitFeature("shared-mem"); diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp index 96a38e40aff7c..858c9aab7298d 100644 --- a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp @@ -335,9 +335,9 @@ class CoalesceFeaturesAndStripAtomics final : public ModulePass { std::string Ret; for (const SubtargetFeatureKV &KV : WebAssemblyFeatureKV) { if (Features[KV.Value]) - Ret += (StringRef("+") + KV.key() + ",").str(); + Ret += (StringRef("+") + KV.Key + ",").str(); else - Ret += (StringRef("-") + KV.key() + ",").str(); + Ret += (StringRef("-") + KV.Key + ",").str(); } // remove trailing ',' Ret.pop_back(); @@ -403,7 +403,7 @@ class CoalesceFeaturesAndStripAtomics final : public ModulePass { for (const SubtargetFeatureKV &KV : WebAssemblyFeatureKV) { if (Features[KV.Value]) { // Mark features as used - std::string MDKey = (StringRef("wasm-feature-") + KV.key()).str(); + std::string MDKey = (StringRef("wasm-feature-") + KV.Key).str(); M.addModuleFlag(Module::ModFlagBehavior::Error, MDKey, wasm::WASM_FEATURE_PREFIX_USED); } diff --git a/mlir/lib/Target/LLVMIR/Transforms/TargetToTargetFeatures.cpp b/mlir/lib/Target/LLVMIR/Transforms/TargetToTargetFeatures.cpp index 24f37cce513af..1fb26689ea5c9 100644 --- a/mlir/lib/Target/LLVMIR/Transforms/TargetToTargetFeatures.cpp +++ b/mlir/lib/Target/LLVMIR/Transforms/TargetToTargetFeatures.cpp @@ -56,12 +56,12 @@ struct TargetToTargetFeaturesPass llvm::MCSubtargetInfo const &subTargetInfo = (*targetMachine)->getMCSubtargetInfo(); - const std::vector<const llvm::SubtargetFeatureKV *> enabledFeatures = + const std::vector<llvm::SubtargetFeatureKV> enabledFeatures = subTargetInfo.getEnabledProcessorFeatures(); auto plussedFeatures = llvm::map_to_vector( - enabledFeatures, [](const llvm::SubtargetFeatureKV *feature) { - return std::string("+") + feature->key(); + enabledFeatures, [](llvm::SubtargetFeatureKV feature) { + return std::string("+") + feature.Key; }); auto plussedFeaturesRefs = llvm::map_to_vector( _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
