Skip to content

Conversation

quic-garvgupt
Copy link
Contributor

This patch adds the framework for resolving encoding conflicts among CSRs.

Specifically, this patch adds a support for emitting a second lookup function for the primary key which takes an additional arguement List of type std::vector and inside the function definition, will populate the List with all sysreg that matches primary key.

While printing the CSR name during objdump, iterate over the List and print the name of only that CSR which satisfies the feature requirement of subtarget.

Below are the signatures of the functions for primary key:

`const SysReg *lookupSysRegByEncoding(uint16_t Encoding);` 
`void lookupSysRegByEncoding(uint16_t Encoding, std::vector<const SysReg*> &List);`

Below is the definition for the second primary function:

void lookupSysRegByEncoding(uint16_t Encoding, std::vector<const SysReg*> &List) {
  struct KeyType {
    uint16_t Encoding;
  };
  KeyType Key = {Encoding};
  auto Table = ArrayRef(SysRegsList);
  auto Idx = std::lower_bound(Table.begin(), Table.end(), Key,
    [](const SysReg &LHS, const KeyType &RHS) {
      if (LHS.Encoding < RHS.Encoding)
        return true;
      if (LHS.Encoding > RHS.Encoding)
        return false;
      return false;
    });

  if (Idx == Table.end() ||
      Key.Encoding != Idx->Encoding)
    return;

  auto UpperBound = std::upper_bound(Table.begin(), Table.end(), Key,
    [](const KeyType &LHS, const SysReg &RHS) {
      if (LHS.Encoding < RHS.Encoding)
        return true;
      if (LHS.Encoding > RHS.Encoding)
        return false;
      return false;
    });

  while(Idx != UpperBound){
    List.push_back(&SysRegsList[Idx - Table.begin()]);
    Idx++;
  }
}

Usage: CSRs with same encodings need to be under separate features. Below is example illustrating the same

let FeaturesRequired = [{ {Feature1} }] in {
def : SysReg<"csr1", 0x123>;
}

let FeaturesRequired = [{ {Feature2} }] in {
def : SysReg<"csr2", 0x123>;
}

…ific CSRs

This patch adds the framework for resolving encoding conflicts among CSRs.

Specifically, this patch adds a support for emitting a second lookup function
for the primary key which takes an additional arguemnt `List` of type `std::vector`
and inside the function definition, will populate the `List` with all sysreg
that matches primary key.

While printing the CSR name during objdump, iterate over the `List` and print the
name of only that CSR which satisifes the feature requirement of subtarget.

Below are the signatures of the functions for primary key:
`const SysReg *lookupSysRegByEncoding(uint16_t Encoding);`
`void lookupSysRegByEncoding(uint16_t Encoding, std::vector<const SysReg*> &List);`

Below is the definition for the second primary function:

void lookupSysRegByEncoding(uint16_t Encoding, std::vector<const SysReg*> &List) {
  struct KeyType {
    uint16_t Encoding;
  };
  KeyType Key = {Encoding};
  auto Table = ArrayRef(SysRegsList);
  auto Idx = std::lower_bound(Table.begin(), Table.end(), Key,
    [](const SysReg &LHS, const KeyType &RHS) {
      if (LHS.Encoding < RHS.Encoding)
        return true;
      if (LHS.Encoding > RHS.Encoding)
        return false;
      return false;
    });

  if (Idx == Table.end() ||
      Key.Encoding != Idx->Encoding)
    return;

  auto UpperBound = std::upper_bound(Table.begin(), Table.end(), Key,
    [](const KeyType &LHS, const SysReg &RHS) {
      if (LHS.Encoding < RHS.Encoding)
        return true;
      if (LHS.Encoding > RHS.Encoding)
        return false;
      return false;
    });

  while(Idx != UpperBound){
    List.push_back(&SysRegsList[Idx - Table.begin()]);
    Idx++;
  }
}

Usage: CSRs with same encodings need to be under separate features.
Below is example illustrating the same

let FeaturesRequired = [{ {Feature1} }] in {
def : SysReg<"csr1", 0x123>;
}

let FeaturesRequired = [{ {Feature2} }] in {
def : SysReg<"csr2", 0x123>;
}
@llvmbot
Copy link
Member

llvmbot commented Jun 19, 2024

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

Author: Garvit Gupta (quic-garvgupt)

Changes

This patch adds the framework for resolving encoding conflicts among CSRs.

Specifically, this patch adds a support for emitting a second lookup function for the primary key which takes an additional arguement List of type std::vector and inside the function definition, will populate the List with all sysreg that matches primary key.

While printing the CSR name during objdump, iterate over the List and print the name of only that CSR which satisfies the feature requirement of subtarget.

Below are the signatures of the functions for primary key:

`const SysReg *lookupSysRegByEncoding(uint16_t Encoding);` 
`void lookupSysRegByEncoding(uint16_t Encoding, std::vector&lt;const SysReg*&gt; &amp;List);`

Below is the definition for the second primary function:

