[llvm] [TableGen][SubtargetEmitter] Early exit from loop in FindWriteResources and FindReadAdvance (PR #92202)

Michael Maitland via llvm-commits llvm-commits at lists.llvm.org
Wed May 15 10:47:50 PDT 2024


https://github.com/michaelmaitland updated https://github.com/llvm/llvm-project/pull/92202

>From 9235b8d019c754b1a56a55ec157c10d522f7c2b1 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Tue, 14 May 2024 18:53:54 -0700
Subject: [PATCH 1/8] [TableGen][SubtargetEmitter] Early exit from loop in
 FindWriteResources

This gives us a 26% speed improvement in our downstream.
---
 llvm/utils/TableGen/SubtargetEmitter.cpp | 25 ++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 9e32d2de19b2c..737b1bdfd58b6 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -904,14 +904,17 @@ SubtargetEmitter::FindWriteResources(const CodeGenSchedRW &SchedWrite,
       continue;
     if (AliasDef == WR->getValueAsDef("WriteType") ||
         SchedWrite.TheDef == WR->getValueAsDef("WriteType")) {
-      if (ResDef) {
-        PrintFatalError(WR->getLoc(), "Resources are defined for both "
-                                      "SchedWrite and its alias on processor " +
-                                          ProcModel.ModelName);
-      }
       ResDef = WR;
+      break;
     }
   }
+
+  if (ResDef && AliasDef) {
+    PrintFatalError(ResDef->getLoc(), "Resources are defined for both "
+                                      "SchedWrite and its alias on processor " +
+                                          ProcModel.ModelName);
+  }
+
   // TODO: If ProcModel has a base model (previous generation processor),
   // then call FindWriteResources recursively with that model here.
   if (!ResDef) {
@@ -958,14 +961,16 @@ Record *SubtargetEmitter::FindReadAdvance(const CodeGenSchedRW &SchedRead,
       continue;
     if (AliasDef == RA->getValueAsDef("ReadType") ||
         SchedRead.TheDef == RA->getValueAsDef("ReadType")) {
-      if (ResDef) {
-        PrintFatalError(RA->getLoc(), "Resources are defined for both "
-                                      "SchedRead and its alias on processor " +
-                                          ProcModel.ModelName);
-      }
       ResDef = RA;
+      break;
     }
   }
+  if (ResDef && AliasDef) {
+    PrintFatalError(ResDef->getLoc(), "Resources are defined for both "
+                                      "SchedRead and its alias on processor " +
+                                          ProcModel.ModelName);
+  }
+
   // TODO: If ProcModel has a base model (previous generation processor),
   // then call FindReadAdvance recursively with that model here.
   if (!ResDef && SchedRead.TheDef->getName() != "ReadDefault") {

>From 6b710d0a16602f012377fe469d0992fd64caaf64 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Wed, 15 May 2024 10:13:07 -0700
Subject: [PATCH 2/8] fixup! make sure we PrintFatalError

---
 llvm/utils/TableGen/SubtargetEmitter.cpp | 37 ++++++++++++++----------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 737b1bdfd58b6..9892f101d0fff 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -902,19 +902,21 @@ SubtargetEmitter::FindWriteResources(const CodeGenSchedRW &SchedWrite,
   for (Record *WR : ProcModel.WriteResDefs) {
     if (!WR->isSubClassOf("WriteRes"))
       continue;
-    if (AliasDef == WR->getValueAsDef("WriteType") ||
-        SchedWrite.TheDef == WR->getValueAsDef("WriteType")) {
+    // If there is no AliasDef and we find a match, we can early exit since
+    // there is no need to verify whether there are resources defined for both
+    // SchedWrite and its alias.
+    if (!AliasDef && SchedWrite.TheDef == WR->getValueAsDef("WriteType")) {
       ResDef = WR;
       break;
-    }
-  }
-
-  if (ResDef && AliasDef) {
-    PrintFatalError(ResDef->getLoc(), "Resources are defined for both "
+    } else if (AliasDef == WR->getValueAsDef("WriteType")) {
+      if (ResDef) {
+        PrintFatalError(WR->getLoc(), "Resources are defined for both "
                                       "SchedWrite and its alias on processor " +
                                           ProcModel.ModelName);
+      }
+      ResDef = WR;
+    }
   }
-
   // TODO: If ProcModel has a base model (previous generation processor),
   // then call FindWriteResources recursively with that model here.
   if (!ResDef) {
@@ -959,18 +961,21 @@ Record *SubtargetEmitter::FindReadAdvance(const CodeGenSchedRW &SchedRead,
   for (Record *RA : ProcModel.ReadAdvanceDefs) {
     if (!RA->isSubClassOf("ReadAdvance"))
       continue;
-    if (AliasDef == RA->getValueAsDef("ReadType") ||
-        SchedRead.TheDef == RA->getValueAsDef("ReadType")) {
-      ResDef = RA;
+    // If there is no AliasDef and we find a match, we can early exit since
+    // there is no need to verify whether there are resources defined for both
+    // SchedWrite and its alias.
+    if (!AliasDef && AliasDef == RA->getValueAsDef("ReadType")) {
+      ResDef = WR;
       break;
-    }
-  }
-  if (ResDef && AliasDef) {
-    PrintFatalError(ResDef->getLoc(), "Resources are defined for both "
+    } else if (SchedRead.TheDef == RA->getValueAsDef("ReadType")) {
+      if (ResDef) {
+        PrintFatalError(RA->getLoc(), "Resources are defined for both "
                                       "SchedRead and its alias on processor " +
                                           ProcModel.ModelName);
+      }
+      ResDef = RA;
+    }
   }
-
   // TODO: If ProcModel has a base model (previous generation processor),
   // then call FindReadAdvance recursively with that model here.
   if (!ResDef && SchedRead.TheDef->getName() != "ReadDefault") {

>From 5212ee108819263c0c56e1e9d2022a5f12e91114 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Wed, 15 May 2024 10:20:44 -0700
Subject: [PATCH 3/8] fixup! fix typo

---
 llvm/utils/TableGen/SubtargetEmitter.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 9892f101d0fff..fb06d05c26f38 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -965,7 +965,7 @@ Record *SubtargetEmitter::FindReadAdvance(const CodeGenSchedRW &SchedRead,
     // there is no need to verify whether there are resources defined for both
     // SchedWrite and its alias.
     if (!AliasDef && AliasDef == RA->getValueAsDef("ReadType")) {
-      ResDef = WR;
+      ResDef = RA;
       break;
     } else if (SchedRead.TheDef == RA->getValueAsDef("ReadType")) {
       if (ResDef) {

>From a802de7dd659639c701bfcccf19d9f08d8a7a0ef Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Wed, 15 May 2024 10:26:54 -0700
Subject: [PATCH 4/8] fixup! no else after break

---
 llvm/utils/TableGen/SubtargetEmitter.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index fb06d05c26f38..2c7e0c923cff8 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -908,7 +908,8 @@ SubtargetEmitter::FindWriteResources(const CodeGenSchedRW &SchedWrite,
     if (!AliasDef && SchedWrite.TheDef == WR->getValueAsDef("WriteType")) {
       ResDef = WR;
       break;
-    } else if (AliasDef == WR->getValueAsDef("WriteType")) {
+    } 
+    if (AliasDef == WR->getValueAsDef("WriteType")) {
       if (ResDef) {
         PrintFatalError(WR->getLoc(), "Resources are defined for both "
                                       "SchedWrite and its alias on processor " +
@@ -967,7 +968,8 @@ Record *SubtargetEmitter::FindReadAdvance(const CodeGenSchedRW &SchedRead,
     if (!AliasDef && AliasDef == RA->getValueAsDef("ReadType")) {
       ResDef = RA;
       break;
-    } else if (SchedRead.TheDef == RA->getValueAsDef("ReadType")) {
+    } 
+    if (SchedRead.TheDef == RA->getValueAsDef("ReadType")) {
       if (ResDef) {
         PrintFatalError(RA->getLoc(), "Resources are defined for both "
                                       "SchedRead and its alias on processor " +

>From 56d79aa4211658237593a485158e4ab6d4cea050 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Wed, 15 May 2024 10:28:36 -0700
Subject: [PATCH 5/8] fixup! swap conditions

---
 llvm/utils/TableGen/SubtargetEmitter.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 2c7e0c923cff8..12ed3392fe405 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -965,11 +965,11 @@ Record *SubtargetEmitter::FindReadAdvance(const CodeGenSchedRW &SchedRead,
     // If there is no AliasDef and we find a match, we can early exit since
     // there is no need to verify whether there are resources defined for both
     // SchedWrite and its alias.
-    if (!AliasDef && AliasDef == RA->getValueAsDef("ReadType")) {
+    if (!AliasDef && SchedRead.TheDef == RA->getValueAsDef("ReadType")) {
       ResDef = RA;
       break;
-    } 
-    if (SchedRead.TheDef == RA->getValueAsDef("ReadType")) {
+    }
+    if (AliasDef == RA->getValueAsDef("ReadType")) {
       if (ResDef) {
         PrintFatalError(RA->getLoc(), "Resources are defined for both "
                                       "SchedRead and its alias on processor " +

>From d70b2015c53b1247e876b49aa44c45c5edd43d2c Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Wed, 15 May 2024 10:29:51 -0700
Subject: [PATCH 6/8] fixup! SchedWrite -> SchedRead typo

---
 llvm/utils/TableGen/SubtargetEmitter.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 12ed3392fe405..4409e6bb40b29 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -964,7 +964,7 @@ Record *SubtargetEmitter::FindReadAdvance(const CodeGenSchedRW &SchedRead,
       continue;
     // If there is no AliasDef and we find a match, we can early exit since
     // there is no need to verify whether there are resources defined for both
-    // SchedWrite and its alias.
+    // SchedRead and its alias.
     if (!AliasDef && SchedRead.TheDef == RA->getValueAsDef("ReadType")) {
       ResDef = RA;
       break;

>From 775f64471501a10d3f53702f8b96dd24d427b027 Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Wed, 15 May 2024 10:31:54 -0700
Subject: [PATCH 7/8] fixup! cache local variable

---
 llvm/utils/TableGen/SubtargetEmitter.cpp | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 4409e6bb40b29..7729164172166 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -905,11 +905,12 @@ SubtargetEmitter::FindWriteResources(const CodeGenSchedRW &SchedWrite,
     // If there is no AliasDef and we find a match, we can early exit since
     // there is no need to verify whether there are resources defined for both
     // SchedWrite and its alias.
-    if (!AliasDef && SchedWrite.TheDef == WR->getValueAsDef("WriteType")) {
+    Record *WRDef = WR->getValueAsDef("WriteType");
+    if (!AliasDef && SchedWrite.TheDef == WRDef) {
       ResDef = WR;
       break;
-    } 
-    if (AliasDef == WR->getValueAsDef("WriteType")) {
+    }
+    if (AliasDef == WRDef) {
       if (ResDef) {
         PrintFatalError(WR->getLoc(), "Resources are defined for both "
                                       "SchedWrite and its alias on processor " +
@@ -965,11 +966,12 @@ Record *SubtargetEmitter::FindReadAdvance(const CodeGenSchedRW &SchedRead,
     // If there is no AliasDef and we find a match, we can early exit since
     // there is no need to verify whether there are resources defined for both
     // SchedRead and its alias.
-    if (!AliasDef && SchedRead.TheDef == RA->getValueAsDef("ReadType")) {
+    Record *RADef = RA->getValueAsDef("ReadType");
+    if (!AliasDef && SchedRead.TheDef == RADef) {
       ResDef = RA;
       break;
     }
-    if (AliasDef == RA->getValueAsDef("ReadType")) {
+    if (AliasDef == RADef) {
       if (ResDef) {
         PrintFatalError(RA->getLoc(), "Resources are defined for both "
                                       "SchedRead and its alias on processor " +

>From 4ffd8fa9e80e110715addf32fd52503de7d8503d Mon Sep 17 00:00:00 2001
From: Michael Maitland <michaeltmaitland at gmail.com>
Date: Wed, 15 May 2024 10:46:12 -0700
Subject: [PATCH 8/8] fixup! Assign ResDef if ALiasDef is non-null

---
 llvm/utils/TableGen/SubtargetEmitter.cpp | 28 ++++++++++--------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/llvm/utils/TableGen/SubtargetEmitter.cpp b/llvm/utils/TableGen/SubtargetEmitter.cpp
index 7729164172166..606b1e799b9ae 100644
--- a/llvm/utils/TableGen/SubtargetEmitter.cpp
+++ b/llvm/utils/TableGen/SubtargetEmitter.cpp
@@ -902,21 +902,19 @@ SubtargetEmitter::FindWriteResources(const CodeGenSchedRW &SchedWrite,
   for (Record *WR : ProcModel.WriteResDefs) {
     if (!WR->isSubClassOf("WriteRes"))
       continue;
-    // If there is no AliasDef and we find a match, we can early exit since
-    // there is no need to verify whether there are resources defined for both
-    // SchedWrite and its alias.
     Record *WRDef = WR->getValueAsDef("WriteType");
-    if (!AliasDef && SchedWrite.TheDef == WRDef) {
-      ResDef = WR;
-      break;
-    }
-    if (AliasDef == WRDef) {
+    if (AlisDef == WRDef || SchedWrite.TheDef == WRDef) {
       if (ResDef) {
         PrintFatalError(WR->getLoc(), "Resources are defined for both "
                                       "SchedWrite and its alias on processor " +
                                           ProcModel.ModelName);
       }
       ResDef = WR;
+      // If there is no AliasDef and we find a match, we can early exit since
+      // there is no need to verify whether there are resources defined for both
+      // SchedWrite and its alias.
+      if (!AliasDef)
+        break;
     }
   }
   // TODO: If ProcModel has a base model (previous generation processor),
@@ -963,21 +961,19 @@ Record *SubtargetEmitter::FindReadAdvance(const CodeGenSchedRW &SchedRead,
   for (Record *RA : ProcModel.ReadAdvanceDefs) {
     if (!RA->isSubClassOf("ReadAdvance"))
       continue;
-    // If there is no AliasDef and we find a match, we can early exit since
-    // there is no need to verify whether there are resources defined for both
-    // SchedRead and its alias.
     Record *RADef = RA->getValueAsDef("ReadType");
-    if (!AliasDef && SchedRead.TheDef == RADef) {
-      ResDef = RA;
-      break;
-    }
-    if (AliasDef == RADef) {
+    if (AliasDef == RADef || SchedRead.TheDef == RADef) {
       if (ResDef) {
         PrintFatalError(RA->getLoc(), "Resources are defined for both "
                                       "SchedRead and its alias on processor " +
                                           ProcModel.ModelName);
       }
       ResDef = RA;
+      // If there is no AliasDef and we find a match, we can early exit since
+      // there is no need to verify whether there are resources defined for both
+      // SchedRead and its alias.
+      if (!AliasDef)
+        break;
     }
   }
   // TODO: If ProcModel has a base model (previous generation processor),



More information about the llvm-commits mailing list