From c702d65b9cfe8862c8429ee23ebfde0ea28c9068 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Fri, 1 Mar 2019 02:19:26 +0000 Subject: [PATCH] [Subtarget] Remove static global constructor call from the tablegened subtarget feature tables Subtarget features are stored in a std::bitset that has been subclassed. There is a special constructor to allow the tablegen files to provide a list of bits to initialize the std::bitset to. This constructor isn't constexpr and std::bitset doesn't support many constexpr operations either. This results in a static global constructor being used to initialize the feature bitsets in these files at startup. To fix this I've introduced a new FeatureBitArray class that holds three 64-bit values representing the initial bit values and taught tablegen to emit hex constants for them based on the feature enum values. This makes the tablegen files less readable than they were before. I can add the list of features back as a comment if we think that's important. I've added a method to convert from this class into the std::bitset subclass we had before. I considered making the new FeatureBitArray class just implement the std::bitset interface we need instead, but thought I'd see how others felts about that first. I've simplified the interfaces to SetImpliedBits and ClearImpliedBits a little minimize the number of times we need to convert to the bitset. This removes about 27K from my local release+asserts build of llc. Differential Revision: https://reviews.llvm.org/D58520 llvm-svn: 355167 --- include/llvm/MC/SubtargetFeature.h | 27 +++++++++++- lib/MC/SubtargetFeature.cpp | 32 +++++++------- utils/TableGen/SubtargetEmitter.cpp | 65 +++++++++++++++++++---------- 3 files changed, 83 insertions(+), 41 deletions(-) diff --git a/include/llvm/MC/SubtargetFeature.h b/include/llvm/MC/SubtargetFeature.h index 9c0a3f6e45f..6dc4931f5ac 100644 --- a/include/llvm/MC/SubtargetFeature.h +++ b/include/llvm/MC/SubtargetFeature.h @@ -18,6 +18,7 @@ #define LLVM_MC_SUBTARGETFEATURE_H #include "llvm/ADT/StringRef.h" +#include #include #include #include @@ -29,7 +30,9 @@ template class ArrayRef; class raw_ostream; class Triple; -const unsigned MAX_SUBTARGET_FEATURES = 192; +const unsigned MAX_SUBTARGET_WORDS = 3; +const unsigned MAX_SUBTARGET_FEATURES = MAX_SUBTARGET_WORDS * 64; + /// Container class for subtarget features. /// This is convenient because std::bitset does not have a constructor /// with an initializer list of set bits. @@ -46,6 +49,26 @@ public: } }; +/// Class used to store the subtarget bits in the tables created by tablegen. +/// The std::initializer_list constructor of FeatureBitset can't be done at +/// compile time and requires a static constructor to run at startup. +class FeatureBitArray { + std::array Bits; + +public: + constexpr FeatureBitArray(const std::array &B) + : Bits(B) {} + + FeatureBitset getAsBitset() const { + FeatureBitset Result; + + for (unsigned i = 0, e = Bits.size(); i != e; ++i) + Result |= FeatureBitset(Bits[i]) << (64 * i); + + return Result; + } +}; + //===----------------------------------------------------------------------===// /// Used to provide key value pairs for feature and CPU bit flags. @@ -53,7 +76,7 @@ struct SubtargetFeatureKV { const char *Key; ///< K-V key string const char *Desc; ///< Help descriptor unsigned Value; ///< K-V integer value - FeatureBitset Implies; ///< K-V bit mask + FeatureBitArray Implies; ///< K-V bit mask /// Compare routine for std::lower_bound bool operator<(StringRef S) const { diff --git a/lib/MC/SubtargetFeature.cpp b/lib/MC/SubtargetFeature.cpp index 50f979b9d11..c34c6ff9007 100644 --- a/lib/MC/SubtargetFeature.cpp +++ b/lib/MC/SubtargetFeature.cpp @@ -122,29 +122,24 @@ std::string SubtargetFeatures::getString() const { /// For each feature that is (transitively) implied by this feature, set it. static -void SetImpliedBits(FeatureBitset &Bits, const SubtargetFeatureKV &FeatureEntry, +void SetImpliedBits(FeatureBitset &Bits, const FeatureBitset &Implies, ArrayRef FeatureTable) { for (const SubtargetFeatureKV &FE : FeatureTable) { - if (FeatureEntry.Value == FE.Value) continue; - - if (FeatureEntry.Implies.test(FE.Value)) { + if (Implies.test(FE.Value)) { Bits.set(FE.Value); - SetImpliedBits(Bits, FE, FeatureTable); + SetImpliedBits(Bits, FE.Implies.getAsBitset(), FeatureTable); } } } /// For each feature that (transitively) implies this feature, clear it. static -void ClearImpliedBits(FeatureBitset &Bits, - const SubtargetFeatureKV &FeatureEntry, +void ClearImpliedBits(FeatureBitset &Bits, unsigned Value, ArrayRef FeatureTable) { for (const SubtargetFeatureKV &FE : FeatureTable) { - if (FeatureEntry.Value == FE.Value) continue; - - if (FE.Implies.test(FeatureEntry.Value)) { + if (FE.Implies.getAsBitset().test(Value)) { Bits.reset(FE.Value); - ClearImpliedBits(Bits, FE, FeatureTable); + ClearImpliedBits(Bits, FE.Value, FeatureTable); } } } @@ -160,12 +155,12 @@ SubtargetFeatures::ToggleFeature(FeatureBitset &Bits, StringRef Feature, if (Bits.test(FeatureEntry->Value)) { Bits.reset(FeatureEntry->Value); // For each feature that implies this, clear it. - ClearImpliedBits(Bits, *FeatureEntry, FeatureTable); + ClearImpliedBits(Bits, FeatureEntry->Value, FeatureTable); } else { Bits.set(FeatureEntry->Value); // For each feature that this implies, set it. - SetImpliedBits(Bits, *FeatureEntry, FeatureTable); + SetImpliedBits(Bits, FeatureEntry->Implies.getAsBitset(), FeatureTable); } } else { errs() << "'" << Feature << "' is not a recognized feature for this target" @@ -187,12 +182,12 @@ void SubtargetFeatures::ApplyFeatureFlag(FeatureBitset &Bits, StringRef Feature, Bits.set(FeatureEntry->Value); // For each feature that this implies, set it. - SetImpliedBits(Bits, *FeatureEntry, FeatureTable); + SetImpliedBits(Bits, FeatureEntry->Implies.getAsBitset(), FeatureTable); } else { Bits.reset(FeatureEntry->Value); // For each feature that implies this, clear it. - ClearImpliedBits(Bits, *FeatureEntry, FeatureTable); + ClearImpliedBits(Bits, FeatureEntry->Value, FeatureTable); } } else { errs() << "'" << Feature << "' is not a recognized feature for this target" @@ -225,12 +220,13 @@ SubtargetFeatures::getFeatureBits(StringRef CPU, // If there is a match if (CPUEntry) { // Set base feature bits - Bits = CPUEntry->Implies; + FeatureBitset CPUImplies = CPUEntry->Implies.getAsBitset(); + Bits = CPUImplies; // Set the feature implied by this CPU feature, if any. for (auto &FE : FeatureTable) { - if (CPUEntry->Implies.test(FE.Value)) - SetImpliedBits(Bits, FE, FeatureTable); + if (CPUImplies.test(FE.Value)) + SetImpliedBits(Bits, FE.Implies.getAsBitset(), FeatureTable); } } else { errs() << "'" << CPU << "' is not a recognized processor for this target" diff --git a/utils/TableGen/SubtargetEmitter.cpp b/utils/TableGen/SubtargetEmitter.cpp index 92b85d88235..677cd2ee355 100644 --- a/utils/TableGen/SubtargetEmitter.cpp +++ b/utils/TableGen/SubtargetEmitter.cpp @@ -73,9 +73,11 @@ class SubtargetEmitter { CodeGenSchedModels &SchedModels; std::string Target; - void Enumeration(raw_ostream &OS); - unsigned FeatureKeyValues(raw_ostream &OS); - unsigned CPUKeyValues(raw_ostream &OS); + void Enumeration(raw_ostream &OS, DenseMap &FeatureMap); + unsigned FeatureKeyValues(raw_ostream &OS, + const DenseMap &FeatureMap); + unsigned CPUKeyValues(raw_ostream &OS, + const DenseMap &FeatureMap); void FormItineraryStageString(const std::string &Names, Record *ItinData, std::string &ItinString, unsigned &NStages); @@ -137,7 +139,8 @@ public: // // Enumeration - Emit the specified class as an enumeration. // -void SubtargetEmitter::Enumeration(raw_ostream &OS) { +void SubtargetEmitter::Enumeration(raw_ostream &OS, + DenseMap &FeatureMap) { // Get all records of class and sort std::vector DefList = Records.getAllDerivedDefinitions("SubtargetFeature"); @@ -161,6 +164,9 @@ void SubtargetEmitter::Enumeration(raw_ostream &OS) { // Get and emit name OS << " " << Def->getName() << " = " << i << ",\n"; + + // Save the index for this feature. + FeatureMap[Def] = i; } // Close enumeration and namespace @@ -168,11 +174,29 @@ void SubtargetEmitter::Enumeration(raw_ostream &OS) { OS << "} // end namespace " << Target << "\n"; } +static void printFeatureMask(raw_ostream &OS, RecVec &FeatureList, + const DenseMap &FeatureMap) { + std::array Mask = {}; + for (unsigned j = 0, M = FeatureList.size(); j < M; ++j) { + unsigned Bit = FeatureMap.lookup(FeatureList[j]); + Mask[Bit / 64] |= 1ULL << (Bit % 64); + } + + OS << "{ { "; + for (unsigned i = 0; i != Mask.size(); ++i) { + OS << "0x"; + OS.write_hex(Mask[i]); + OS << "ULL, "; + } + OS << "} }"; +} + // // FeatureKeyValues - Emit data of all the subtarget features. Used by the // command line. // -unsigned SubtargetEmitter::FeatureKeyValues(raw_ostream &OS) { +unsigned SubtargetEmitter::FeatureKeyValues( + raw_ostream &OS, const DenseMap &FeatureMap) { // Gather and sort all the features std::vector FeatureList = Records.getAllDerivedDefinitions("SubtargetFeature"); @@ -207,12 +231,9 @@ unsigned SubtargetEmitter::FeatureKeyValues(raw_ostream &OS) { RecVec ImpliesList = Feature->getValueAsListOfDefs("Implies"); - OS << "{"; - for (unsigned j = 0, M = ImpliesList.size(); j < M;) { - OS << " " << Target << "::" << ImpliesList[j]->getName(); - if (++j < M) OS << ","; - } - OS << " } },\n"; + printFeatureMask(OS, ImpliesList, FeatureMap); + + OS << " },\n"; ++NumFeatures; } @@ -226,7 +247,9 @@ unsigned SubtargetEmitter::FeatureKeyValues(raw_ostream &OS) { // CPUKeyValues - Emit data of all the subtarget processors. Used by command // line. // -unsigned SubtargetEmitter::CPUKeyValues(raw_ostream &OS) { +unsigned +SubtargetEmitter::CPUKeyValues(raw_ostream &OS, + const DenseMap &FeatureMap) { // Gather and sort processor information std::vector ProcessorList = Records.getAllDerivedDefinitions("Processor"); @@ -248,12 +271,10 @@ unsigned SubtargetEmitter::CPUKeyValues(raw_ostream &OS) { << "\"" << Name << "\", " << "\"Select the " << Name << " processor\", 0, "; - OS << "{"; - for (unsigned j = 0, M = FeatureList.size(); j < M;) { - OS << " " << Target << "::" << FeatureList[j]->getName(); - if (++j < M) OS << ","; - } - OS << " } },\n"; + printFeatureMask(OS, FeatureList, FeatureMap); + + // The {{}} is for the "implies" section of this data structure. + OS << " },\n"; } // End processor table @@ -1789,8 +1810,10 @@ void SubtargetEmitter::run(raw_ostream &OS) { OS << "\n#ifdef GET_SUBTARGETINFO_ENUM\n"; OS << "#undef GET_SUBTARGETINFO_ENUM\n\n"; + DenseMap FeatureMap; + OS << "namespace llvm {\n"; - Enumeration(OS); + Enumeration(OS, FeatureMap); OS << "} // end namespace llvm\n\n"; OS << "#endif // GET_SUBTARGETINFO_ENUM\n\n"; @@ -1801,9 +1824,9 @@ void SubtargetEmitter::run(raw_ostream &OS) { #if 0 OS << "namespace {\n"; #endif - unsigned NumFeatures = FeatureKeyValues(OS); + unsigned NumFeatures = FeatureKeyValues(OS, FeatureMap); OS << "\n"; - unsigned NumProcs = CPUKeyValues(OS); + unsigned NumProcs = CPUKeyValues(OS, FeatureMap); OS << "\n"; EmitSchedModel(OS); OS << "\n";