[llvm] [TableGen][SubtargetEmitter] Remove isSubClassOf check for WriteResDefs and ReadAdvanceDefs where possible (PR #92567)

Michael Maitland via llvm-commits llvm-commits at lists.llvm.org
Fri May 17 09:01:48 PDT 2024


https://github.com/michaelmaitland created https://github.com/llvm/llvm-project/pull/92567

WriteResDefs contained WriteRes and SchedWriteRes records. WriteRes and SchedWriteRes are not subclasses of eachother.

ReadAdvanceDefs contained ReadAdvance and SchedReadAdvance records. ReadAdvance and SchedReadAdvance are not subclasses of eachother.

FindWriteResources and FindReadAdvance are the only users of WriteResDefs and ReadAdvanceDefs respectively. These functions call isSubClassOf to skip records that are not WriteRes or ReadAdvance. That means that SchedWriteRes and SchedReadAdvance records are added to the list but always skipped over.

We opt to not put them in the list in the first place so we can remove the isSubClassOf checks in FindWriteResources and FindReadAdvance.

We still call addWriteRes and addReadAdvance for SchedWriteRes and SchedReadAdvance because those functions perform error checking and discover processor resources.

If we were to assume that there is no SchedWriteRes that also `isSubClassOf(WriteRes)`, and there is no SchedReadAdvance that is also `isSubClassOf(ReadAdvance)`, then we could remove the `isSubClassOf` checks passed to `addWriteRes` and `addReadAdvance` where the def is a SchedWriteRes or SchedReadAdvance. Although unlikely, I think its possible for a subtarget to define a `Foo : SchedWriteRes, WriteRes`. Feedback on whether we can make this assumption would be appreciated.

This improves gen-subtarget on RISC-V by 1% upstream and by 10% downstream.

>From 7f0f63062b8b341bd533feb59a99ff1428e62830 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Fri, 17 May 2024 06:39:40 -0700
Subject: [PATCH] Remove isSubClassOf check in FindWriteResources and
 FindReadAdvance

WriteResDefs contained WriteRes and SchedWriteRes records. WriteRes and
SchedWriteRes are not subclasses of eachother.

ReadAdvanceDefs contained ReadAdvance and SchedReadAdvance records.
ReadAdvance and SchedReadAdvance are not subclasses of eachother.

FindWriteResources and FindReadAdvance are the only users of
WriteResDefs and ReadAdvanceDefs respectively. These functions call
isSubClassOf to skip records that are not WriteRes or ReadAdvance. That
means that SchedWriteRes and SchedReadAdvance records are added to the
list but always skipped over.

We opt to not put them in the list in the first place so we can remove the
isSubClassOf checks in FindWriteResources and FindReadAdvance.

We still call addWriteRes and addReadAdvance for SchedWriteRes and
SchedReadAdvance because those functions perform error checking and
discover processor resources.

This improves gen-subtarget on RISC-V by 10%.
---
 .../utils/TableGen/Common/CodeGenSchedule.cpp | 59 +++++++++++--------
 llvm/utils/TableGen/Common/CodeGenSchedule.h  | 16 ++++-
 llvm/utils/TableGen/SubtargetEmitter.cpp      |  4 --
 3 files changed, 47 insertions(+), 32 deletions(-)

diff --git a/llvm/utils/TableGen/Common/CodeGenSchedule.cpp b/llvm/utils/TableGen/Common/CodeGenSchedule.cpp
index 2ec0812320d14..60e0826bd8546 100644
--- a/llvm/utils/TableGen/Common/CodeGenSchedule.cpp
+++ b/llvm/utils/TableGen/Common/CodeGenSchedule.cpp
@@ -1903,23 +1903,25 @@ void CodeGenSchedModels::collectProcResources() {
   RecVec WRDefs = Records.getAllDerivedDefinitions("WriteRes");
   for (Record *WR : WRDefs) {
     Record *ModelDef = WR->getValueAsDef("SchedModel");
-    addWriteRes(WR, getProcModel(ModelDef).Index);
+    addWriteRes(WR, getProcModel(ModelDef).Index, true);
   }
   RecVec SWRDefs = Records.getAllDerivedDefinitions("SchedWriteRes");
   for (Record *SWR : SWRDefs) {
     Record *ModelDef = SWR->getValueAsDef("SchedModel");
-    addWriteRes(SWR, getProcModel(ModelDef).Index);
+    addWriteRes(SWR, getProcModel(ModelDef).Index,
+                SWR->isSubClassOf("WriteRes"));
   }
   RecVec RADefs = Records.getAllDerivedDefinitions("ReadAdvance");
   for (Record *RA : RADefs) {
     Record *ModelDef = RA->getValueAsDef("SchedModel");
-    addReadAdvance(RA, getProcModel(ModelDef).Index);
+    addReadAdvance(RA, getProcModel(ModelDef).Index, true);
   }
   RecVec SRADefs = Records.getAllDerivedDefinitions("SchedReadAdvance");
   for (Record *SRA : SRADefs) {
     if (SRA->getValueInit("SchedModel")->isComplete()) {
       Record *ModelDef = SRA->getValueAsDef("SchedModel");
-      addReadAdvance(SRA, getProcModel(ModelDef).Index);
+      addReadAdvance(SRA, getProcModel(ModelDef).Index,
+                     SRA->isSubClassOf("ReadAdvance"));
     }
   }
   // Add ProcResGroups that are defined within this processor model, which may
@@ -1948,18 +1950,11 @@ void CodeGenSchedModels::collectProcResources() {
     LLVM_DEBUG(
         PM.dump(); dbgs() << "WriteResDefs: "; for (auto WriteResDef
                                                     : PM.WriteResDefs) {
-          if (WriteResDef->isSubClassOf("WriteRes"))
-            dbgs() << WriteResDef->getValueAsDef("WriteType")->getName() << " ";
-          else
-            dbgs() << WriteResDef->getName() << " ";
+          dbgs() << WriteResDef->getValueAsDef("WriteType")->getName() << " ";
         } dbgs() << "\nReadAdvanceDefs: ";
         for (Record *ReadAdvanceDef
              : PM.ReadAdvanceDefs) {
-          if (ReadAdvanceDef->isSubClassOf("ReadAdvance"))
-            dbgs() << ReadAdvanceDef->getValueAsDef("ReadType")->getName()
-                   << " ";
-          else
-            dbgs() << ReadAdvanceDef->getName() << " ";
+          dbgs() << ReadAdvanceDef->getValueAsDef("ReadType")->getName() << " ";
         } dbgs()
         << "\nProcResourceDefs: ";
         for (Record *ProcResourceDef
@@ -2062,10 +2057,12 @@ void CodeGenSchedModels::collectRWResources(unsigned RWIdx, bool IsRead,
   if (SchedRW.TheDef) {
     if (!IsRead && SchedRW.TheDef->isSubClassOf("SchedWriteRes")) {
       for (unsigned Idx : ProcIndices)
-        addWriteRes(SchedRW.TheDef, Idx);
+        addWriteRes(SchedRW.TheDef, Idx,
+                    SchedRW.TheDef->isSubClassOf("WriteRes"));
     } else if (IsRead && SchedRW.TheDef->isSubClassOf("SchedReadAdvance")) {
       for (unsigned Idx : ProcIndices)
-        addReadAdvance(SchedRW.TheDef, Idx);
+        addReadAdvance(SchedRW.TheDef, Idx,
+                       SchedRW.TheDef->isSubClassOf("ReadAdvance"));
     }
   }
   for (auto *Alias : SchedRW.Aliases) {
@@ -2160,13 +2157,19 @@ void CodeGenSchedModels::addProcResource(Record *ProcResKind,
 }
 
 // Add resources for a SchedWrite to this processor if they don't exist.
-void CodeGenSchedModels::addWriteRes(Record *ProcWriteResDef, unsigned PIdx) {
+// IsWriteRes is true if ProcWriteResDef->isSubClassOf("WriteRes").
+void CodeGenSchedModels::addWriteRes(Record *ProcWriteResDef, unsigned PIdx,
+                                     bool IsWriteRes) {
   assert(PIdx && "don't add resources to an invalid Processor model");
 
-  RecVec &WRDefs = ProcModels[PIdx].WriteResDefs;
-  if (is_contained(WRDefs, ProcWriteResDef))
-    return;
-  WRDefs.push_back(ProcWriteResDef);
+  // WriteResDefs only tracks WriteRes, not SchedWriteRes, unless someone made
+  // a custom subclass that inherited ReadAdvance and SchedReadAdvance.
+  if (IsWriteRes) {
+    RecVec &WRDefs = ProcModels[PIdx].WriteResDefs;
+    if (is_contained(WRDefs, ProcWriteResDef))
+      return;
+    WRDefs.push_back(ProcWriteResDef);
+  }
 
   // Visit ProcResourceKinds referenced by the newly discovered WriteRes.
   RecVec ProcResDefs = ProcWriteResDef->getValueAsListOfDefs("ProcResources");
@@ -2176,8 +2179,9 @@ void CodeGenSchedModels::addWriteRes(Record *ProcWriteResDef, unsigned PIdx) {
 }
 
 // Add resources for a ReadAdvance to this processor if they don't exist.
+// IsReadAdvance is true if ProcReadAdvanceDef->isSubClassOf("ReadAdvance").
 void CodeGenSchedModels::addReadAdvance(Record *ProcReadAdvanceDef,
-                                        unsigned PIdx) {
+                                        unsigned PIdx, bool IsReadAdvance) {
   for (const Record *ValidWrite :
        ProcReadAdvanceDef->getValueAsListOfDefs("ValidWrites"))
     if (getSchedRWIdx(ValidWrite, /*IsRead=*/false) == 0)
@@ -2187,10 +2191,15 @@ void CodeGenSchedModels::addReadAdvance(Record *ProcReadAdvanceDef,
           "any instruction (" +
               ValidWrite->getName() + ")");
 
-  RecVec &RADefs = ProcModels[PIdx].ReadAdvanceDefs;
-  if (is_contained(RADefs, ProcReadAdvanceDef))
-    return;
-  RADefs.push_back(ProcReadAdvanceDef);
+  // ReadAdvanceDefs only tracks ReadAdvance, not SchedReadAdvance, unless
+  // someone made a custom subclass that inherited ReadAdvance and
+  // SchedReadAdvance.
+  if (IsReadAdvance) {
+    RecVec &RADefs = ProcModels[PIdx].ReadAdvanceDefs;
+    if (is_contained(RADefs, ProcReadAdvanceDef))
+      return;
+    RADefs.push_back(ProcReadAdvanceDef);
+  }
 }
 
 unsigned CodeGenProcModel::getProcResourceIdx(Record *PRDef) const {
diff --git a/llvm/utils/TableGen/Common/CodeGenSchedule.h b/llvm/utils/TableGen/Common/CodeGenSchedule.h
index 10ec7f41f56ff..e332135907eb8 100644
--- a/llvm/utils/TableGen/Common/CodeGenSchedule.h
+++ b/llvm/utils/TableGen/Common/CodeGenSchedule.h
@@ -239,8 +239,13 @@ struct CodeGenProcModel {
   // This list is empty if the Processor has no UnsupportedFeatures.
   RecVec UnsupportedFeaturesDefs;
 
-  // All read/write resources associated with this processor.
+  // All WriteRes records associated with this processor. Does not contain
+  // SchedWriteRes records (unless a custom class that inherits both was used).
   RecVec WriteResDefs;
+
+  // All ReadAdvance records associated with this processor. Does not contain
+  // SchedReadAdvance records (unless a custom class that inherits both was
+  // used).
   RecVec ReadAdvanceDefs;
 
   // Per-operand machine model resources associated with this processor.
@@ -644,9 +649,14 @@ class CodeGenSchedModels {
   void addProcResource(Record *ProcResourceKind, CodeGenProcModel &PM,
                        ArrayRef<SMLoc> Loc);
 
-  void addWriteRes(Record *ProcWriteResDef, unsigned PIdx);
+  // Add resources for a SchedWrite to this processor if they don't exist.
+  // IsWriteRes is true if ProcWriteResDef isSubClassOf("WriteRes").
+  void addWriteRes(Record *ProcWriteResDef, unsigned PIdx, bool IsWriteRes);
 
-  void addReadAdvance(Record *ProcReadAdvanceDef, unsigned PIdx);
+  // Add resources for a ReadAdvance to this processor if they don't exist.
+  // IsReadAdvance is true if ProcReadAdvanceDef->isSubClassOf("ReadAdvance").
+  void addReadAdvance(Record *ProcReadAdvanceDef, unsigned PIdx,
+                      bool IsReadAdvance);
 };
 
 } // namespace llvm
diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 323470940fec5..dc5c026277e41 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -900,8 +900,6 @@ SubtargetEmitter::FindWriteResources(const CodeGenSchedRW &SchedWrite,
   // Check this processor's list of write resources.
   Record *ResDef = nullptr;
   for (Record *WR : ProcModel.WriteResDefs) {
-    if (!WR->isSubClassOf("WriteRes"))
-      continue;
     Record *WRDef = WR->getValueAsDef("WriteType");
     if (AliasDef == WRDef || SchedWrite.TheDef == WRDef) {
       if (ResDef) {
@@ -959,8 +957,6 @@ Record *SubtargetEmitter::FindReadAdvance(const CodeGenSchedRW &SchedRead,
   // Check this processor's ReadAdvanceList.
   Record *ResDef = nullptr;
   for (Record *RA : ProcModel.ReadAdvanceDefs) {
-    if (!RA->isSubClassOf("ReadAdvance"))
-      continue;
     Record *RADef = RA->getValueAsDef("ReadType");
     if (AliasDef == RADef || SchedRead.TheDef == RADef) {
       if (ResDef) {



More information about the llvm-commits mailing list