void lookupSysRegByEncoding(uint16_t Encoding, std::vector&lt;const SysReg*&gt; &amp;List) {
  struct KeyType {
    uint16_t Encoding;
  };
  KeyType Key = {Encoding};
  auto Table = ArrayRef(SysRegsList);
  auto Idx = std::lower_bound(Table.begin(), Table.end(), Key,
    [](const SysReg &amp;LHS, const KeyType &amp;RHS) {
      if (LHS.Encoding &lt; RHS.Encoding)
        return true;
      if (LHS.Encoding &gt; RHS.Encoding)
        return false;
      return false;
    });

  if (Idx == Table.end() ||
      Key.Encoding != Idx-&gt;Encoding)
    return;

  auto UpperBound = std::upper_bound(Table.begin(), Table.end(), Key,
    [](const KeyType &amp;LHS, const SysReg &amp;RHS) {
      if (LHS.Encoding &lt; RHS.Encoding)
        return true;
      if (LHS.Encoding &gt; RHS.Encoding)
        return false;
      return false;
    });

  while(Idx != UpperBound){
    List.push_back(&amp;SysRegsList[Idx - Table.begin()]);
    Idx++;
  }
}

Usage: CSRs with same encodings need to be under separate features. Below is example illustrating the same

let FeaturesRequired = [{ {Feature1} }] in {
def : SysReg&lt;"csr1", 0x123&gt;;
}

let FeaturesRequired = [{ {Feature2} }] in {
def : SysReg&lt;"csr2", 0x123&gt;;
}

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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp (+9-5)
  • (modified) llvm/utils/TableGen/SearchableTableEmitter.cpp (+81-37)
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
index 48b669c78cade..57d74f1502c7c 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
@@ -121,11 +121,15 @@ 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);
+  std::vector<const RISCVSysReg::SysReg *> CSRList;
+  RISCVSysReg::lookupSysRegByEncoding(Imm, CSRList);
+  for(auto &Reg : CSRList){
+    if (Reg->haveRequiredFeatures(STI.getFeatureBits())) {
+      markup(O, Markup::Register) << Reg->Name;
+      return;
+    }
+  }
+  markup(O, Markup::Register) << formatImm(Imm);
 }
 
 void RISCVInstPrinter::printFenceArg(const MCInst *MI, unsigned OpNo,
diff --git a/llvm/utils/TableGen/SearchableTableEmitter.cpp b/llvm/utils/TableGen/SearchableTableEmitter.cpp
index 48ee23db957de..0773b41031950 100644
--- a/llvm/utils/TableGen/SearchableTableEmitter.cpp
+++ b/llvm/utils/TableGen/SearchableTableEmitter.cpp
@@ -190,9 +190,11 @@ 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 UseListArg,
+                             raw_ostream &OS);
   void emitLookupFunction(const GenericTable &Table, const SearchIndex &Index,
-                          bool IsPrimary, raw_ostream &OS);
+                          bool IsPrimary, bool UseListArg,
+                          raw_ostream &OS);
   void emitIfdef(StringRef Guard, raw_ostream &OS);
 
   bool parseFieldType(GenericField &Field, Init *II);
@@ -319,9 +321,10 @@ void SearchableTableEmitter::emitGenericEnum(const GenericEnum &Enum,
 void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
                                                 const SearchIndex &Index,
                                                 bool IsPrimary,
+                                                bool UseListArg,
                                                 raw_ostream &OS) {
   OS << "\n";
-  emitLookupDeclaration(Table, Index, OS);
+  emitLookupDeclaration(Table, Index, UseListArg, OS);
   OS << " {\n";
 
   std::vector<Record *> IndexRowsStorage;
@@ -423,7 +426,10 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
         Index.Loc, Field, IndexRows.back()->getValueInit(Field.Name));
     OS << "  if ((" << Field.Name << " < " << FirstRepr << ") ||\n";
     OS << "      (" << Field.Name << " > " << LastRepr << "))\n";
-    OS << "    return nullptr;\n\n";
+    if (UseListArg)
+      OS << "    return;\n\n";
+    else
+      OS << "    return nullptr;\n\n";
   }
 
   OS << "  struct KeyType {\n";
