Author: Alexis Engelke Date: 2026-06-27T10:42:18+02:00 New Revision: 0b3d2ae22d4adbdb45a1ed0348ff574b8eeb80a4
URL: https://github.com/llvm/llvm-project/commit/0b3d2ae22d4adbdb45a1ed0348ff574b8eeb80a4 DIFF: https://github.com/llvm/llvm-project/commit/0b3d2ae22d4adbdb45a1ed0348ff574b8eeb80a4.diff LOG: [MC][NFC] Make FeatureKV/SubtargetKV pointers private (#206178) This is preliminary work for changing the representation of FeatureKV/SubTypeKV to need less relocations. As a first step, avoid all direct references to these pointers. Added: Modified: clang/tools/driver/cc1_main.cpp llvm/include/llvm/MC/MCSubtargetInfo.h llvm/lib/MC/MCSubtargetInfo.cpp llvm/lib/Target/AMDGPU/AMDGPURemoveIncompatibleFunctions.cpp llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp mlir/lib/Target/LLVMIR/Transforms/TargetToTargetFeatures.cpp Removed: ################################################################################ diff --git a/clang/tools/driver/cc1_main.cpp b/clang/tools/driver/cc1_main.cpp index 9e2e3f9c645e9..89b0a340e6672 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<llvm::SubtargetFeatureKV> Features = + const std::vector<const 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 708de6d98f40b..0d690586f246b 100644 --- a/llvm/include/llvm/MC/MCSubtargetInfo.h +++ b/llvm/include/llvm/MC/MCSubtargetInfo.h @@ -34,19 +34,26 @@ class MCInst; /// Used to provide key value pairs for feature and CPU bit flags. struct SubtargetFeatureKV { - const char *Key; ///< K-V key string - const char *Desc; ///< Help descriptor + // 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 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()); } }; @@ -54,19 +61,26 @@ struct SubtargetFeatureKV { /// Used to provide key value pairs for feature and CPU bit flags. struct SubtargetSubTypeKV { - const char *Key; ///< K-V key string + // 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 FeatureBitArray Implies; ///< K-V bit mask FeatureBitArray TuneImplies; ///< K-V bit mask - const MCSchedModel *SchedModel; + 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; } /// 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()); } }; @@ -230,7 +244,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. @@ -244,7 +258,7 @@ class LLVM_ABI MCSubtargetInfo { } /// Return the list of processor features currently enabled. - std::vector<SubtargetFeatureKV> getEnabledProcessorFeatures() const; + std::vector<const 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 ed263a2b4817f..2cae4643641a3 100644 --- a/llvm/lib/MC/MCSubtargetInfo.cpp +++ b/llvm/lib/MC/MCSubtargetInfo.cpp @@ -28,7 +28,8 @@ 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; } @@ -97,7 +98,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; } @@ -138,7 +139,8 @@ 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" @@ -354,8 +356,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 @@ -369,13 +371,12 @@ void MCSubtargetInfo::initInstrItins(InstrItineraryData &InstrItins) const { ForwardingPaths); } -std::vector<SubtargetFeatureKV> +std::vector<const SubtargetFeatureKV *> MCSubtargetInfo::getEnabledProcessorFeatures() const { - std::vector<SubtargetFeatureKV> EnabledFeatures; - auto IsEnabled = [&](const SubtargetFeatureKV &FeatureKV) { - return FeatureBits.test(FeatureKV.Value); - }; - llvm::copy_if(ProcFeatures, std::back_inserter(EnabledFeatures), IsEnabled); + std::vector<const SubtargetFeatureKV *> EnabledFeatures; + for (const SubtargetFeatureKV &FeatureKV : ProcFeatures) + if (FeatureBits.test(FeatureKV.Value)) + EnabledFeatures.push_back(&FeatureKV); return EnabledFeatures; } diff --git a/llvm/lib/Target/AMDGPU/AMDGPURemoveIncompatibleFunctions.cpp b/llvm/lib/Target/AMDGPU/AMDGPURemoveIncompatibleFunctions.cpp index 246d2a9738d88..6ee58aaba6e5b 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 c7a9999851319..41e83210ed356 100644 --- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp +++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp @@ -2084,9 +2084,10 @@ 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, [&](auto Feature) { - return SysReg->FeaturesRequired[Feature.Value]; - }); + const auto *Feature = + llvm::find_if(RISCVFeatureKV, [&](const 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"; @@ -2095,7 +2096,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); @@ -3131,8 +3132,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, @@ -3150,8 +3151,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()) @@ -3246,7 +3247,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()); @@ -3254,7 +3255,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); @@ -3276,13 +3277,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 364f3601766b8..526ee0f1efd27 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 (auto Feature : RISCVFeatureKV) { + for (const 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 7b4e2e7390058..8ad3297a44fbb 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 29472dcd57136..57669e45d9f78 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 858c9aab7298d..96a38e40aff7c 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 1fb26689ea5c9..24f37cce513af 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<llvm::SubtargetFeatureKV> enabledFeatures = + const std::vector<const llvm::SubtargetFeatureKV *> enabledFeatures = subTargetInfo.getEnabledProcessorFeatures(); auto plussedFeatures = llvm::map_to_vector( - enabledFeatures, [](llvm::SubtargetFeatureKV feature) { - return std::string("+") + feature.Key; + enabledFeatures, [](const 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
