[llvm] [RISCV] Enable encodng conflict framework for RISCV target. (PR #97287)

via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 1 05:12:06 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-risc-v

Author: Garvit Gupta (quic-garvgupt)

<details>
<summary>Changes</summary>

This PR is a follow-up of PR https://github.com/llvm/llvm-project/pull/96174 which added the frameowrk to resolve
encoding conflicts. This PR explicitly enables this only for RISCV target.

---
Full diff: https://github.com/llvm/llvm-project/pull/97287.diff


6 Files Affected:

- (modified) llvm/include/llvm/TableGen/SearchableTable.td (+8) 
- (modified) llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp (+12-5) 
- (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp (+8-5) 
- (modified) llvm/lib/Target/RISCV/RISCVSystemOperands.td (+1) 
- (added) llvm/test/TableGen/EncodingConflict.td (+109) 
- (modified) llvm/utils/TableGen/SearchableTableEmitter.cpp (+71-32) 


``````````diff
diff --git a/llvm/include/llvm/TableGen/SearchableTable.td b/llvm/include/llvm/TableGen/SearchableTable.td
index 9dddd5e578ff1..b9dfda0e23b73 100644
--- a/llvm/include/llvm/TableGen/SearchableTable.td
+++ b/llvm/include/llvm/TableGen/SearchableTable.td
@@ -114,6 +114,9 @@ class GenericTable {
 
   // See SearchIndex.EarlyOut
   bit PrimaryKeyEarlyOut = false;
+
+  // See SearchIndex.ReturnRange
+  bit PrimaryKeyReturnRange = false;
 }
 
 // Define a record derived from this class to generate an additional search
@@ -135,6 +138,11 @@ class SearchIndex {
   //
   // Can only be used when the first field is an integral (non-string) type.
   bit EarlyOut = false;
+
+  // If true, will generate a different function signature which will return
+  // a std::pair<> of iterators pointing to start and end value of the range
+  // e.g. lookupSysRegByEncoding returns multiple CSRs for same encoding.
+  bit ReturnRange = false;
 }
 
 // Legacy table type with integrated enum.
diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index 8ac1cdf0a7a9c..b61224922d45d 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -1860,11 +1860,18 @@ ParseStatus RISCVAsmParser::parseCSRSystemRegister(OperandVector &Operands) {
     if (CE) {
       int64_t Imm = CE->getValue();
       if (isUInt<12>(Imm)) {
-        auto SysReg = RISCVSysReg::lookupSysRegByEncoding(Imm);
-        // Accept an immediate representing a named or un-named Sys Reg
-        // if the range is valid, regardless of the required features.
-        Operands.push_back(
-            RISCVOperand::createSysReg(SysReg ? SysReg->Name : "", S, Imm));
+        auto Range = RISCVSysReg::lookupSysRegByEncoding(Imm);
+        // Accept an immediate representing a named Sys Reg if it satisfies the
+        // the requried features.
+        for (auto It : Range) {
+          if (It.haveRequiredFeatures(STI->getFeatureBits())) {
+            Operands.push_back(RISCVOperand::createSysReg(It.Name, S, Imm));
+            return ParseStatus::Success;
+          }
+        }
+        // Accept an immediate representing an un-named Sys Reg if the range is
+        // valid, regardless of the required features.
+        Operands.push_back(RISCVOperand::createSysReg("", S, Imm));
         return ParseStatus::Success;
       }
     }
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
index 48b669c78cade..19cfde4fbd051 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
@@ -121,11 +121,14 @@ void RISCVInstPrinter::printCSRSystemRegister(const MCInst *MI, unsigned OpNo,
                                               const MCSubtargetInfo &STI,
                                               raw_ostream &O) {
   unsigned Imm = MI->getOperand(OpNo).getImm();
-  auto SysReg = RISCVSysReg::lookupSysRegByEncoding(Imm);
-  if (SysReg && SysReg->haveRequiredFeatures(STI.getFeatureBits()))
-    markup(O, Markup::Register) << SysReg->Name;
-  else
-    markup(O, Markup::Register) << formatImm(Imm);
+  auto Range = RISCVSysReg::lookupSysRegByEncoding(Imm);
+  for(auto It : Range){
+    if (It.haveRequiredFeatures(STI.getFeatureBits())) {
+      markup(O, Markup::Register) << It.Name;
+      return;
+    }
+  }
+  markup(O, Markup::Register) << formatImm(Imm);
 }
 
 void RISCVInstPrinter::printFenceArg(const MCInst *MI, unsigned OpNo,
diff --git a/llvm/lib/Target/RISCV/RISCVSystemOperands.td b/llvm/lib/Target/RISCV/RISCVSystemOperands.td
index db840b3027492..a836227e18957 100644
--- a/llvm/lib/Target/RISCV/RISCVSystemOperands.td
+++ b/llvm/lib/Target/RISCV/RISCVSystemOperands.td
@@ -49,6 +49,7 @@ def SysRegsList : GenericTable {
 
   let PrimaryKey = [ "Encoding" ];
   let PrimaryKeyName = "lookupSysRegByEncoding";
+  let PrimaryKeyReturnRange = true;
 }
 
 def lookupSysRegByName : SearchIndex {
diff --git a/llvm/test/TableGen/EncodingConflict.td b/llvm/test/TableGen/EncodingConflict.td
new file mode 100644
index 0000000000000..3dfb389f6c716
--- /dev/null
+++ b/llvm/test/TableGen/EncodingConflict.td
@@ -0,0 +1,109 @@
+// RUN: llvm-tblgen -gen-searchable-tables -I %p/../../include %s | FileCheck %s
+
+include "llvm/TableGen/SearchableTable.td"
+
+class SysReg<string name, bits<12> op> {
+  string Name = name;
+  bits<12> Encoding = op;
+  code FeaturesRequired = [{ {} }];
+}
+
+def List1 : GenericTable {
+  let FilterClass = "SysReg";
+  let Fields = [
+     "Name", "Encoding", "FeaturesRequired",
+  ];
+
+  let PrimaryKey = [ "Encoding" ];
+  let PrimaryKeyName = "lookupSysRegByEncoding";
+  let PrimaryKeyReturnRange = true;
+}
+
+
+let FeaturesRequired = [{ {Feature1} }] in {
+def : SysReg<"csr1", 0x7C0>;
+}
+
+let FeaturesRequired = [{ {Feature2} }] in {
+def : SysReg<"csr2", 0x7C0>;
+}
+
+// CHECK: #ifdef GET_List1_DECL
+// CHECK-NEXT: llvm::iterator_range<const SysReg *>lookupSysRegByEncoding(uint16_t Encoding);
+// CHECK-NEXT: #endif
+
+// CHECK: #ifdef GET_List1_IMPL
+// CHECK-NEXT: constexpr SysReg List1[] = {
+// CHECK-NEXT:   { "csr1", 0x7C0,  {Feature1}  }, // 0
+// CHECK-NEXT:   { "csr2", 0x7C0,  {Feature2}  }, // 1
+// CHECK-NEXT:  };
+
+// CHECK: llvm::iterator_range<const SysReg *>lookupSysRegByEncoding(uint16_t Encoding) {
+// CHECK-NEXT:   SysReg Key;
+// CHECK-NEXT:   Key.Encoding = Encoding;
+// CHECK-NEXT:   auto Table = ArrayRef(List1);
+// CHECK-NEXT:   auto It = std::equal_range(Table.begin(), Table.end(), Key,
+// CHECK-NEXT:     [](const SysReg &LHS, const SysReg &RHS) {
+// CHECK-NEXT:       if (LHS.Encoding < RHS.Encoding)
+// CHECK-NEXT:         return true;
+// CHECK-NEXT:       if (LHS.Encoding > RHS.Encoding)
+// CHECK-NEXT:         return false;
+// CHECK-NEXT:       return false;
+// CHECK-NEXT:     });
+
+// CHECK:   return llvm::make_range(It.first, It.second);
+// CHECK-NEXT: }
+// CHECK-NEXT: #endif
+
+def List2 : GenericTable {
+  let FilterClass = "SysReg";
+  let Fields = [
+     "Name", "Encoding", "FeaturesRequired",
+  ];
+}
+
+def lookupSysRegByName : SearchIndex {
+  let Table = List2;
+  let Key = [ "Name" ];
+  let ReturnRange = true;
+}
+
+// CHECK: #ifdef GET_List2_DECL
+// CHECK-NEXT: llvm::iterator_range<const SysReg *>lookupSysRegByName(StringRef Name);
+// CHECK-NEXT: #endif
+
+// CHECK: #ifdef GET_List2_IMPL
+// CHECK-NEXT: constexpr SysReg List2[] = {
+// CHECK-NEXT:   { "csr1", 0x7C0,  {Feature1}  }, // 0
+// CHECK-NEXT:   { "csr2", 0x7C0,  {Feature2}  }, // 1
+// CHECK-NEXT:  };
+
+// CHECK: llvm::iterator_range<const SysReg *>lookupSysRegByName(StringRef Name) {
+// CHECK-NEXT:   struct IndexType {
+// CHECK-NEXT:     const char * Name;
+// CHECK-NEXT:     unsigned _index;
+// CHECK-NEXT:   };
+// CHECK-NEXT:   static const struct IndexType Index[] = {
+// CHECK-NEXT:     { "CSR1", 0 },
+// CHECK-NEXT:     { "CSR2", 1 },
+// CHECK-NEXT:   };
+
+// CHECK:   IndexType Key;
+// CHECK-NEXT:   Key.Name = Name.upper();
+// CHECK-NEXT:   auto Table = ArrayRef(Index);
+// CHECK-NEXT:   auto It = std::equal_range(Table.begin(), Table.end(), Key,
+// CHECK-NEXT:     [](const IndexType &LHS, const IndexType &RHS) {
+// CHECK-NEXT:       int CmpName = StringRef(LHS.Name).compare(RHS.Name);
+// CHECK-NEXT:       if (CmpName < 0) return true;
+// CHECK-NEXT:       if (CmpName > 0) return false;
+// CHECK-NEXT:       return false;
+// CHECK-NEXT:     });
+
+// CHECK:   return llvm::make_range(It.first, It.second);
+// CHECK-NEXT: }
+// CHECK-NEXT: #endif
+
+// CHECK: #undef GET_List1_DECL
+// CHECK-NEXT: #undef GET_List1_IMPL
+// CHECK-NEXT: #undef GET_List2_DECL
+// CHECK-NEXT: #undef GET_List2_IMPL
\ No newline at end of file
diff --git a/llvm/utils/TableGen/SearchableTableEmitter.cpp b/llvm/utils/TableGen/SearchableTableEmitter.cpp
index 48ee23db957de..7cab1d90b3c9b 100644
--- a/llvm/utils/TableGen/SearchableTableEmitter.cpp
+++ b/llvm/utils/TableGen/SearchableTableEmitter.cpp
@@ -68,6 +68,7 @@ struct SearchIndex {
   SMLoc Loc; // Source location of PrimaryKey or Key field definition.
   SmallVector<GenericField, 1> Fields;
   bool EarlyOut = false;
+  bool ReturnRange = false;
 };
 
 struct GenericTable {
@@ -190,15 +191,18 @@ class SearchableTableEmitter {
   void emitGenericTable(const GenericTable &Table, raw_ostream &OS);
   void emitGenericEnum(const GenericEnum &Enum, raw_ostream &OS);
   void emitLookupDeclaration(const GenericTable &Table,
-                             const SearchIndex &Index, raw_ostream &OS);
+                             const SearchIndex &Index, bool ShouldReturnRange,
+                             raw_ostream &OS);
   void emitLookupFunction(const GenericTable &Table, const SearchIndex &Index,
-                          bool IsPrimary, raw_ostream &OS);
+                          bool IsPrimary, bool ShouldReturnRange,
+                          raw_ostream &OS);
   void emitIfdef(StringRef Guard, raw_ostream &OS);
 
   bool parseFieldType(GenericField &Field, Init *II);
   std::unique_ptr<SearchIndex>
   parseSearchIndex(GenericTable &Table, const RecordVal *RecVal, StringRef Name,
-                   const std::vector<StringRef> &Key, bool EarlyOut);
+                   const std::vector<StringRef> &Key, bool EarlyOut,
+                   bool ReturnRange);
   void collectEnumEntries(GenericEnum &Enum, StringRef NameField,
                           StringRef ValueField,
                           const std::vector<Record *> &Items);
@@ -319,9 +323,10 @@ void SearchableTableEmitter::emitGenericEnum(const GenericEnum &Enum,
 void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
                                                 const SearchIndex &Index,
                                                 bool IsPrimary,
+                                                bool ShouldReturnRange,
                                                 raw_ostream &OS) {
   OS << "\n";
-  emitLookupDeclaration(Table, Index, OS);
+  emitLookupDeclaration(Table, Index, ShouldReturnRange, OS);
   OS << " {\n";
 
   std::vector<Record *> IndexRowsStorage;
@@ -426,16 +431,24 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
     OS << "    return nullptr;\n\n";
   }
 
-  OS << "  struct KeyType {\n";
-  for (const auto &Field : Index.Fields) {
-    OS << "    " << searchableFieldType(Table, Index, Field, TypeInTempStruct)
-       << " " << Field.Name << ";\n";
+  if (ShouldReturnRange)
+    OS << "  " << IndexTypeName << " Key;\n";
+  else {
+    OS << "  struct KeyType {\n";
+    for (const auto &Field : Index.Fields) {
+      OS << "    " << searchableFieldType(Table, Index, Field, TypeInTempStruct)
+         << " " << Field.Name << ";\n";
+    }
+    OS << "  };\n";
+    OS << "  KeyType Key = {";
   }
-  OS << "  };\n";
-  OS << "  KeyType Key = {";
+
   ListSeparator LS;
   for (const auto &Field : Index.Fields) {
-    OS << LS << Field.Name;
+    if (ShouldReturnRange)
+      OS << "  Key." << Field.Name << " = " << Field.Name;
+    else
+      OS << LS << Field.Name;
     if (isa<StringRecTy>(Field.RecType)) {
       OS << ".upper()";
       if (IsPrimary)
@@ -445,12 +458,21 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
                             "case-insensitive comparison of field '" +
                             Field.Name + "'");
     }
+    if (ShouldReturnRange)
+      OS << ";\n";
   }
-  OS << "};\n";
+  if (!ShouldReturnRange)
+    OS << "};\n";
 
   OS << "  auto Table = ArrayRef(" << IndexName << ");\n";
-  OS << "  auto Idx = std::lower_bound(Table.begin(), Table.end(), Key,\n";
-  OS << "    [](const " << IndexTypeName << " &LHS, const KeyType &RHS) {\n";
+  if (ShouldReturnRange) {
+    OS << "  auto It = std::equal_range(Table.begin(), Table.end(), Key,\n";
+    OS << "    [](const " << IndexTypeName << " &LHS, const " << IndexTypeName
+       << " &RHS) {\n";
+  } else {
+    OS << "  auto Idx = std::lower_bound(Table.begin(), Table.end(), Key,\n";
+    OS << "    [](const " << IndexTypeName << " &LHS, const KeyType &RHS) {\n";
+  }
 
   for (const auto &Field : Index.Fields) {
     if (isa<StringRecTy>(Field.RecType)) {
@@ -478,25 +500,34 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
   OS << "      return false;\n";
   OS << "    });\n\n";
 
-  OS << "  if (Idx == Table.end()";
-
-  for (const auto &Field : Index.Fields)
-    OS << " ||\n      Key." << Field.Name << " != Idx->" << Field.Name;
-  OS << ")\n    return nullptr;\n";
+  if (!ShouldReturnRange) {
+    OS << "  if (Idx == Table.end()";
+    for (const auto &Field : Index.Fields)
+      OS << " ||\n      Key." << Field.Name << " != Idx->" << Field.Name;
+  }
 
-  if (IsPrimary)
+  if (ShouldReturnRange)
+    OS << "  return llvm::make_range(It.first, It.second);\n";
+  else if (IsPrimary) {
+    OS << ")\n    return nullptr;\n\n";
     OS << "  return &*Idx;\n";
-  else
+  } else {
+    OS << ")\n    return nullptr;\n\n";
     OS << "  return &" << Table.Name << "[Idx->_index];\n";
+  }
 
   OS << "}\n";
 }
 
 void SearchableTableEmitter::emitLookupDeclaration(const GenericTable &Table,
                                                    const SearchIndex &Index,
+                                                   bool ShouldReturnRange,
                                                    raw_ostream &OS) {
-  OS << "const " << Table.CppTypeName << " *" << Index.Name << "(";
-
+  if (ShouldReturnRange)
+    OS << "llvm::iterator_range<const " << Table.CppTypeName << " *>";
+  else
+    OS << "const " << Table.CppTypeName << " *";
+  OS << Index.Name << "(";
   ListSeparator LS;
   for (const auto &Field : Index.Fields)
     OS << LS << searchableFieldType(Table, Index, Field, TypeInArgument) << " "
@@ -510,11 +541,12 @@ void SearchableTableEmitter::emitGenericTable(const GenericTable &Table,
 
   // Emit the declarations for the functions that will perform lookup.
   if (Table.PrimaryKey) {
-    emitLookupDeclaration(Table, *Table.PrimaryKey, OS);
+    auto &Index = Table.PrimaryKey;
+    emitLookupDeclaration(Table, *Index, Index->ReturnRange, OS);
     OS << ";\n";
   }
   for (const auto &Index : Table.Indices) {
-    emitLookupDeclaration(Table, *Index, OS);
+    emitLookupDeclaration(Table, *Index, Index->ReturnRange, OS);
     OS << ";\n";
   }
 
@@ -540,10 +572,14 @@ void SearchableTableEmitter::emitGenericTable(const GenericTable &Table,
 
   // Indexes are sorted "{ Thing, PrimaryIdx }" arrays, so that a binary
   // search can be performed by "Thing".
-  if (Table.PrimaryKey)
-    emitLookupFunction(Table, *Table.PrimaryKey, true, OS);
+  if (Table.PrimaryKey) {
+    auto &Index = Table.PrimaryKey;
+    emitLookupFunction(Table, *Index, /*IsPrimary=*/true, Index->ReturnRange,
+                       OS);
+  }
   for (const auto &Index : Table.Indices)
-    emitLookupFunction(Table, *Index, false, OS);
+    emitLookupFunction(Table, *Index, /*IsPrimary=*/false, Index->ReturnRange,
+                       OS);
 
   OS << "#endif\n\n";
 }
@@ -569,11 +605,12 @@ bool SearchableTableEmitter::parseFieldType(GenericField &Field, Init *TypeOf) {
 
 std::unique_ptr<SearchIndex> SearchableTableEmitter::parseSearchIndex(
     GenericTable &Table, const RecordVal *KeyRecVal, StringRef Name,
-    const std::vector<StringRef> &Key, bool EarlyOut) {
+    const std::vector<StringRef> &Key, bool EarlyOut, bool ReturnRange) {
   auto Index = std::make_unique<SearchIndex>();
   Index->Name = std::string(Name);
   Index->Loc = KeyRecVal->getLoc();
   Index->EarlyOut = EarlyOut;
+  Index->ReturnRange = ReturnRange;
 
   for (const auto &FieldName : Key) {
     const GenericField *Field = Table.getFieldByName(FieldName);
@@ -769,7 +806,8 @@ void SearchableTableEmitter::run(raw_ostream &OS) {
           parseSearchIndex(*Table, TableRec->getValue("PrimaryKey"),
                            TableRec->getValueAsString("PrimaryKeyName"),
                            TableRec->getValueAsListOfStrings("PrimaryKey"),
-                           TableRec->getValueAsBit("PrimaryKeyEarlyOut"));
+                           TableRec->getValueAsBit("PrimaryKeyEarlyOut"),
+                           TableRec->getValueAsBit("PrimaryKeyReturnRange"));
 
       llvm::stable_sort(Table->Entries, [&](Record *LHS, Record *RHS) {
         return compareBy(LHS, RHS, *Table->PrimaryKey);
@@ -793,7 +831,8 @@ void SearchableTableEmitter::run(raw_ostream &OS) {
     Table.Indices.push_back(
         parseSearchIndex(Table, IndexRec->getValue("Key"), IndexRec->getName(),
                          IndexRec->getValueAsListOfStrings("Key"),
-                         IndexRec->getValueAsBit("EarlyOut")));
+                         IndexRec->getValueAsBit("EarlyOut"),
+                         IndexRec->getValueAsBit("ReturnRange")));
   }
 
   // Translate legacy tables.
@@ -848,7 +887,7 @@ void SearchableTableEmitter::run(raw_ostream &OS) {
       std::string Name =
           (Twine("lookup") + Table->CppTypeName + "By" + Field).str();
       Table->Indices.push_back(parseSearchIndex(*Table, Class->getValue(Field),
-                                                Name, {Field}, false));
+                                                Name, {Field}, false, false));
     }
 
     Tables.emplace_back(std::move(Table));

``````````

</details>


https://github.com/llvm/llvm-project/pull/97287


More information about the llvm-commits mailing list