Skip to content

Conversation

@jurahul
Copy link
Contributor

@jurahul jurahul commented Feb 16, 2025

  • Use range for loops for processor models and schedule classes.
  • Cleanup duplicated or unused iterators in CodeGenSchedule.h

- Use range for loops for iterating over processor models and
  schedule classes.
- Cleanup duplicated or unused iterators in CodeGenSchedule.h
@jurahul jurahul force-pushed the codegen_schedule_cleanup_iterators branch from a3cf00f to 2517fd6 Compare February 16, 2025 16:33
@jurahul jurahul marked this pull request as ready for review February 16, 2025 17:28
@llvmbot
Copy link
Member

llvmbot commented Feb 16, 2025

@llvm/pr-subscribers-tablegen

Author: Rahul Joshi (jurahul)

Changes
  • Use range for loops for iterating over processor models and schedule classes.
  • Cleanup duplicated or unused iterators in CodeGenSchedule.h

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

3 Files Affected:

  • (modified) llvm/utils/TableGen/Common/CodeGenSchedule.h (+5-26)
  • (modified) llvm/utils/TableGen/InstrInfoEmitter.cpp (+1-1)
  • (modified) llvm/utils/TableGen/SubtargetEmitter.cpp (+36-52)
diff --git a/llvm/utils/TableGen/Common/CodeGenSchedule.h b/llvm/utils/TableGen/Common/CodeGenSchedule.h
index 981782c17c48c..8343257b458dd 100644
--- a/llvm/utils/TableGen/Common/CodeGenSchedule.h
+++ b/llvm/utils/TableGen/Common/CodeGenSchedule.h
@@ -467,23 +467,6 @@ class CodeGenSchedModels {
 public:
   CodeGenSchedModels(const RecordKeeper &RK, const CodeGenTarget &TGT);
 
-  // iterator access to the scheduling classes.
-  using class_iterator = std::vector<CodeGenSchedClass>::iterator;
-  using const_class_iterator = std::vector<CodeGenSchedClass>::const_iterator;
-  class_iterator classes_begin() { return SchedClasses.begin(); }
-  const_class_iterator classes_begin() const { return SchedClasses.begin(); }
-  class_iterator classes_end() { return SchedClasses.end(); }
-  const_class_iterator classes_end() const { return SchedClasses.end(); }
-  iterator_range<class_iterator> classes() {
-    return make_range(classes_begin(), classes_end());
-  }
-  iterator_range<const_class_iterator> classes() const {
-    return make_range(classes_begin(), classes_end());
-  }
-  ArrayRef<CodeGenSchedClass> explicit_classes() const {
-    return ArrayRef(SchedClasses).take_front(NumInstrSchedClasses);
-  }
-
   const Record *getModelOrItinDef(const Record *ProcDef) const {
     const Record *ModelDef = ProcDef->getValueAsDef("SchedModel");
     const Record *ItinsDef = ProcDef->getValueAsDef("ProcItin");
@@ -497,13 +480,13 @@ class CodeGenSchedModels {
 
   const CodeGenProcModel &getModelForProc(const Record *ProcDef) const {
     const Record *ModelDef = getModelOrItinDef(ProcDef);
-    ProcModelMapTy::const_iterator I = ProcModelMap.find(ModelDef);
+    auto I = ProcModelMap.find(ModelDef);
     assert(I != ProcModelMap.end() && "missing machine model");
     return ProcModels[I->second];
   }
 
   const CodeGenProcModel &getProcModel(const Record *ModelDef) const {
-    ProcModelMapTy::const_iterator I = ProcModelMap.find(ModelDef);
+    auto I = ProcModelMap.find(ModelDef);
     assert(I != ProcModelMap.end() && "missing machine model");
     return ProcModels[I->second];
   }
@@ -512,10 +495,6 @@ class CodeGenSchedModels {
         static_cast<const CodeGenSchedModels &>(*this).getProcModel(ModelDef));
   }
 
-  // Iterate over the unique processor models.
-  using ProcIter = std::vector<CodeGenProcModel>::const_iterator;
-  ProcIter procModelBegin() const { return ProcModels.begin(); }
-  ProcIter procModelEnd() const { return ProcModels.end(); }
   ArrayRef<CodeGenProcModel> procModels() const { return ProcModels; }
 
   // Return true if any processors have itineraries.
@@ -564,10 +543,10 @@ class CodeGenSchedModels {
   // for NoItinerary.
   unsigned getSchedClassIdx(const CodeGenInstruction &Inst) const;
 
-  using SchedClassIter = std::vector<CodeGenSchedClass>::const_iterator;
-  SchedClassIter schedClassBegin() const { return SchedClasses.begin(); }
-  SchedClassIter schedClassEnd() const { return SchedClasses.end(); }
   ArrayRef<CodeGenSchedClass> schedClasses() const { return SchedClasses; }
+  ArrayRef<CodeGenSchedClass> explicitSchedClasses() const {
+    return schedClasses().take_front(NumInstrSchedClasses);
+  }
 
   unsigned numInstrSchedClasses() const { return NumInstrSchedClasses; }
 
diff --git a/llvm/utils/TableGen/InstrInfoEmitter.cpp b/llvm/utils/TableGen/InstrInfoEmitter.cpp
index 3ea76ed414d91..377bfb593be5f 100644
--- a/llvm/utils/TableGen/InstrInfoEmitter.cpp
+++ b/llvm/utils/TableGen/InstrInfoEmitter.cpp
@@ -1244,7 +1244,7 @@ void InstrInfoEmitter::emitEnums(
   OS << "#undef GET_INSTRINFO_SCHED_ENUM\n";
   OS << "namespace llvm::" << Namespace << "::Sched {\n\n";
   OS << "  enum {\n";
-  auto ExplictClasses = SchedModels.explicit_classes();
+  auto ExplictClasses = SchedModels.explicitSchedClasses();
   for (const auto &[Idx, Class] : enumerate(ExplictClasses))
     OS << "    " << Class.Name << "\t= " << Idx << ",\n";
   OS << "    SCHED_LIST_END = " << ExplictClasses.size() << '\n';
diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 49362ff5ef655..c54e00cdb0ce1 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -106,7 +106,7 @@ class SubtargetEmitter {
   void emitStageAndOperandCycleData(
       raw_ostream &OS, std::vector<std::vector<InstrItinerary>> &ProcItinLists);
   void emitItineraries(raw_ostream &OS,
-                       std::vector<std::vector<InstrItinerary>> &ProcItinLists);
+                       ArrayRef<std::vector<InstrItinerary>> ProcItinLists);
   unsigned emitRegisterFileTables(const CodeGenProcModel &ProcModel,
                                   raw_ostream &OS);
   void emitLoadStoreQueueInfo(const CodeGenProcModel &ProcModel,
@@ -477,7 +477,6 @@ void SubtargetEmitter::emitStageAndOperandCycleData(
 
   // Emit functional units for all the itineraries.
   for (const CodeGenProcModel &ProcModel : SchedModels.procModels()) {
-
     if (!ItinsDefSet.insert(ProcModel.ItinsDef).second)
       continue;
 
@@ -489,25 +488,23 @@ void SubtargetEmitter::emitStageAndOperandCycleData(
     OS << "\n// Functional units for \"" << Name << "\"\n"
        << "namespace " << Name << "FU {\n";
 
-    for (unsigned J = 0, FUN = FUs.size(); J < FUN; ++J)
-      OS << "  const InstrStage::FuncUnits " << FUs[J]->getName()
-         << " = 1ULL << " << J << ";\n";
+    for (const auto &[Idx, FU] : enumerate(FUs))
+      OS << "  const InstrStage::FuncUnits " << FU->getName() << " = 1ULL << "
+         << Idx << ";\n";
 
     OS << "} // end namespace " << Name << "FU\n";
 
     ConstRecVec BPs = ProcModel.ItinsDef->getValueAsListOfDefs("BP");
-    if (!BPs.empty()) {
-      OS << "\n// Pipeline forwarding paths for itineraries \"" << Name
-         << "\"\n"
-         << "namespace " << Name << "Bypass {\n";
+    if (BPs.empty())
+      continue;
+    OS << "\n// Pipeline forwarding paths for itineraries \"" << Name << "\"\n"
+       << "namespace " << Name << "Bypass {\n";
 
-      OS << "  const unsigned NoBypass = 0;\n";
-      for (unsigned J = 0, BPN = BPs.size(); J < BPN; ++J)
-        OS << "  const unsigned " << BPs[J]->getName() << " = 1 << " << J
-           << ";\n";
+    OS << "  const unsigned NoBypass = 0;\n";
+    for (const auto &[Idx, BP] : enumerate(BPs))
+      OS << "  const unsigned " << BP->getName() << " = 1 << " << Idx << ";\n";
 
-      OS << "} // end namespace " << Name << "Bypass\n";
-    }
+    OS << "} // end namespace " << Name << "Bypass\n";
   }
 
   // Begin stages table
@@ -647,46 +644,39 @@ void SubtargetEmitter::emitStageAndOperandCycleData(
 // CodeGenSchedClass::Index.
 //
 void SubtargetEmitter::emitItineraries(
-    raw_ostream &OS, std::vector<std::vector<InstrItinerary>> &ProcItinLists) {
+    raw_ostream &OS, ArrayRef<std::vector<InstrItinerary>> ProcItinLists) {
   // Multiple processor models may share an itinerary record. Emit it once.
   SmallPtrSet<const Record *, 8> ItinsDefSet;
 
-  // For each processor's machine model
-  std::vector<std::vector<InstrItinerary>>::iterator ProcItinListsIter =
-      ProcItinLists.begin();
-  for (CodeGenSchedModels::ProcIter PI = SchedModels.procModelBegin(),
-                                    PE = SchedModels.procModelEnd();
-       PI != PE; ++PI, ++ProcItinListsIter) {
-
-    const Record *ItinsDef = PI->ItinsDef;
+  for (const auto &[Proc, ItinList] :
+       zip_equal(SchedModels.procModels(), ProcItinLists)) {
+    const Record *ItinsDef = Proc.ItinsDef;
     if (!ItinsDefSet.insert(ItinsDef).second)
       continue;
 
-    // Get the itinerary list for the processor.
-    assert(ProcItinListsIter != ProcItinLists.end() && "bad iterator");
-    std::vector<InstrItinerary> &ItinList = *ProcItinListsIter;
-
     // Empty itineraries aren't referenced anywhere in the tablegen output
     // so don't emit them.
     if (ItinList.empty())
       continue;
 
+    // Begin processor itinerary table
     OS << "\n";
-    OS << "static const llvm::InstrItinerary ";
+    OS << "static constexpr llvm::InstrItinerary " << ItinsDef->getName()
+       << "[] = {\n";
 
-    // Begin processor itinerary table
-    OS << ItinsDef->getName() << "[] = {\n";
+    ArrayRef<CodeGenSchedClass> ItinSchedClasses =
+        SchedModels.schedClasses().take_front(ItinList.size());
 
     // For each itinerary class in CodeGenSchedClass::Index order.
-    for (unsigned J = 0, M = ItinList.size(); J < M; ++J) {
-      InstrItinerary &Intinerary = ItinList[J];
-
+    for (const auto &[Idx, Intinerary, SchedClass] :
+         enumerate(ItinList, ItinSchedClasses)) {
       // Emit Itinerary in the form of
-      // { firstStage, lastStage, firstCycle, lastCycle } // index
+      // { NumMicroOps, FirstStage, LastStage, FirstOperandCycle,
+      // LastOperandCycle } // index class name
       OS << "  { " << Intinerary.NumMicroOps << ", " << Intinerary.FirstStage
          << ", " << Intinerary.LastStage << ", " << Intinerary.FirstOperandCycle
-         << ", " << Intinerary.LastOperandCycle << " }"
-         << ", // " << J << " " << SchedModels.getSchedClass(J).Name << "\n";
+         << ", " << Intinerary.LastOperandCycle << " }" << ", // " << Idx << " "
+         << SchedClass.Name << "\n";
     }
     // End processor itinerary table
     OS << "  { 0, uint16_t(~0U), uint16_t(~0U), uint16_t(~0U), uint16_t(~0U) }"
@@ -1438,18 +1428,16 @@ void SubtargetEmitter::emitSchedClassTables(SchedClassTables &SchedTables,
   OS << "}; // " << Target << "ReadAdvanceTable\n";
 
   // Emit a SchedClass table for each processor.
-  for (CodeGenSchedModels::ProcIter PI = SchedModels.procModelBegin(),
-                                    PE = SchedModels.procModelEnd();
-       PI != PE; ++PI) {
-    if (!PI->hasInstrSchedModel())
+  for (const auto &[Idx, Proc] : enumerate(SchedModels.procModels())) {
+    if (!Proc.hasInstrSchedModel())
       continue;
 
     std::vector<MCSchedClassDesc> &SCTab =
-        SchedTables.ProcSchedClasses[1 + (PI - SchedModels.procModelBegin())];
+        SchedTables.ProcSchedClasses[1 + Idx];
 
     OS << "\n// {Name, NumMicroOps, BeginGroup, EndGroup, RetireOOO,"
        << " WriteProcResIdx,#, WriteLatencyIdx,#, ReadAdvanceIdx,#}\n";
-    OS << "static const llvm::MCSchedClassDesc " << PI->ModelName
+    OS << "static const llvm::MCSchedClassDesc " << Proc.ModelName
        << "SchedClasses[] = {\n";
 
     // The first class is always invalid. We no way to distinguish it except by
@@ -1476,7 +1464,7 @@ void SubtargetEmitter::emitSchedClassTables(SchedClassTables &SchedTables,
          << format("%2d", MCDesc.ReadAdvanceIdx) << ", "
          << MCDesc.NumReadAdvanceEntries << "}, // #" << SCIdx << '\n';
     }
-    OS << "}; // " << PI->ModelName << "SchedClasses\n";
+    OS << "}; // " << Proc.ModelName << "SchedClasses\n";
   }
 }
 
@@ -1524,14 +1512,10 @@ void SubtargetEmitter::emitProcessorModels(raw_ostream &OS) {
 
     OS << "  " << PM.Index << ", // Processor ID\n";
     if (PM.hasInstrSchedModel())
-      OS << "  " << PM.ModelName << "ProcResources"
-         << ",\n"
-         << "  " << PM.ModelName << "SchedClasses"
-         << ",\n"
+      OS << "  " << PM.ModelName << "ProcResources" << ",\n"
+         << "  " << PM.ModelName << "SchedClasses" << ",\n"
          << "  " << PM.ProcResourceDefs.size() + 1 << ",\n"
-         << "  "
-         << (SchedModels.schedClassEnd() - SchedModels.schedClassBegin())
-         << ",\n";
+         << "  " << SchedModels.schedClasses().size() << ",\n";
     else
       OS << "  nullptr, nullptr, 0, 0,"
          << " // No instruction-level machine model.\n";
@@ -1743,7 +1727,7 @@ void SubtargetEmitter::emitSchedModelHelpersImpl(
                    ? "if (CPUID == "
                    : "if (SchedModel->getProcessorID() == ");
         OS << PI << ") ";
-        OS << "{ // " << (SchedModels.procModelBegin() + PI)->ModelName << '\n';
+        OS << "{ // " << SchedModels.procModels()[PI].ModelName << '\n';
       }
 
       // Now emit transitions associated with processor PI.

Copy link
Collaborator

@topperc topperc 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 1fd0600 into llvm:main Feb 18, 2025
11 checks passed
@jurahul jurahul deleted the codegen_schedule_cleanup_iterators branch February 18, 2025 16:57
wldfngrs pushed a commit to wldfngrs/llvm-project that referenced this pull request Feb 19, 2025
- Use range for loops for processor models and schedule classes.
- Cleanup duplicated or unused iterators in CodeGenSchedule.h
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