From 85fdca935d8aeaf74b46672658ddc916b83f55aa Mon Sep 17 00:00:00 2001 From: Sergei Barannikov Date: Sat, 23 Aug 2025 06:16:25 +0300 Subject: [PATCH] [TableGen][DecoderEmitter] Repurpose Filter class There was a lot of confusion about the responsibilities of Filter and FilterChooser. They created instances of each other and called each other's methods. Some of the methods had similar names and did similar things. This change moves most of the Filter members to FilterChooser and turns Filter into a supplementary class with short lifetime. FilterChooser constructs an array of (candidate) Filters, chooses the best performing one, and applies it to the given set of encodings, creating inferior FilterChoosers as necessary. The Filter array is then destroyed. All responsibility for generating the decoder table now lies with FilterChooser. --- llvm/utils/TableGen/DecoderEmitter.cpp | 148 ++++++++++++------------- 1 file changed, 69 insertions(+), 79 deletions(-) diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp index 611951b7bbd4f..84a9d35436bef 100644 --- a/llvm/utils/TableGen/DecoderEmitter.cpp +++ b/llvm/utils/TableGen/DecoderEmitter.cpp @@ -371,8 +371,6 @@ class DecoderEmitter { namespace { -class FilterChooser; - /// Filter - Filter works with FilterChooser to produce the decoding tree for /// the ISA. /// @@ -409,9 +407,7 @@ class FilterChooser; /// decoder could try to decode the even/odd register numbering and assign to /// VST4q8a or VST4q8b, but for the time being, the decoder chooses the "a" /// version and return the Opcode since the two have the same Asm format string. -class Filter { -protected: - const FilterChooser &Owner; // FilterChooser who owns this filter +struct Filter { unsigned StartBit; // the starting bit position unsigned NumBits; // number of bits to filter @@ -421,14 +417,8 @@ class Filter { // Set of uid's with non-constant segment values. std::vector VariableIDs; - // Map of well-known segment value to its delegate. - std::map> FilterChooserMap; - - // A filter chooser for encodings that contain some '?' in the filtered range. - std::unique_ptr VariableFC; - -public: - Filter(const FilterChooser &owner, unsigned startBit, unsigned numBits); + Filter(ArrayRef Encodings, + ArrayRef EncodingIDs, unsigned StartBit, unsigned NumBits); bool hasSingleFilteredID() const { return FilteredIDs.size() == 1 && FilteredIDs.begin()->second.size() == 1; @@ -439,25 +429,6 @@ class Filter { return FilteredIDs.begin()->second.front(); } - // Return the filter chooser for the group of instructions without constant - // segment values. - const FilterChooser &getVariableFC() const { - assert(hasSingleFilteredID() && FilterChooserMap.empty()); - return *VariableFC; - } - - // Divides the decoding task into sub tasks and delegates them to the - // inferior FilterChooser's. - // - // A special case arises when there's only one entry in the filtered - // instructions. In order to unambiguously decode the singleton, we need to - // match the remaining undecoded encoding bits against the singleton. - void recurse(); - - // Emit table entries to decode instructions given a segment or segments of - // bits. - void emitTableEntry(DecoderTableInfo &TableInfo) const; - // Returns the number of fanout produced by the filter. More fanout implies // the filter distinguishes more categories of instructions. unsigned usefulness() const; @@ -490,9 +461,6 @@ enum bitAttr_t { /// decide what further remaining bits to look at. class FilterChooser { -protected: - friend class Filter; - // Vector of encodings to choose our filter. ArrayRef Encodings; @@ -500,9 +468,6 @@ class FilterChooser { /// Sorted by non-decreasing encoding width. SmallVector EncodingIDs; - // The selected filter, if any. - std::unique_ptr BestFilter; - // Array of bit values passed down from our parent. // Set to all unknown for Parent == nullptr. KnownBits FilterBits; @@ -517,6 +482,29 @@ class FilterChooser { // Parent emitter const DecoderEmitter *Emitter; + /// If the selected filter matches multiple encodings, then this is the + /// starting position and the width of the filtered range. + unsigned StartBit; + unsigned NumBits; + + /// If the selected filter matches multiple encodings, and there is + /// *exactly one* encoding in which all bits are known in the filtered range, + /// then this is the ID of that encoding. + std::optional SingletonEncodingID; + + /// If the selected filter matches multiple encodings, and there is + /// *at least one* encoding in which all bits are known in the filtered range, + /// then this is the FilterChooser created for the subset of encodings that + /// contain some unknown bits in the filtered range. + std::unique_ptr VariableFC; + + /// If the selected filter matches multiple encodings, and there is + /// *more than one* encoding in which all bits are known in the filtered + /// range, then this is a map of field values to FilterChoosers created for + /// the subset of encodings sharing that field value. + /// The "field value" here refers to the encoding bits in the filtered range. + std::map> FilterChooserMap; + struct Island { unsigned StartBit; unsigned NumBits; @@ -566,7 +554,11 @@ class FilterChooser { return Encodings[EncodingIDs.back()].getBitWidth(); } -protected: +private: + /// Applies the given filter to the set of encodings this FilterChooser + /// works with, creating inferior FilterChoosers as necessary. + void applyFilter(const Filter &F); + /// dumpStack - dumpStack traverses the filter chooser chain and calls /// dumpFilterArray on each filter chooser up to the top level one. void dumpStack(raw_ostream &OS, indent Indent, unsigned PadToWidth) const; @@ -600,8 +592,11 @@ class FilterChooser { unsigned EncodingID) const; // Emits code to decode the singleton, and then to decode the rest. - void emitSingletonTableEntry(DecoderTableInfo &TableInfo, - const Filter &Best) const; + void emitSingletonTableEntry(DecoderTableInfo &TableInfo) const; + + // Emit table entries to decode instructions given a segment or segments of + // bits. + void emitTableEntry(DecoderTableInfo &TableInfo) const; void emitBinaryParser(raw_ostream &OS, indent Indent, const OperandInfo &OpInfo) const; @@ -644,12 +639,12 @@ class FilterChooser { // // /////////////////////////// -Filter::Filter(const FilterChooser &owner, unsigned startBit, unsigned numBits) - : Owner(owner), StartBit(startBit), NumBits(numBits) { - assert(StartBit + NumBits - 1 < Owner.FilterBits.getBitWidth()); - - for (unsigned EncodingID : Owner.EncodingIDs) { - const InstructionEncoding &Encoding = Owner.Encodings[EncodingID]; +Filter::Filter(ArrayRef Encodings, + ArrayRef EncodingIDs, unsigned StartBit, + unsigned NumBits) + : StartBit(StartBit), NumBits(NumBits) { + for (unsigned EncodingID : EncodingIDs) { + const InstructionEncoding &Encoding = Encodings[EncodingID]; KnownBits EncodingBits = Encoding.getMandatoryBits(); // Scans the segment for possibly well-specified encoding bits. @@ -670,48 +665,44 @@ Filter::Filter(const FilterChooser &owner, unsigned startBit, unsigned numBits) "Filter returns no instruction categories"); } -// Divides the decoding task into sub tasks and delegates them to the -// inferior FilterChooser's. -// -// A special case arises when there's only one entry in the filtered -// instructions. In order to unambiguously decode the singleton, we need to -// match the remaining undecoded encoding bits against the singleton. -void Filter::recurse() { - assert(Owner.FilterBits.extractBits(NumBits, StartBit).isUnknown()); +void FilterChooser::applyFilter(const Filter &F) { + StartBit = F.StartBit; + NumBits = F.NumBits; + assert(FilterBits.extractBits(NumBits, StartBit).isUnknown()); - if (!VariableIDs.empty()) { + if (!F.VariableIDs.empty()) { // Delegates to an inferior filter chooser for further processing on this // group of instructions whose segment values are variable. - VariableFC = std::make_unique(Owner.Encodings, VariableIDs, - Owner.FilterBits, Owner); + VariableFC = std::make_unique(Encodings, F.VariableIDs, + FilterBits, *this); } // No need to recurse for a singleton filtered instruction. // See also Filter::emit*(). - if (hasSingleFilteredID()) { + if (F.hasSingleFilteredID()) { + SingletonEncodingID = F.getSingletonEncodingID(); assert(VariableFC && "Shouldn't have created a filter for one encoding!"); return; } // Otherwise, create sub choosers. - for (const auto &[FilterVal, InferiorEncodingIDs] : FilteredIDs) { + for (const auto &[FilterVal, InferiorEncodingIDs] : F.FilteredIDs) { // Create a new filter by inserting the field bits into the parent filter. APInt FieldBits(NumBits, FilterVal); - KnownBits InferiorFilterBits = Owner.FilterBits; + KnownBits InferiorFilterBits = FilterBits; InferiorFilterBits.insertBits(KnownBits::makeConstant(FieldBits), StartBit); // Delegates to an inferior filter chooser for further processing on this // category of instructions. - FilterChooserMap.try_emplace( - FilterVal, - std::make_unique(Owner.Encodings, InferiorEncodingIDs, - InferiorFilterBits, Owner)); + FilterChooserMap.try_emplace(FilterVal, std::make_unique( + Encodings, InferiorEncodingIDs, + InferiorFilterBits, *this)); } } // Emit table entries to decode instructions given a segment or segments // of bits. -void Filter::emitTableEntry(DecoderTableInfo &TableInfo) const { +void FilterChooser::emitTableEntry(DecoderTableInfo &TableInfo) const { assert(isUInt<8>(NumBits) && "NumBits overflowed uint8 table entry!"); TableInfo.Table.push_back(MCD::OPC_ExtractField); @@ -1436,15 +1427,14 @@ void FilterChooser::emitSingletonTableEntry(DecoderTableInfo &TableInfo, } // Emits table entries to decode the singleton, and then to decode the rest. -void FilterChooser::emitSingletonTableEntry(DecoderTableInfo &TableInfo, - const Filter &Best) const { +void FilterChooser::emitSingletonTableEntry(DecoderTableInfo &TableInfo) const { // complex singletons need predicate checks from the first singleton // to refer forward to the variable filterchooser that follows. TableInfo.pushScope(); - emitSingletonTableEntry(TableInfo, Best.getSingletonEncodingID()); + emitSingletonTableEntry(TableInfo, *SingletonEncodingID); TableInfo.popScope(); - Best.getVariableFC().emitTableEntries(TableInfo); + VariableFC->emitTableEntries(TableInfo); } // reportRegion is a helper function for filterProcessor to mark a region as @@ -1453,8 +1443,8 @@ void FilterChooser::reportRegion(std::vector> &Filters, bitAttr_t RA, unsigned StartBit, unsigned BitIndex, bool AllowMixed) const { if (AllowMixed ? RA == ATTR_MIXED : RA == ATTR_ALL_SET) - Filters.push_back( - std::make_unique(*this, StartBit, BitIndex - StartBit)); + Filters.push_back(std::make_unique(Encodings, EncodingIDs, StartBit, + BitIndex - StartBit)); } std::unique_ptr @@ -1475,8 +1465,8 @@ FilterChooser::findBestFilter(ArrayRef BitAttrs, bool AllowMixed, std::vector Islands = getIslands(EncodingBits); if (!Islands.empty()) { // Found an instruction with island(s). Now just assign a filter. - return std::make_unique(*this, Islands[0].StartBit, - Islands[0].NumBits); + return std::make_unique( + Encodings, EncodingIDs, Islands[0].StartBit, Islands[0].NumBits); } } } @@ -1708,9 +1698,9 @@ void FilterChooser::doFilter() { if (EncodingIDs.size() < 2) return; - BestFilter = findBestFilter(); + std::unique_ptr BestFilter = findBestFilter(); if (BestFilter) { - BestFilter->recurse(); + applyFilter(*BestFilter); return; } @@ -1749,10 +1739,10 @@ void FilterChooser::emitTableEntries(DecoderTableInfo &TableInfo) const { } // Use the best filter to do the decoding! - if (BestFilter->hasSingleFilteredID()) - emitSingletonTableEntry(TableInfo, *BestFilter); + if (SingletonEncodingID) + emitSingletonTableEntry(TableInfo); else - BestFilter->emitTableEntry(TableInfo); + emitTableEntry(TableInfo); } static std::string findOperandDecoderMethod(const Record *Record) {