Skip to content

Conversation

@jurahul
Copy link
Contributor

@jurahul jurahul commented Feb 3, 2025

  • Use StringRef::str() instead of std::string(StringRef).
  • Use const pointers for Candidates in getSuperRegForSubReg().
  • Make AsmParserCat and AsmWriterCat static.
  • Use enumerate() in ComputeInstrsByEnum to assign inst enums.
  • Use range-based for loops.

@jurahul jurahul force-pushed the codegentarget_cleanup branch from 50e6306 to ca30d25 Compare February 3, 2025 20:50
@jurahul jurahul marked this pull request as ready for review February 3, 2025 22:05
@llvmbot
Copy link
Member

llvmbot commented Feb 3, 2025

@llvm/pr-subscribers-tablegen

Author: Rahul Joshi (jurahul)

Changes
  • Use StringRef::str() instead of std::string(StringRef).
  • Use const pointers for Candidates in getSuperRegForSubReg().
  • Make AsmParserCat and AsmWriterCat static.
  • Use enumerate() in ComputeInstrsByEnum to assign inst enums.
  • Use range-based for loops.

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

2 Files Affected:

  • (modified) llvm/utils/TableGen/Common/CodeGenInstruction.h (+1-1)
  • (modified) llvm/utils/TableGen/Common/CodeGenTarget.cpp (+16-16)
diff --git a/llvm/utils/TableGen/Common/CodeGenInstruction.h b/llvm/utils/TableGen/Common/CodeGenInstruction.h
index 44c0ab70dc615d..b5c4e671103a9c 100644
--- a/llvm/utils/TableGen/Common/CodeGenInstruction.h
+++ b/llvm/utils/TableGen/Common/CodeGenInstruction.h
@@ -300,7 +300,7 @@ class CodeGenInstruction {
   const Record *InferredFrom;
 
   // The enum value assigned by CodeGenTarget::computeInstrsByEnum.
-  mutable unsigned EnumVal = 0;
+  unsigned EnumVal = 0;
 
   CodeGenInstruction(const Record *R);
 
diff --git a/llvm/utils/TableGen/Common/CodeGenTarget.cpp b/llvm/utils/TableGen/Common/CodeGenTarget.cpp
index 96829a185a30b7..1be7c1244396f9 100644
--- a/llvm/utils/TableGen/Common/CodeGenTarget.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenTarget.cpp
@@ -28,8 +28,8 @@
 #include <tuple>
 using namespace llvm;
 
-cl::OptionCategory AsmParserCat("Options for -gen-asm-parser");
-cl::OptionCategory AsmWriterCat("Options for -gen-asm-writer");
+static cl::OptionCategory AsmParserCat("Options for -gen-asm-parser");
+static cl::OptionCategory AsmWriterCat("Options for -gen-asm-writer");
 
 static cl::opt<unsigned>
     AsmParserNum("asmparsernum", cl::init(0),
@@ -64,9 +64,9 @@ StringRef llvm::getEnumName(MVT::SimpleValueType T) {
 std::string llvm::getQualifiedName(const Record *R) {
   std::string Namespace;
   if (R->getValue("Namespace"))
-    Namespace = std::string(R->getValueAsString("Namespace"));
+    Namespace = R->getValueAsString("Namespace").str();
   if (Namespace.empty())
-    return std::string(R->getName());
+    return R->getName().str();
   return Namespace + "::" + R->getName().str();
 }
 
@@ -166,14 +166,15 @@ CodeGenRegBank &CodeGenTarget::getRegBank() const {
 const CodeGenRegisterClass *CodeGenTarget::getSuperRegForSubReg(
     const ValueTypeByHwMode &ValueTy, CodeGenRegBank &RegBank,
     const CodeGenSubRegIndex *SubIdx, bool MustBeAllocatable) const {
-  std::vector<CodeGenRegisterClass *> Candidates;
+  std::vector<const CodeGenRegisterClass *> Candidates;
   auto &RegClasses = RegBank.getRegClasses();
 
   // Try to find a register class which supports ValueTy, and also contains
   // SubIdx.
-  for (CodeGenRegisterClass &RC : RegClasses) {
+  for (const CodeGenRegisterClass &RC : RegClasses) {
     // Is there a subclass of this class which contains this subregister index?
-    CodeGenRegisterClass *SubClassWithSubReg = RC.getSubClassWithSubReg(SubIdx);
+    const CodeGenRegisterClass *SubClassWithSubReg =
+        RC.getSubClassWithSubReg(SubIdx);
     if (!SubClassWithSubReg)
       continue;
 
@@ -268,32 +269,32 @@ void CodeGenTarget::ReadInstructions() const {
 }
 
 static const CodeGenInstruction *GetInstByName(
-    const char *Name,
+    StringRef Name,
     const DenseMap<const Record *, std::unique_ptr<CodeGenInstruction>> &Insts,
     const RecordKeeper &Records) {
   const Record *Rec = Records.getDef(Name);
 
   const auto I = Insts.find(Rec);
   if (!Rec || I == Insts.end())
-    PrintFatalError(Twine("Could not find '") + Name + "' instruction!");
+    PrintFatalError("Could not find '" + Name + "' instruction!");
   return I->second.get();
 }
 
 static const char *FixedInstrs[] = {
 #define HANDLE_TARGET_OPCODE(OPC) #OPC,
 #include "llvm/Support/TargetOpcodes.def"
-    nullptr};
+};
 
 unsigned CodeGenTarget::getNumFixedInstructions() {
-  return std::size(FixedInstrs) - 1;
+  return std::size(FixedInstrs);
 }
 
 /// Return all of the instructions defined by the target, ordered by
 /// their enum value.
 void CodeGenTarget::ComputeInstrsByEnum() const {
   const auto &Insts = getInstructions();
-  for (const char *const *p = FixedInstrs; *p; ++p) {
-    const CodeGenInstruction *Instr = GetInstByName(*p, Insts, Records);
+  for (const char *Name : FixedInstrs) {
+    const CodeGenInstruction *Instr = GetInstByName(Name, Insts, Records);
     assert(Instr && "Missing target independent instruction");
     assert(Instr->Namespace == "TargetOpcode" && "Bad namespace");
     InstrsByEnum.push_back(Instr);
@@ -324,9 +325,8 @@ void CodeGenTarget::ComputeInstrsByEnum() const {
       });
 
   // Assign an enum value to each instruction according to the sorted order.
-  unsigned Num = 0;
-  for (const CodeGenInstruction *Inst : InstrsByEnum)
-    Inst->EnumVal = Num++;
+  for (const auto &[Idx, Inst] : enumerate(InstrsByEnum))
+    const_cast<CodeGenInstruction *>(Inst)->EnumVal = Idx;
 }
 
 /// isLittleEndianEncoding - Return whether this target encodes its instruction

@jurahul jurahul changed the title [TableGen] Code cleanup in CodeGenTarget.cpp [NFC][TableGen] Code cleanup in CodeGenTarget.cpp Feb 3, 2025
- Use StringRef::str() instead of std::string(StringRef).
- Use const pointers for `Candidates` in getSuperRegForSubReg().
- Make `AsmParserCat` and `AsmWriterCat` static.
- Use enumerate() in `ComputeInstrsByEnum` to assign inst enums.
@jurahul jurahul force-pushed the codegentarget_cleanup branch from ca30d25 to 6910a14 Compare February 3, 2025 22:50
Copy link
Contributor

@s-barannikov s-barannikov left a comment

Choose a reason for hiding this comment

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

LGTM

@jurahul jurahul merged commit 749372b into llvm:main Feb 4, 2025
8 checks passed
@jurahul jurahul deleted the codegentarget_cleanup branch February 4, 2025 04:04
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
- Use StringRef::str() instead of std::string(StringRef).
- Use const pointers for `Candidates` in getSuperRegForSubReg().
- Make `AsmParserCat` and `AsmWriterCat` static.
- Use enumerate() in `ComputeInstrsByEnum` to assign inst enums.
- Use range-based for loops.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants