[llvm] [TableGen] Split DAGISelMatcherOpt FactorNodes into 2 functions. NFC (PR #125330)

Craig Topper via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 1 22:08:41 PST 2025


https://github.com/topperc updated https://github.com/llvm/llvm-project/pull/125330

>From 91ac2decf947eac8b8137b88efff5f68e11e7da4 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Fri, 31 Jan 2025 19:10:15 -0800
Subject: [PATCH 1/2] TableGen] Split DAGISelMatcherOpt FactorNodes into 2
 functions. NFC

The loop at the top of FactorNodes creates additional variables to
deal with needing to use a pointer to a unique_ptr instead of a
reference. Encapsulate this to its own function for better scoping.

This also allows us to directly skip this loop when we already know
we have a ScopeMatcher.
---
 llvm/utils/TableGen/DAGISelMatcherOpt.cpp | 61 ++++++++++++-----------
 1 file changed, 31 insertions(+), 30 deletions(-)

diff --git a/llvm/utils/TableGen/DAGISelMatcherOpt.cpp b/llvm/utils/TableGen/DAGISelMatcherOpt.cpp
index f747944543cfd0..5193b3c0741937 100644
--- a/llvm/utils/TableGen/DAGISelMatcherOpt.cpp
+++ b/llvm/utils/TableGen/DAGISelMatcherOpt.cpp
@@ -191,34 +191,10 @@ static Matcher *FindNodeWithKind(Matcher *M, Matcher::KindTy Kind) {
   return nullptr;
 }
 
-/// FactorNodes - Turn matches like this:
-///   Scope
-///     OPC_CheckType i32
-///       ABC
-///     OPC_CheckType i32
-///       XYZ
-/// into:
-///   OPC_CheckType i32
-///     Scope
-///       ABC
-///       XYZ
-///
-static void FactorNodes(std::unique_ptr<Matcher> &InputMatcherPtr) {
-  // Look for a push node. Iterates instead of recurses to reduce stack usage.
-  ScopeMatcher *Scope = nullptr;
-  std::unique_ptr<Matcher> *RebindableMatcherPtr = &InputMatcherPtr;
-  while (!Scope) {
-    // If we reached the end of the chain, we're done.
-    Matcher *N = RebindableMatcherPtr->get();
-    if (!N)
-      return;
-
-    // If this is not a push node, just scan for one.
-    Scope = dyn_cast<ScopeMatcher>(N);
-    if (!Scope)
-      RebindableMatcherPtr = &(N->getNextPtr());
-  }
-  std::unique_ptr<Matcher> &MatcherPtr = *RebindableMatcherPtr;
+static void FactorNodes(std::unique_ptr<Matcher> &InputMatcherPtr);
+
+static void FactorScope(std::unique_ptr<Matcher> &MatcherPtr) {
+  ScopeMatcher *Scope = cast<ScopeMatcher>(MatcherPtr.get());
 
   // Okay, pull together the children of the scope node into a vector so we can
   // inspect it more easily.
@@ -353,7 +329,7 @@ static void FactorNodes(std::unique_ptr<Matcher> &InputMatcherPtr) {
       Shared->setNext(new ScopeMatcher(std::move(EqualMatchers)));
 
       // Recursively factor the newly created node.
-      FactorNodes(Shared->getNextPtr());
+      FactorScope(Shared->getNextPtr());
     }
 
     // Put the new Matcher where we started in OptionsToMatch.
@@ -470,7 +446,7 @@ static void FactorNodes(std::unique_ptr<Matcher> &InputMatcherPtr) {
     for (auto &M : Cases) {
       if (ScopeMatcher *SM = dyn_cast<ScopeMatcher>(M.second)) {
         std::unique_ptr<Matcher> Scope(SM);
-        FactorNodes(Scope);
+        FactorScope(Scope);
         M.second = Scope.release();
         assert(M.second && "null matcher");
       }
@@ -492,6 +468,31 @@ static void FactorNodes(std::unique_ptr<Matcher> &InputMatcherPtr) {
     Scope->resetChild(i, OptionsToMatch[i]);
 }
 
+/// FactorNodes - Turn matches like this:
+///   Scope
+///     OPC_CheckType i32
+///       ABC
+///     OPC_CheckType i32
+///       XYZ
+/// into:
+///   OPC_CheckType i32
+///     Scope
+///       ABC
+///       XYZ
+///
+static void FactorNodes(std::unique_ptr<Matcher> &InputMatcherPtr) {
+  // Look for a scope matcher. Iterates instead of recurses to reduce stack
+  // usage.
+  std::unique_ptr<Matcher> *MatcherPtr = &InputMatcherPtr;
+  do {
+    if (isa<ScopeMatcher>(*MatcherPtr))
+      return FactorScope(*MatcherPtr);
+
+    // If this is not a scope matcher, go to the next node.
+    MatcherPtr = &(MatcherPtr->get()->getNextPtr());
+  } while (MatcherPtr->get());
+}
+
 void llvm::OptimizeMatcher(std::unique_ptr<Matcher> &MatcherPtr,
                            const CodeGenDAGPatterns &CGP) {
   ContractNodes(MatcherPtr, CGP);

>From 909a08381960fd671cbe2360d4ef8387093796a3 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Sat, 1 Feb 2025 22:08:24 -0800
Subject: [PATCH 2/2] fixup! Cleanup comments a little.

---
 llvm/utils/TableGen/DAGISelMatcherOpt.cpp | 25 ++++++++++++-----------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/llvm/utils/TableGen/DAGISelMatcherOpt.cpp b/llvm/utils/TableGen/DAGISelMatcherOpt.cpp
index 5193b3c0741937..62d6038e580f7e 100644
--- a/llvm/utils/TableGen/DAGISelMatcherOpt.cpp
+++ b/llvm/utils/TableGen/DAGISelMatcherOpt.cpp
@@ -193,6 +193,18 @@ static Matcher *FindNodeWithKind(Matcher *M, Matcher::KindTy Kind) {
 
 static void FactorNodes(std::unique_ptr<Matcher> &InputMatcherPtr);
 
+/// Turn matches like this:
+///   Scope
+///     OPC_CheckType i32
+///       ABC
+///     OPC_CheckType i32
+///       XYZ
+/// into:
+///   OPC_CheckType i32
+///     Scope
+///       ABC
+///       XYZ
+///
 static void FactorScope(std::unique_ptr<Matcher> &MatcherPtr) {
   ScopeMatcher *Scope = cast<ScopeMatcher>(MatcherPtr.get());
 
@@ -468,18 +480,7 @@ static void FactorScope(std::unique_ptr<Matcher> &MatcherPtr) {
     Scope->resetChild(i, OptionsToMatch[i]);
 }
 
-/// FactorNodes - Turn matches like this:
-///   Scope
-///     OPC_CheckType i32
-///       ABC
-///     OPC_CheckType i32
-///       XYZ
-/// into:
-///   OPC_CheckType i32
-///     Scope
-///       ABC
-///       XYZ
-///
+/// Search a ScopeMatcher to factor with FactorScope.
 static void FactorNodes(std::unique_ptr<Matcher> &InputMatcherPtr) {
   // Look for a scope matcher. Iterates instead of recurses to reduce stack
   // usage.



More information about the llvm-commits mailing list