@@ -452,55 +458,80 @@ void SearchableTableEmitter::emitLookupFunction(const GenericTable &Table,
   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)) {
-      OS << "      int Cmp" << Field.Name << " = StringRef(LHS." << Field.Name
-         << ").compare(RHS." << Field.Name << ");\n";
-      OS << "      if (Cmp" << Field.Name << " < 0) return true;\n";
-      OS << "      if (Cmp" << Field.Name << " > 0) return false;\n";
-    } else if (Field.Enum) {
-      // Explicitly cast to unsigned, because the signedness of enums is
-      // compiler-dependent.
-      OS << "      if ((unsigned)LHS." << Field.Name << " < (unsigned)RHS."
-         << Field.Name << ")\n";
-      OS << "        return true;\n";
-      OS << "      if ((unsigned)LHS." << Field.Name << " > (unsigned)RHS."
-         << Field.Name << ")\n";
-      OS << "        return false;\n";
-    } else {
-      OS << "      if (LHS." << Field.Name << " < RHS." << Field.Name << ")\n";
-      OS << "        return true;\n";
-      OS << "      if (LHS." << Field.Name << " > RHS." << Field.Name << ")\n";
-      OS << "        return false;\n";
+  auto emitComparator = [&]() {
+    for (const auto &Field : Index.Fields) {
+      if (isa<StringRecTy>(Field.RecType)) {
+        OS << "      int Cmp" << Field.Name << " = StringRef(LHS." << Field.Name
+           << ").compare(RHS." << Field.Name << ");\n";
+        OS << "      if (Cmp" << Field.Name << " < 0) return true;\n";
+        OS << "      if (Cmp" << Field.Name << " > 0) return false;\n";
+      } else if (Field.Enum) {
+        // Explicitly cast to unsigned, because the signedness of enums is
+        // compiler-dependent.
+        OS << "      if ((unsigned)LHS." << Field.Name << " < (unsigned)RHS."
+           << Field.Name << ")\n";
+        OS << "        return true;\n";
+        OS << "      if ((unsigned)LHS." << Field.Name << " > (unsigned)RHS."
+           << Field.Name << ")\n";
+        OS << "        return false;\n";
+      } else {
+        OS << "      if (LHS." << Field.Name << " < RHS." << Field.Name
+           << ")\n";
+        OS << "        return true;\n";
+        OS << "      if (LHS." << Field.Name << " > RHS." << Field.Name
+           << ")\n";
+        OS << "        return false;\n";
+      }
     }
-  }
 
-  OS << "      return false;\n";
-  OS << "    });\n\n";
+    OS << "      return false;\n";
+    OS << "    });\n\n";
+  };
+  emitComparator();
 
   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 (IsPrimary)
+  if (UseListArg) {
+    OS << ")\n    return;\n\n";
+    OS << "  auto UpperBound = std::upper_bound(Table.begin(), Table.end(), "
+          "Key,\n";
+    OS << "    [](const KeyType &LHS, const " << IndexTypeName << " &RHS) {\n";
+    emitComparator();
+    OS << "  while(Idx != UpperBound){\n";
+    OS << "    List.push_back(&" << Table.Name << "[Idx - Table.begin()]);\n";
+    OS << "    Idx++;\n";
+    OS << "  }\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 UseListArg,
                                                    raw_ostream &OS) {
-  OS << "const " << Table.CppTypeName << " *" << Index.Name << "(";
-
+  if (UseListArg)
+    OS << "void ";
+  else
+    OS << "const " << Table.CppTypeName << " *";
+  OS << Index.Name << "(";
   ListSeparator LS;
   for (const auto &Field : Index.Fields)
     OS << LS << searchableFieldType(Table, Index, Field, TypeInArgument) << " "
        << Field.Name;
+
+  if (UseListArg)
+    OS << ", std::vector<const " << Table.CppTypeName << "*> &List";
+
   OS << ")";
 }
 
@@ -510,11 +541,14 @@ 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, /*UseListArg=*/false, OS);
+    OS << ";\n";
+    emitLookupDeclaration(Table, Index, /*UseListArg=*/true, OS);
     OS << ";\n";
   }
   for (const auto &Index : Table.Indices) {
-    emitLookupDeclaration(Table, *Index, OS);
+    emitLookupDeclaration(Table, *Index, /*UseListArg=*/false, OS);
     OS << ";\n";
   }
 
@@ -540,10 +574,20 @@ 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;
+    // Two lookupfunction functions need to be generated to allow more than one
+    // lookup signature for the primary key lookup : first will return a SysReg
+    // that matches the primary key, second will populate a vector passed as
+    // argument with all the SysRegs that match the primary key.
+    emitLookupFunction(Table, Index, /*IsPrimary=*/true,
+                       /*UseListArg=*/false, OS);
+    emitLookupFunction(Table, Index, /*IsPrimary=*/true,
+                       /*UseListArg=*/true, OS);
+  }
   for (const auto &Index : Table.Indices)
-    emitLookupFunction(Table, *Index, false, OS);
+    emitLookupFunction(Table, *Index, /*IsPrimary=*/false,
+                       /*UseListArg=*/false, OS);
 
   OS << "#endif\n\n";
 }

@quic-garvgupt
Copy link
Contributor Author

@topperc @apazos @asb, can you please add yourself as reviewers for this PR? Thanks

@wangpc-pp wangpc-pp requested review from topperc, asb and apazos June 19, 2024 12:01
// lookup signature for the primary key lookup : first will return a SysReg
// that matches the primary key, second will populate a vector passed as
// argument with all the SysRegs that match the primary key.
emitLookupFunction(Table, Index, /*IsPrimary=*/true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these SearchableTable changes should be in separate PR.

@quic-garvgupt
Copy link
Contributor Author

Abandoning this PR because directly created it for the main branch. Will create a new PR.

Apologies for the inconvenience

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants