[llvm] [LLVM][TableGen] Code cleanup in FastISelEmitter.cpp (PR #139644)

via llvm-commits llvm-commits at lists.llvm.org
Mon May 12 22:24:07 PDT 2025


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-tablegen

Author: Rahul Joshi (jurahul)

<details>
<summary>Changes</summary>

- Use StringRef instead of std::string for `InstructionMemo::Name`.
- Use range for loops, zip_equal and structured bindings in loops.
- Use llvm::any_of instead of manual loops.
- Use ListSeparator.
- Remove {} around single-line if-else chains.
- Use ArrayRef<> instead of const vector reference for function args.
- Change `getLegalCName` to accept a `StringRef` to avoid StringRef->std::string casting in several places.
- Use StringRef instead of std::string for `OpcodeName` (and in associated maps).

Tested by verifying no changes in .inc files with and without this change.

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


1 Files Affected:

- (modified) llvm/utils/TableGen/FastISelEmitter.cpp (+93-129) 


``````````diff
diff --git a/llvm/utils/TableGen/FastISelEmitter.cpp b/llvm/utils/TableGen/FastISelEmitter.cpp
index 9aa6ec1064276..e31d60fc0e42a 100644
--- a/llvm/utils/TableGen/FastISelEmitter.cpp
+++ b/llvm/utils/TableGen/FastISelEmitter.cpp
@@ -35,7 +35,7 @@ using namespace llvm;
 ///
 namespace {
 struct InstructionMemo {
-  std::string Name;
+  StringRef Name;
   const CodeGenRegisterClass *RC;
   std::string SubRegNo;
   std::vector<std::string> PhysRegs;
@@ -71,10 +71,7 @@ class ImmPredicateSet {
     return Entry - 1;
   }
 
-  const TreePredicateFn &getPredicate(unsigned i) {
-    assert(i < PredsByName.size());
-    return PredsByName[i];
-  }
+  const TreePredicateFn &getPredicate(unsigned Idx) { return PredsByName[Idx]; }
 
   typedef std::vector<TreePredicateFn>::const_iterator iterator;
   iterator begin() const { return PredsByName.begin(); }
@@ -151,37 +148,32 @@ struct OperandsSignature {
   bool empty() const { return Operands.empty(); }
 
   bool hasAnyImmediateCodes() const {
-    for (unsigned i = 0, e = Operands.size(); i != e; ++i)
-      if (Operands[i].isImm() && Operands[i].getImmCode() != 0)
-        return true;
-    return false;
+    return llvm::any_of(Operands, [](OpKind Kind) {
+      return Kind.isImm() && Kind.getImmCode() != 0;
+    });
   }
 
   /// getWithoutImmCodes - Return a copy of this with any immediate codes forced
   /// to zero.
   OperandsSignature getWithoutImmCodes() const {
     OperandsSignature Result;
-    for (unsigned i = 0, e = Operands.size(); i != e; ++i)
-      if (!Operands[i].isImm())
-        Result.Operands.push_back(Operands[i]);
-      else
-        Result.Operands.push_back(OpKind::getImm(0));
+    Result.Operands.resize(Operands.size());
+    for (auto [RO, O] : zip_equal(Result.Operands, Operands))
+      RO = O.isImm() ? OpKind::getImm(0) : O;
     return Result;
   }
 
-  void emitImmediatePredicate(raw_ostream &OS, ImmPredicateSet &ImmPredicates) {
-    bool EmittedAnything = false;
-    for (unsigned i = 0, e = Operands.size(); i != e; ++i) {
-      if (!Operands[i].isImm())
+  void emitImmediatePredicate(raw_ostream &OS,
+                              ImmPredicateSet &ImmPredicates) const {
+    ListSeparator LS(" &&\n        ");
+    for (auto [Idx, Opnd] : enumerate(Operands)) {
+      if (!Opnd.isImm())
         continue;
 
-      unsigned Code = Operands[i].getImmCode();
+      unsigned Code = Opnd.getImmCode();
       if (Code == 0)
         continue;
 
-      if (EmittedAnything)
-        OS << " &&\n        ";
-
       TreePredicateFn PredFn = ImmPredicates.getPredicate(Code - 1);
 
       // Emit the type check.
@@ -189,10 +181,9 @@ struct OperandsSignature {
       ValueTypeByHwMode VVT = TP->getTree(0)->getType(0);
       assert(VVT.isSimple() &&
              "Cannot use variable value types with fast isel");
-      OS << "VT == " << getEnumName(VVT.getSimple().SimpleTy) << " && ";
+      OS << LS << "VT == " << getEnumName(VVT.getSimple().SimpleTy) << " && ";
 
-      OS << PredFn.getFnName() << "(imm" << i << ')';
-      EmittedAnything = true;
+      OS << PredFn.getFnName() << "(imm" << Idx << ')';
     }
   }
 
@@ -304,77 +295,74 @@ struct OperandsSignature {
 
   void PrintParameters(raw_ostream &OS) const {
     ListSeparator LS;
-    for (unsigned i = 0, e = Operands.size(); i != e; ++i) {
+    for (const auto [Idx, Opnd] : enumerate(Operands)) {
       OS << LS;
-      if (Operands[i].isReg()) {
-        OS << "Register Op" << i;
-      } else if (Operands[i].isImm()) {
-        OS << "uint64_t imm" << i;
-      } else if (Operands[i].isFP()) {
-        OS << "const ConstantFP *f" << i;
-      } else {
+      if (Opnd.isReg())
+        OS << "Register Op" << Idx;
+      else if (Opnd.isImm())
+        OS << "uint64_t imm" << Idx;
+      else if (Opnd.isFP())
+        OS << "const ConstantFP *f" << Idx;
+      else
         llvm_unreachable("Unknown operand kind!");
-      }
     }
   }
 
-  void PrintArguments(raw_ostream &OS,
-                      const std::vector<std::string> &PR) const {
-    assert(PR.size() == Operands.size());
+  void PrintArguments(raw_ostream &OS, ArrayRef<std::string> PhyRegs) const {
     ListSeparator LS;
-    for (unsigned i = 0, e = Operands.size(); i != e; ++i) {
-      if (PR[i] != "")
+    for (const auto [Idx, Opnd, PhyReg] : enumerate(Operands, PhyRegs)) {
+      if (PhyReg != "") {
         // Implicit physical register operand.
         continue;
+      }
 
       OS << LS;
-      if (Operands[i].isReg()) {
-        OS << "Op" << i;
-      } else if (Operands[i].isImm()) {
-        OS << "imm" << i;
-      } else if (Operands[i].isFP()) {
-        OS << "f" << i;
-      } else {
+      if (Opnd.isReg())
+        OS << "Op" << Idx;
+      else if (Opnd.isImm())
+        OS << "imm" << Idx;
+      else if (Opnd.isFP())
+        OS << "f" << Idx;
+      else
         llvm_unreachable("Unknown operand kind!");
-      }
     }
   }
 
   void PrintArguments(raw_ostream &OS) const {
     ListSeparator LS;
-    for (unsigned i = 0, e = Operands.size(); i != e; ++i) {
+    for (const auto [Idx, Opnd] : enumerate(Operands)) {
       OS << LS;
-      if (Operands[i].isReg()) {
-        OS << "Op" << i;
-      } else if (Operands[i].isImm()) {
-        OS << "imm" << i;
-      } else if (Operands[i].isFP()) {
-        OS << "f" << i;
-      } else {
+      if (Opnd.isReg())
+        OS << "Op" << Idx;
+      else if (Opnd.isImm())
+        OS << "imm" << Idx;
+      else if (Opnd.isFP())
+        OS << "f" << Idx;
+      else
         llvm_unreachable("Unknown operand kind!");
-      }
     }
   }
 
-  void PrintManglingSuffix(raw_ostream &OS, const std::vector<std::string> &PR,
+  void PrintManglingSuffix(raw_ostream &OS, ArrayRef<std::string> PhyRegs,
                            ImmPredicateSet &ImmPredicates,
                            bool StripImmCodes = false) const {
-    for (unsigned i = 0, e = Operands.size(); i != e; ++i) {
-      if (PR[i] != "")
+    for (const auto [PR, Opnd] : zip_equal(PhyRegs, Operands)) {
+      if (PR != "") {
         // Implicit physical register operand. e.g. Instruction::Mul expect to
         // select to a binary op. On x86, mul may take a single operand with
         // the other operand being implicit. We must emit something that looks
         // like a binary instruction except for the very inner fastEmitInst_*
         // call.
         continue;
-      Operands[i].printManglingSuffix(OS, ImmPredicates, StripImmCodes);
+      }
+      Opnd.printManglingSuffix(OS, ImmPredicates, StripImmCodes);
     }
   }
 
   void PrintManglingSuffix(raw_ostream &OS, ImmPredicateSet &ImmPredicates,
                            bool StripImmCodes = false) const {
-    for (unsigned i = 0, e = Operands.size(); i != e; ++i)
-      Operands[i].printManglingSuffix(OS, ImmPredicates, StripImmCodes);
+    for (const OpKind Opnd : Operands)
+      Opnd.printManglingSuffix(OS, ImmPredicates, StripImmCodes);
   }
 };
 } // End anonymous namespace
@@ -386,14 +374,14 @@ class FastISelMap {
   typedef std::multimap<int, InstructionMemo> PredMap;
   typedef std::map<MVT::SimpleValueType, PredMap> RetPredMap;
   typedef std::map<MVT::SimpleValueType, RetPredMap> TypeRetPredMap;
-  typedef std::map<std::string, TypeRetPredMap> OpcodeTypeRetPredMap;
+  typedef std::map<StringRef, TypeRetPredMap> OpcodeTypeRetPredMap;
   typedef std::map<OperandsSignature, OpcodeTypeRetPredMap>
       OperandsOpcodeTypeRetPredMap;
 
   OperandsOpcodeTypeRetPredMap SimplePatterns;
 
   // This is used to check that there are no duplicate predicates
-  std::set<std::tuple<OperandsSignature, std::string, MVT::SimpleValueType,
+  std::set<std::tuple<OperandsSignature, StringRef, MVT::SimpleValueType,
                       MVT::SimpleValueType, std::string>>
       SimplePatternsCheck;
 
@@ -412,20 +400,16 @@ class FastISelMap {
 
 private:
   void emitInstructionCode(raw_ostream &OS, const OperandsSignature &Operands,
-                           const PredMap &PM, const std::string &RetVTName);
+                           const PredMap &PM, StringRef RetVTName);
 };
 } // End anonymous namespace
 
-static std::string getOpcodeName(const Record *Op,
-                                 const CodeGenDAGPatterns &CGP) {
-  return CGP.getSDNodeInfo(Op).getEnumName().str();
-}
-
-static std::string getLegalCName(std::string OpName) {
-  std::string::size_type pos = OpName.find("::");
-  if (pos != std::string::npos)
-    OpName.replace(pos, 2, "_");
-  return OpName;
+static std::string getLegalCName(StringRef OpName) {
+  std::string CName = OpName.str();
+  std::string::size_type Pos = CName.find("::");
+  if (Pos != std::string::npos)
+    CName.replace(Pos, 2, "_");
+  return CName;
 }
 
 FastISelMap::FastISelMap(StringRef instns) : InstNS(instns) {}
@@ -452,10 +436,7 @@ void FastISelMap::collectPatterns(const CodeGenDAGPatterns &CGP) {
   const CodeGenTarget &Target = CGP.getTargetInfo();
 
   // Scan through all the patterns and record the simple ones.
-  for (CodeGenDAGPatterns::ptm_iterator I = CGP.ptm_begin(), E = CGP.ptm_end();
-       I != E; ++I) {
-    const PatternToMatch &Pattern = *I;
-
+  for (const PatternToMatch &Pattern : CGP.ptms()) {
     // For now, just look at Instructions, so that we don't have to worry
     // about emitting multiple instructions for a pattern.
     TreePatternNode &Dst = Pattern.getDstPattern();
@@ -464,15 +445,15 @@ void FastISelMap::collectPatterns(const CodeGenDAGPatterns &CGP) {
     const Record *Op = Dst.getOperator();
     if (!Op->isSubClassOf("Instruction"))
       continue;
-    CodeGenInstruction &II = CGP.getTargetInfo().getInstruction(Op);
-    if (II.Operands.empty())
+    CodeGenInstruction &Inst = CGP.getTargetInfo().getInstruction(Op);
+    if (Inst.Operands.empty())
       continue;
 
     // Allow instructions to be marked as unavailable for FastISel for
     // certain cases, i.e. an ISA has two 'and' instruction which differ
     // by what registers they can use but are otherwise identical for
     // codegen purposes.
-    if (II.FastISelShouldIgnore)
+    if (Inst.FastISelShouldIgnore)
       continue;
 
     // For now, ignore multi-instruction patterns.
@@ -493,7 +474,7 @@ void FastISelMap::collectPatterns(const CodeGenDAGPatterns &CGP) {
     const CodeGenRegisterClass *DstRC = nullptr;
     std::string SubRegNo;
     if (Op->getName() != "EXTRACT_SUBREG") {
-      const Record *Op0Rec = II.Operands[0].Rec;
+      const Record *Op0Rec = Inst.Operands[0].Rec;
       if (Op0Rec->isSubClassOf("RegisterOperand"))
         Op0Rec = Op0Rec->getValueAsDef("RegClass");
       if (!Op0Rec->isSubClassOf("RegisterClass"))
@@ -524,7 +505,7 @@ void FastISelMap::collectPatterns(const CodeGenDAGPatterns &CGP) {
       continue;
 
     const Record *InstPatOp = InstPatNode.getOperator();
-    std::string OpcodeName = getOpcodeName(InstPatOp, CGP);
+    StringRef OpcodeName = CGP.getSDNodeInfo(InstPatOp).getEnumName();
     MVT::SimpleValueType RetVT = MVT::isVoid;
     if (InstPatNode.getNumTypes())
       RetVT = InstPatNode.getSimpleType(0);
@@ -591,7 +572,7 @@ void FastISelMap::collectPatterns(const CodeGenDAGPatterns &CGP) {
                          DstRC, std::move(SubRegNo), std::move(PhysRegInputs),
                          PredicateCheck);
 
-    int complexity = Pattern.getPatternComplexity(CGP);
+    int Complexity = Pattern.getPatternComplexity(CGP);
 
     auto inserted_simple_pattern = SimplePatternsCheck.insert(
         {Operands, OpcodeName, VT, RetVT, PredicateCheck});
@@ -602,7 +583,7 @@ void FastISelMap::collectPatterns(const CodeGenDAGPatterns &CGP) {
 
     // Note: Instructions with the same complexity will appear in the order
     // that they are encountered.
-    SimplePatterns[Operands][OpcodeName][VT][RetVT].emplace(complexity,
+    SimplePatterns[Operands][OpcodeName][VT][RetVT].emplace(Complexity,
                                                             std::move(Memo));
 
     // If any of the operands were immediates with predicates on them, strip
@@ -631,16 +612,13 @@ void FastISelMap::printImmediatePredicates(raw_ostream &OS) {
 
 void FastISelMap::emitInstructionCode(raw_ostream &OS,
                                       const OperandsSignature &Operands,
-                                      const PredMap &PM,
-                                      const std::string &RetVTName) {
+                                      const PredMap &PM, StringRef RetVTName) {
   // Emit code for each possible instruction. There may be
   // multiple if there are subtarget concerns.  A reverse iterator
   // is used to produce the ones with highest complexity first.
 
   bool OneHadNoPredicate = false;
-  for (PredMap::const_reverse_iterator PI = PM.rbegin(), PE = PM.rend();
-       PI != PE; ++PI) {
-    const InstructionMemo &Memo = PI->second;
+  for (const auto &[_, Memo] : reverse(PM)) {
     std::string PredicateCheck = Memo.PredicateCheck;
 
     if (PredicateCheck.empty()) {
@@ -659,11 +637,11 @@ void FastISelMap::emitInstructionCode(raw_ostream &OS,
       OS << "  ";
     }
 
-    for (unsigned i = 0; i < Memo.PhysRegs.size(); ++i) {
-      if (Memo.PhysRegs[i] != "")
+    for (const auto [Idx, PhyReg] : enumerate(Memo.PhysRegs)) {
+      if (PhyReg != "")
         OS << "  BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, MIMD, "
-           << "TII.get(TargetOpcode::COPY), " << Memo.PhysRegs[i]
-           << ").addReg(Op" << i << ");\n";
+           << "TII.get(TargetOpcode::COPY), " << PhyReg << ").addReg(Op" << Idx
+           << ");\n";
     }
 
     OS << "  return fastEmitInst_";
@@ -681,9 +659,8 @@ void FastISelMap::emitInstructionCode(raw_ostream &OS,
          << ");\n";
     }
 
-    if (!PredicateCheck.empty()) {
+    if (!PredicateCheck.empty())
       OS << "  }\n";
-    }
   }
   // Return Register() if all of the possibilities had predicates but none
   // were satisfied.
@@ -699,48 +676,38 @@ void FastISelMap::printFunctionDefinitions(raw_ostream &OS) {
     const OperandsSignature &Operands = SimplePattern.first;
     const OpcodeTypeRetPredMap &OTM = SimplePattern.second;
 
-    for (const auto &I : OTM) {
-      const std::string &Opcode = I.first;
-      const TypeRetPredMap &TM = I.second;
-
+    for (const auto &[Opcode, TM] : OTM) {
       OS << "// FastEmit functions for " << Opcode << ".\n";
       OS << "\n";
 
       // Emit one function for each opcode,type pair.
-      for (const auto &TI : TM) {
-        MVT::SimpleValueType VT = TI.first;
-        const RetPredMap &RM = TI.second;
+      for (const auto &[VT, RM] : TM) {
         if (RM.size() != 1) {
-          for (const auto &RI : RM) {
-            MVT::SimpleValueType RetVT = RI.first;
-            const PredMap &PM = RI.second;
-
+          for (const auto &[RetVT, PM] : RM) {
             OS << "Register fastEmit_" << getLegalCName(Opcode) << "_"
-               << getLegalCName(getEnumName(VT).str()) << "_"
-               << getLegalCName(getEnumName(RetVT).str()) << "_";
+               << getLegalCName(getEnumName(VT)) << "_"
+               << getLegalCName(getEnumName(RetVT)) << "_";
             Operands.PrintManglingSuffix(OS, ImmediatePredicates);
             OS << "(";
             Operands.PrintParameters(OS);
             OS << ") {\n";
 
-            emitInstructionCode(OS, Operands, PM, getEnumName(RetVT).str());
+            emitInstructionCode(OS, Operands, PM, getEnumName(RetVT));
           }
 
           // Emit one function for the type that demultiplexes on return type.
           OS << "Register fastEmit_" << getLegalCName(Opcode) << "_"
-             << getLegalCName(getEnumName(VT).str()) << "_";
+             << getLegalCName(getEnumName(VT)) << "_";
           Operands.PrintManglingSuffix(OS, ImmediatePredicates);
           OS << "(MVT RetVT";
           if (!Operands.empty())
             OS << ", ";
           Operands.PrintParameters(OS);
           OS << ") {\nswitch (RetVT.SimpleTy) {\n";
-          for (const auto &RI : RM) {
-            MVT::SimpleValueType RetVT = RI.first;
+          for (const auto &[RetVT, _] : RM) {
             OS << "  case " << getEnumName(RetVT) << ": return fastEmit_"
-               << getLegalCName(Opcode) << "_"
-               << getLegalCName(getEnumName(VT).str()) << "_"
-               << getLegalCName(getEnumName(RetVT).str()) << "_";
+               << getLegalCName(Opcode) << "_" << getLegalCName(getEnumName(VT))
+               << "_" << getLegalCName(getEnumName(RetVT)) << "_";
             Operands.PrintManglingSuffix(OS, ImmediatePredicates);
             OS << "(";
             Operands.PrintArguments(OS);
@@ -751,7 +718,7 @@ void FastISelMap::printFunctionDefinitions(raw_ostream &OS) {
         } else {
           // Non-variadic return type.
           OS << "Register fastEmit_" << getLegalCName(Opcode) << "_"
-             << getLegalCName(getEnumName(VT).str()) << "_";
+             << getLegalCName(getEnumName(VT)) << "_";
           Operands.PrintManglingSuffix(OS, ImmediatePredicates);
           OS << "(MVT RetVT";
           if (!Operands.empty())
@@ -777,9 +744,8 @@ void FastISelMap::printFunctionDefinitions(raw_ostream &OS) {
       Operands.PrintParameters(OS);
       OS << ") {\n";
       OS << "  switch (VT.SimpleTy) {\n";
-      for (const auto &TI : TM) {
-        MVT::SimpleValueType VT = TI.first;
-        std::string TypeName = getEnumName(VT).str();
+      for (const auto &[VT, _] : TM) {
+        StringRef TypeName = getEnumName(VT);
         OS << "  case " << TypeName << ": return fastEmit_"
            << getLegalCName(Opcode) << "_" << getLegalCName(TypeName) << "_";
         Operands.PrintManglingSuffix(OS, ImmediatePredicates);
@@ -825,15 +791,15 @@ void FastISelMap::printFunctionDefinitions(raw_ostream &OS) {
       // Check each in order it was seen.  It would be nice to have a good
       // relative ordering between them, but we're not going for optimality
       // here.
-      for (unsigned i = 0, e = MI->second.size(); i != e; ++i) {
+      for (const OperandsSignature &Sig : MI->second) {
         OS << "  if (";
-        MI->second[i].emitImmediatePredicate(OS, ImmediatePredicates);
+        Sig.emitImmediatePredicate(OS, ImmediatePredicates);
         OS << ")\n    if (Register Reg = fastEmit_";
-        MI->second[i].PrintManglingSuffix(OS, ImmediatePredicates);
+        Sig.PrintManglingSuffix(OS, ImmediatePredicates);
         OS << "(VT, RetVT, Opcode";
-        if (!MI->second[i].empty())
+        if (!Sig.empty())
           OS << ", ";
-        MI->second[i].PrintArguments(OS);
+        Sig.PrintArguments(OS);
         OS << "))\n      return Reg;\n\n";
       }
 
@@ -842,9 +808,7 @@ void FastISelMap::printFunctionDefinitions(raw_ostream &OS) {
     }
 
     OS << "  switch (Opcode) {\n";
-    for (const auto &I : OTM) {
-      const std::string &Opcode = I.first;
-
+    for (const auto &[Opcode, _] : OTM) {
       OS << "  case " << Opcode << ": return fastEmit_" << getLegalCName(Opcode)
          << "_";
       Operands.PrintManglingSuffix(OS, ImmediatePredicates);

``````````

</details>


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


More information about the llvm-commits mailing list