Skip to content

Conversation

@slackito
Copy link
Collaborator

When I first added the Region class I was unclear whether it was SandboxVectorizer-specific or something more general that could go in SandboxIR (as evidenced by the fact that I got the Region.h include guard wrong and didn't have to update it for this commit :)

Now I'm convinced that it belongs in SandboxVectorizer, so I'd like to move it back. This change does just that:

  • Region.h, Region.cpp and RegionTest.cpp are moved into the right directories for SandboxVectorizer.

  • The RegionPass and RegionPassManager classes are moved into Region.h. Their tests are moved into RegionTest.cpp as well.

  • All the other touched files are just mechanical changes to make everything work. Mainly updating include paths and qualifying some names in tests.

When I first added the Region class I was unclear whether it was
SandboxVectorizer-specific or something more general that could go in
SandboxIR (as evidenced by the fact that I got the Region.h include
guard wrong and didn't have to update it for this commit :)

Now I'm convinced that it belongs in SandboxVectorizer, so I'd like to
move it back. This change does just that:

- Region.h, Region.cpp and RegionTest.cpp are moved into the right
directories for SandboxVectorizer.

- The RegionPass and RegionPassManager classes are moved into Region.h.

- All the other touched files are just mechanical changes to make
  everything work. Mainly updating include paths and qualifying some
  names in tests.
@llvmbot
Copy link
Member

llvmbot commented Nov 21, 2024

@llvm/pr-subscribers-vectorizers

Author: Jorge Gorbe Moya (slackito)

Changes

When I first added the Region class I was unclear whether it was SandboxVectorizer-specific or something more general that could go in SandboxIR (as evidenced by the fact that I got the Region.h include guard wrong and didn't have to update it for this commit :)

Now I'm convinced that it belongs in SandboxVectorizer, so I'd like to move it back. This change does just that:

  • Region.h, Region.cpp and RegionTest.cpp are moved into the right directories for SandboxVectorizer.

  • The RegionPass and RegionPassManager classes are moved into Region.h. Their tests are moved into RegionTest.cpp as well.

  • All the other touched files are just mechanical changes to make everything work. Mainly updating include paths and qualifying some names in tests.


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

17 Files Affected:

  • (modified) llvm/include/llvm/SandboxIR/Pass.h (-10)
  • (modified) llvm/include/llvm/SandboxIR/PassManager.h (-9)
  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h (+1)
  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h (+1-3)
  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCount.h (+1-2)
  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h (+1)
  • (renamed) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Region.h (+22)
  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h (+1)
  • (modified) llvm/lib/SandboxIR/CMakeLists.txt (-1)
  • (modified) llvm/lib/SandboxIR/PassManager.cpp (-10)
  • (modified) llvm/lib/Transforms/Vectorize/CMakeLists.txt (+1)
  • (modified) llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp (+1-1)
  • (renamed) llvm/lib/Transforms/Vectorize/SandboxVectorizer/Region.cpp (+11-1)
  • (modified) llvm/unittests/SandboxIR/CMakeLists.txt (-1)
  • (modified) llvm/unittests/SandboxIR/PassTest.cpp (-122)
  • (modified) llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt (+2-1)
  • (renamed) llvm/unittests/Transforms/Vectorize/SandboxVectorizer/RegionTest.cpp (+128-1)
diff --git a/llvm/include/llvm/SandboxIR/Pass.h b/llvm/include/llvm/SandboxIR/Pass.h
index 4f4eae87cd3ff7..f7df8f671ecb28 100644
--- a/llvm/include/llvm/SandboxIR/Pass.h
+++ b/llvm/include/llvm/SandboxIR/Pass.h
@@ -20,7 +20,6 @@ class ScalarEvolution;
 namespace sandboxir {
 
 class Function;
-class Region;
 
 class Analyses {
   AAResults *AA = nullptr;
@@ -76,15 +75,6 @@ class FunctionPass : public Pass {
   virtual bool runOnFunction(Function &F, const Analyses &A) = 0;
 };
 
-/// A pass that runs on a sandbox::Region.
-class RegionPass : public Pass {
-public:
-  /// \p Name can't contain any spaces or start with '-'.
-  RegionPass(StringRef Name) : Pass(Name) {}
-  /// \Returns true if it modifies \p R.
-  virtual bool runOnRegion(Region &R, const Analyses &A) = 0;
-};
-
 } // namespace sandboxir
 } // namespace llvm
 
diff --git a/llvm/include/llvm/SandboxIR/PassManager.h b/llvm/include/llvm/SandboxIR/PassManager.h
index 77154cc7143454..3a4c6b881c98e2 100644
--- a/llvm/include/llvm/SandboxIR/PassManager.h
+++ b/llvm/include/llvm/SandboxIR/PassManager.h
@@ -211,15 +211,6 @@ class FunctionPassManager final
   bool runOnFunction(Function &F, const Analyses &A) final;
 };
 
-class RegionPassManager final : public PassManager<RegionPass, RegionPass> {
-public:
-  RegionPassManager(StringRef Name) : PassManager(Name) {}
-  RegionPassManager(StringRef Name, StringRef Pipeline,
-                    CreatePassFunc CreatePass)
-      : PassManager(Name, Pipeline, CreatePass) {}
-  bool runOnRegion(Region &R, const Analyses &A) final;
-};
-
 } // namespace llvm::sandboxir
 
 #endif // LLVM_SANDBOXIR_PASSMANAGER_H
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
index bd45634814b076..f68d68283fd8c9 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
@@ -19,6 +19,7 @@
 #include "llvm/SandboxIR/PassManager.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Region.h"
 
 namespace llvm::sandboxir {
 
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h
index 1025379770bac0..f381f5fc367c76 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h
@@ -1,12 +1,10 @@
 #ifndef LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_NULLPASS_H
 #define LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_NULLPASS_H
 
-#include "llvm/SandboxIR/Pass.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Region.h"
 
 namespace llvm::sandboxir {
 
-class Region;
-
 /// A Region pass that does nothing, for use as a placeholder in tests.
 class NullPass final : public RegionPass {
 public:
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCount.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCount.h
index cd11d4c1489268..afc6fa98517249 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCount.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCount.h
@@ -1,8 +1,7 @@
 #ifndef LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_PRINTINSTRUCTIONCOUNT_H
 #define LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_PRINTINSTRUCTIONCOUNT_H
 
-#include "llvm/SandboxIR/Pass.h"
-#include "llvm/SandboxIR/Region.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Region.h"
 
 namespace llvm::sandboxir {
 
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h
index 3d738ac8917eff..7c9add7a69abf0 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/SandboxIR/Pass.h"
 #include "llvm/SandboxIR/PassManager.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Region.h"
 
 namespace llvm::sandboxir {
 
diff --git a/llvm/include/llvm/SandboxIR/Region.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Region.h
similarity index 83%
rename from llvm/include/llvm/SandboxIR/Region.h
rename to llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Region.h
index 8133e01734ea78..d5d0496b259ed9 100644
--- a/llvm/include/llvm/SandboxIR/Region.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Region.h
@@ -14,6 +14,8 @@
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/SandboxIR/Instruction.h"
+#include "llvm/SandboxIR/Pass.h"
+#include "llvm/SandboxIR/PassManager.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace llvm::sandboxir {
@@ -107,6 +109,26 @@ class Region {
 #endif
 };
 
+/// A pass that runs on a Region.
+class RegionPass : public Pass {
+public:
+  /// \p Name can't contain any spaces or start with '-'.
+  RegionPass(StringRef Name) : Pass(Name) {}
+  /// \Returns true if it modifies \p R.
+  virtual bool runOnRegion(Region &R, const Analyses &A) = 0;
+};
+
+/// A PassManager for passes that operate on Regions.
+class RegionPassManager final : public PassManager<RegionPass, RegionPass> {
+public:
+  RegionPassManager(StringRef Name) : PassManager(Name) {}
+  RegionPassManager(StringRef Name, StringRef Pipeline,
+                    CreatePassFunc CreatePass)
+      : PassManager(Name, Pipeline, CreatePass) {}
+  bool runOnRegion(Region &R, const Analyses &A) final;
+};
+
+
 } // namespace llvm::sandboxir
 
 #endif // LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_REGION_H
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h
index e3d6ecae836fce..0e3a89eea9948c 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h
@@ -14,6 +14,7 @@
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/SandboxIR/Pass.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Region.h"
 
 #include <memory>
 
diff --git a/llvm/lib/SandboxIR/CMakeLists.txt b/llvm/lib/SandboxIR/CMakeLists.txt
index 3ec53b04b046f9..6a1b53e2749bda 100644
--- a/llvm/lib/SandboxIR/CMakeLists.txt
+++ b/llvm/lib/SandboxIR/CMakeLists.txt
@@ -8,7 +8,6 @@ add_llvm_component_library(LLVMSandboxIR
   Module.cpp
   Pass.cpp
   PassManager.cpp
-  Region.cpp
   Tracker.cpp
   Type.cpp
   User.cpp
diff --git a/llvm/lib/SandboxIR/PassManager.cpp b/llvm/lib/SandboxIR/PassManager.cpp
index aaa49e0f6912b6..a7194bd68b6135 100644
--- a/llvm/lib/SandboxIR/PassManager.cpp
+++ b/llvm/lib/SandboxIR/PassManager.cpp
@@ -20,14 +20,4 @@ bool FunctionPassManager::runOnFunction(Function &F, const Analyses &A) {
   return Change;
 }
 
-bool RegionPassManager::runOnRegion(Region &R, const Analyses &A) {
-  bool Change = false;
-  for (auto &Pass : Passes) {
-    Change |= Pass->runOnRegion(R, A);
-    // TODO: run the verifier.
-  }
-  // TODO: Check ChangeAll against hashes before/after.
-  return Change;
-}
-
 } // namespace llvm::sandboxir
diff --git a/llvm/lib/Transforms/Vectorize/CMakeLists.txt b/llvm/lib/Transforms/Vectorize/CMakeLists.txt
index d769d5100afd23..70bb54d0376d5c 100644
--- a/llvm/lib/Transforms/Vectorize/CMakeLists.txt
+++ b/llvm/lib/Transforms/Vectorize/CMakeLists.txt
@@ -8,6 +8,7 @@ add_llvm_component_library(LLVMVectorize
   SandboxVectorizer/Legality.cpp
   SandboxVectorizer/Passes/BottomUpVec.cpp
   SandboxVectorizer/Passes/RegionsFromMetadata.cpp
+  SandboxVectorizer/Region.cpp
   SandboxVectorizer/SandboxVectorizer.cpp
   SandboxVectorizer/SandboxVectorizerPassBuilder.cpp
   SandboxVectorizer/Scheduler.cpp
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp
index 8e3f5b77429c5a..6d69bcd6e9c871 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp
@@ -8,7 +8,7 @@
 
 #include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h"
 
-#include "llvm/SandboxIR/Region.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Region.h"
 #include "llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h"
 
 namespace llvm::sandboxir {
diff --git a/llvm/lib/SandboxIR/Region.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Region.cpp
similarity index 87%
rename from llvm/lib/SandboxIR/Region.cpp
rename to llvm/lib/Transforms/Vectorize/SandboxVectorizer/Region.cpp
index 1455012440f90c..c1556a461a0fc7 100644
--- a/llvm/lib/SandboxIR/Region.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Region.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/SandboxIR/Region.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Region.h"
 #include "llvm/SandboxIR/Function.h"
 
 namespace llvm::sandboxir {
@@ -81,4 +81,14 @@ SmallVector<std::unique_ptr<Region>> Region::createRegionsFromMD(Function &F) {
   return Regions;
 }
 
+bool RegionPassManager::runOnRegion(Region &R, const Analyses &A) {
+  bool Change = false;
+  for (auto &Pass : Passes) {
+    Change |= Pass->runOnRegion(R, A);
+    // TODO: run the verifier.
+  }
+  // TODO: Check ChangeAll against hashes before/after.
+  return Change;
+}
+
 } // namespace llvm::sandboxir
diff --git a/llvm/unittests/SandboxIR/CMakeLists.txt b/llvm/unittests/SandboxIR/CMakeLists.txt
index b20ef829ed0c95..b4b7a7c580770f 100644
--- a/llvm/unittests/SandboxIR/CMakeLists.txt
+++ b/llvm/unittests/SandboxIR/CMakeLists.txt
@@ -8,7 +8,6 @@ set(LLVM_LINK_COMPONENTS
 add_llvm_unittest(SandboxIRTests
   IntrinsicInstTest.cpp
   PassTest.cpp
-  RegionTest.cpp
   OperatorTest.cpp
   SandboxIRTest.cpp
   TrackerTest.cpp
diff --git a/llvm/unittests/SandboxIR/PassTest.cpp b/llvm/unittests/SandboxIR/PassTest.cpp
index 751aedefd8fe2d..e18d34f867fcef 100644
--- a/llvm/unittests/SandboxIR/PassTest.cpp
+++ b/llvm/unittests/SandboxIR/PassTest.cpp
@@ -13,7 +13,6 @@
 #include "llvm/SandboxIR/Context.h"
 #include "llvm/SandboxIR/Function.h"
 #include "llvm/SandboxIR/PassManager.h"
-#include "llvm/SandboxIR/Region.h"
 #include "llvm/Support/SourceMgr.h"
 #include "gtest/gtest.h"
 
@@ -87,68 +86,6 @@ define void @foo() {
 #endif
 }
 
-TEST_F(PassTest, RegionPass) {
-  auto *F = parseFunction(R"IR(
-define i8 @foo(i8 %v0, i8 %v1) {
-  %t0 = add i8 %v0, 1
-  %t1 = add i8 %t0, %v1, !sandboxvec !0
-  %t2 = add i8 %t1, %v1, !sandboxvec !0
-  ret i8 %t1
-}
-
-!0 = distinct !{!"sandboxregion"}
-)IR",
-                          "foo");
-
-  class TestPass final : public RegionPass {
-    unsigned &InstCount;
-
-  public:
-    TestPass(unsigned &InstCount)
-        : RegionPass("test-pass"), InstCount(InstCount) {}
-    bool runOnRegion(Region &R, const Analyses &A) final {
-      for ([[maybe_unused]] auto &Inst : R) {
-        ++InstCount;
-      }
-      return false;
-    }
-  };
-  unsigned InstCount = 0;
-  TestPass TPass(InstCount);
-  // Check getName(),
-  EXPECT_EQ(TPass.getName(), "test-pass");
-  // Check runOnRegion();
-  llvm::SmallVector<std::unique_ptr<Region>> Regions =
-      Region::createRegionsFromMD(*F);
-  ASSERT_EQ(Regions.size(), 1u);
-  TPass.runOnRegion(*Regions[0], Analyses::emptyForTesting());
-  EXPECT_EQ(InstCount, 2u);
-#ifndef NDEBUG
-  {
-    // Check print().
-    std::string Buff;
-    llvm::raw_string_ostream SS(Buff);
-    TPass.print(SS);
-    EXPECT_EQ(Buff, "test-pass");
-  }
-  {
-    // Check operator<<().
-    std::string Buff;
-    llvm::raw_string_ostream SS(Buff);
-    SS << TPass;
-    EXPECT_EQ(Buff, "test-pass");
-  }
-  // Check pass name assertions.
-  class TestNamePass final : public RegionPass {
-  public:
-    TestNamePass(llvm::StringRef Name) : RegionPass(Name) {}
-    bool runOnRegion(Region &F, const Analyses &A) { return false; }
-  };
-  EXPECT_DEATH(TestNamePass("white space"), ".*whitespace.*");
-  EXPECT_DEATH(TestNamePass("-dash"), ".*start with.*");
-#endif
-}
-
 TEST_F(PassTest, FunctionPassManager) {
   auto *F = parseFunction(R"IR(
 define void @foo() {
@@ -197,65 +134,6 @@ define void @foo() {
 #endif // NDEBUG
 }
 
-TEST_F(PassTest, RegionPassManager) {
-  auto *F = parseFunction(R"IR(
-define i8 @foo(i8 %v0, i8 %v1) {
-  %t0 = add i8 %v0, 1
-  %t1 = add i8 %t0, %v1, !sandboxvec !0
-  %t2 = add i8 %t1, %v1, !sandboxvec !0
-  ret i8 %t1
-}
-
-!0 = distinct !{!"sandboxregion"}
-)IR",
-                          "foo");
-
-  class TestPass1 final : public RegionPass {
-    unsigned &InstCount;
-
-  public:
-    TestPass1(unsigned &InstCount)
-        : RegionPass("test-pass1"), InstCount(InstCount) {}
-    bool runOnRegion(Region &R, const Analyses &A) final {
-      for ([[maybe_unused]] auto &Inst : R)
-        ++InstCount;
-      return false;
-    }
-  };
-  class TestPass2 final : public RegionPass {
-    unsigned &InstCount;
-
-  public:
-    TestPass2(unsigned &InstCount)
-        : RegionPass("test-pass2"), InstCount(InstCount) {}
-    bool runOnRegion(Region &R, const Analyses &A) final {
-      for ([[maybe_unused]] auto &Inst : R)
-        ++InstCount;
-      return false;
-    }
-  };
-  unsigned InstCount1 = 0;
-  unsigned InstCount2 = 0;
-
-  RegionPassManager RPM("test-rpm");
-  RPM.addPass(std::make_unique<TestPass1>(InstCount1));
-  RPM.addPass(std::make_unique<TestPass2>(InstCount2));
-  // Check runOnRegion().
-  llvm::SmallVector<std::unique_ptr<Region>> Regions =
-      Region::createRegionsFromMD(*F);
-  ASSERT_EQ(Regions.size(), 1u);
-  RPM.runOnRegion(*Regions[0], Analyses::emptyForTesting());
-  EXPECT_EQ(InstCount1, 2u);
-  EXPECT_EQ(InstCount2, 2u);
-#ifndef NDEBUG
-  // Check dump().
-  std::string Buff;
-  llvm::raw_string_ostream SS(Buff);
-  RPM.print(SS);
-  EXPECT_EQ(Buff, "test-rpm(test-pass1,test-pass2)");
-#endif // NDEBUG
-}
-
 TEST_F(PassTest, SetPassPipeline) {
   auto *F = parseFunction(R"IR(
 define void @f() {
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt
index df689767b77245..70a2d7f1c872b9 100644
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt
@@ -11,7 +11,8 @@ add_llvm_unittest(SandboxVectorizerTests
   DependencyGraphTest.cpp
   IntervalTest.cpp
   LegalityTest.cpp
+  RegionTest.cpp
   SchedulerTest.cpp
-  SeedCollectorTest.cpp	
+  SeedCollectorTest.cpp
   VecUtilsTest.cpp
 )
diff --git a/llvm/unittests/SandboxIR/RegionTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/RegionTest.cpp
similarity index 64%
rename from llvm/unittests/SandboxIR/RegionTest.cpp
rename to llvm/unittests/Transforms/Vectorize/SandboxVectorizer/RegionTest.cpp
index 47368f93a32c0c..e2aeadd140079f 100644
--- a/llvm/unittests/SandboxIR/RegionTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/RegionTest.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/SandboxIR/Region.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Region.h"
 #include "llvm/AsmParser/Parser.h"
 #include "llvm/SandboxIR/Context.h"
 #include "llvm/SandboxIR/Function.h"
@@ -237,3 +237,130 @@ define i8 @foo(i8 %v0, i8 %v1) {
   EXPECT_EQ(Rgn, *Regions[0].get());
 #endif
 }
+
+TEST_F(RegionTest, RegionPass) {
+  parseIR(C, R"IR(
+define i8 @foo(i8 %v0, i8 %v1) {
+  %t0 = add i8 %v0, 1
+  %t1 = add i8 %t0, %v1, !sandboxvec !0
+  %t2 = add i8 %t1, %v1, !sandboxvec !0
+  ret i8 %t1
+}
+
+!0 = distinct !{!"sandboxregion"}
+)IR");
+  llvm::Function *LLVMF = &*M->getFunction("foo");
+  sandboxir::Context Ctx(C);
+  auto *F = Ctx.createFunction(LLVMF);
+
+  class TestPass final : public sandboxir::RegionPass {
+    unsigned &InstCount;
+
+  public:
+    TestPass(unsigned &InstCount)
+        : RegionPass("test-pass"), InstCount(InstCount) {}
+    bool runOnRegion(sandboxir::Region &R, const sandboxir::Analyses &A) final {
+      for ([[maybe_unused]] auto &Inst : R) {
+        ++InstCount;
+      }
+      return false;
+    }
+  };
+  unsigned InstCount = 0;
+  TestPass TPass(InstCount);
+  // Check getName(),
+  EXPECT_EQ(TPass.getName(), "test-pass");
+  // Check runOnRegion();
+  llvm::SmallVector<std::unique_ptr<sandboxir::Region>> Regions =
+      sandboxir::Region::createRegionsFromMD(*F);
+  ASSERT_EQ(Regions.size(), 1u);
+  TPass.runOnRegion(*Regions[0], sandboxir::Analyses::emptyForTesting());
+  EXPECT_EQ(InstCount, 2u);
+#ifndef NDEBUG
+  {
+    // Check print().
+    std::string Buff;
+    llvm::raw_string_ostream SS(Buff);
+    TPass.print(SS);
+    EXPECT_EQ(Buff, "test-pass");
+  }
+  {
+    // Check operator<<().
+    std::string Buff;
+    llvm::raw_string_ostream SS(Buff);
+    SS << TPass;
+    EXPECT_EQ(Buff, "test-pass");
+  }
+  // Check pass name assertions.
+  class TestNamePass final : public sandboxir::RegionPass {
+  public:
+    TestNamePass(llvm::StringRef Name) : RegionPass(Name) {}
+    bool runOnRegion(sandboxir::Region &F, const sandboxir::Analyses &A) {
+      return false;
+    }
+  };
+  EXPECT_DEATH(TestNamePass("white space"), ".*whitespace.*");
+  EXPECT_DEATH(TestNamePass("-dash"), ".*start with.*");
+#endif
+}
+
+TEST_F(RegionTest, RegionPassManager) {
+  parseIR(C, R"IR(
+define i8 @foo(i8 %v0, i8 %v1) {
+  %t0 = add i8 %v0, 1
+  %t1 = add i8 %t0, %v1, !sandboxvec !0
+  %t2 = add i8 %t1, %v1, !sandboxvec !0
+  ret i8 %t1
+}
+
+!0 = distinct !{!"sandboxregion"}
+)IR");
+  llvm::Function *LLVMF = &*M->getFunction("foo");
+  sandboxir::Context Ctx(C);
+  auto *F = Ctx.createFunction(LLVMF);
+
+  class TestPass1 final : public sandboxir::RegionPass {
+    unsigned &InstCount;
+
+  public:
+    TestPass1(unsigned &InstCount)
+        : RegionPass("test-pass1"), InstCount(InstCount) {}
+    bool runOnRegion(sandboxir::Region &R, const sandboxir::Analyses &A) final {
+      for ([[maybe_unused]] auto &Inst : R)
+        ++InstCount;
+      return false;
+    }
+  };
+  class TestPass2 final : public sandboxir::RegionPass {
+    unsigned &InstCount;
+
+  public:
+    TestPass2(unsigned &InstCount)
+        : RegionPass("test-pass2"), InstCount(InstCount) {}
+    bool runOnRegion(sandboxir::Region &R, const sandboxir::Analyses &A) final {
+      for ([[maybe_unused]] auto &Inst : R)
+        ++InstCount;
+      return false;
+    }
+  };
+  unsigned InstCount1 = 0;
+  unsigned InstCount2 = 0;
+
+  sandboxir::RegionPassManager RPM("test-rpm");
+  RPM.addPass(std::make_unique<TestPass1>(InstCount1));
+  RPM.addPass(std::make_unique<TestPass2>(InstCount2));
+  // Check runOnRegion().
+  llvm::SmallVector<std::unique_ptr<sandboxir::Region>> Regions =
+      sandboxir::Region::createRegionsFromMD(*F);
+  ASSERT_EQ(Regions.size(), 1u);
+  RPM.runOnRegion(*Regions[0], sandboxir::Analyses::emptyForTesting());
+  EXPECT_EQ(InstCount1, 2u);
+  EXPECT_EQ(InstCount2, 2u);
+#ifndef NDEBUG
+  // Check dump().
+  std::string Buff;
+  llvm::raw_string_ostream SS(Buff);
+  RPM.print(SS);
+  EXPECT_EQ(Buff, "test-rpm(test-pass1,test-pass2)");
+#endif // NDEBUG
+}

@llvmbot
Copy link
Member

llvmbot commented Nov 21, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Jorge Gorbe Moya (slackito)

Changes

When I first added the Region class I was unclear whether it was SandboxVectorizer-specific or something more general that could go in SandboxIR (as evidenced by the fact that I got the Region.h include guard wrong and didn't have to update it for this commit :)

Now I'm convinced that it belongs in SandboxVectorizer, so I'd like to move it back. This change does just that:

  • Region.h, Region.cpp and RegionTest.cpp are moved into the right directories for SandboxVectorizer.

  • The RegionPass and RegionPassManager classes are moved into Region.h. Their tests are moved into RegionTest.cpp as well.

  • All the other touched files are just mechanical changes to make everything work. Mainly updating include paths and qualifying some names in tests.


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

17 Files Affected:

  • (modified) llvm/include/llvm/SandboxIR/Pass.h (-10)
  • (modified) llvm/include/llvm/SandboxIR/PassManager.h (-9)
  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h (+1)
  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h (+1-3)
  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCount.h (+1-2)
  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h (+1)
  • (renamed) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Region.h (+22)
  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h (+1)
  • (modified) llvm/lib/SandboxIR/CMakeLists.txt (-1)
  • (modified) llvm/lib/SandboxIR/PassManager.cpp (-10)
  • (modified) llvm/lib/Transforms/Vectorize/CMakeLists.txt (+1)
  • (modified) llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp (+1-1)
  • (renamed) llvm/lib/Transforms/Vectorize/SandboxVectorizer/Region.cpp (+11-1)
  • (modified) llvm/unittests/SandboxIR/CMakeLists.txt (-1)
  • (modified) llvm/unittests/SandboxIR/PassTest.cpp (-122)
  • (modified) llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt (+2-1)
  • (renamed) llvm/unittests/Transforms/Vectorize/SandboxVectorizer/RegionTest.cpp (+128-1)
diff --git a/llvm/include/llvm/SandboxIR/Pass.h b/llvm/include/llvm/SandboxIR/Pass.h
index 4f4eae87cd3ff7..f7df8f671ecb28 100644
--- a/llvm/include/llvm/SandboxIR/Pass.h
+++ b/llvm/include/llvm/SandboxIR/Pass.h
@@ -20,7 +20,6 @@ class ScalarEvolution;
 namespace sandboxir {
 
 class Function;
-class Region;
 
 class Analyses {
   AAResults *AA = nullptr;
@@ -76,15 +75,6 @@ class FunctionPass : public Pass {
   virtual bool runOnFunction(Function &F, const Analyses &A) = 0;
 };
 
-/// A pass that runs on a sandbox::Region.
-class RegionPass : public Pass {
-public:
-  /// \p Name can't contain any spaces or start with '-'.
-  RegionPass(StringRef Name) : Pass(Name) {}
-  /// \Returns true if it modifies \p R.
-  virtual bool runOnRegion(Region &R, const Analyses &A) = 0;
-};
-
 } // namespace sandboxir
 } // namespace llvm
 
diff --git a/llvm/include/llvm/SandboxIR/PassManager.h b/llvm/include/llvm/SandboxIR/PassManager.h
index 77154cc7143454..3a4c6b881c98e2 100644
--- a/llvm/include/llvm/SandboxIR/PassManager.h
+++ b/llvm/include/llvm/SandboxIR/PassManager.h
@@ -211,15 +211,6 @@ class FunctionPassManager final
   bool runOnFunction(Function &F, const Analyses &A) final;
 };
 
-class RegionPassManager final : public PassManager<RegionPass, RegionPass> {
-public:
-  RegionPassManager(StringRef Name) : PassManager(Name) {}
-  RegionPassManager(StringRef Name, StringRef Pipeline,
-                    CreatePassFunc CreatePass)
-      : PassManager(Name, Pipeline, CreatePass) {}
-  bool runOnRegion(Region &R, const Analyses &A) final;
-};
-
 } // namespace llvm::sandboxir
 
 #endif // LLVM_SANDBOXIR_PASSMANAGER_H
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
index bd45634814b076..f68d68283fd8c9 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
@@ -19,6 +19,7 @@
 #include "llvm/SandboxIR/PassManager.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Region.h"
 
 namespace llvm::sandboxir {
 
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h
index 1025379770bac0..f381f5fc367c76 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h
@@ -1,12 +1,10 @@
 #ifndef LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_NULLPASS_H
 #define LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_NULLPASS_H
 
-#include "llvm/SandboxIR/Pass.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Region.h"
 
 namespace llvm::sandboxir {
 
-class Region;
-
 /// A Region pass that does nothing, for use as a placeholder in tests.
 class NullPass final : public RegionPass {
 public:
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCount.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCount.h
index cd11d4c1489268..afc6fa98517249 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCount.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCount.h
@@ -1,8 +1,7 @@
 #ifndef LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_PRINTINSTRUCTIONCOUNT_H
 #define LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_PRINTINSTRUCTIONCOUNT_H
 
-#include "llvm/SandboxIR/Pass.h"
-#include "llvm/SandboxIR/Region.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Region.h"
 
 namespace llvm::sandboxir {
 
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h
index 3d738ac8917eff..7c9add7a69abf0 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h
@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/SandboxIR/Pass.h"
 #include "llvm/SandboxIR/PassManager.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Region.h"
 
 namespace llvm::sandboxir {
 
diff --git a/llvm/include/llvm/SandboxIR/Region.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Region.h
similarity index 83%
rename from llvm/include/llvm/SandboxIR/Region.h
rename to llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Region.h
index 8133e01734ea78..d5d0496b259ed9 100644
--- a/llvm/include/llvm/SandboxIR/Region.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Region.h
@@ -14,6 +14,8 @@
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/SandboxIR/Instruction.h"
+#include "llvm/SandboxIR/Pass.h"
+#include "llvm/SandboxIR/PassManager.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace llvm::sandboxir {
@@ -107,6 +109,26 @@ class Region {
 #endif
 };
 
+/// A pass that runs on a Region.
+class RegionPass : public Pass {
+public:
+  /// \p Name can't contain any spaces or start with '-'.
+  RegionPass(StringRef Name) : Pass(Name) {}
+  /// \Returns true if it modifies \p R.
+  virtual bool runOnRegion(Region &R, const Analyses &A) = 0;
+};
+
+/// A PassManager for passes that operate on Regions.
+class RegionPassManager final : public PassManager<RegionPass, RegionPass> {
+public:
+  RegionPassManager(StringRef Name) : PassManager(Name) {}
+  RegionPassManager(StringRef Name, StringRef Pipeline,
+                    CreatePassFunc CreatePass)
+      : PassManager(Name, Pipeline, CreatePass) {}
+  bool runOnRegion(Region &R, const Analyses &A) final;
+};
+
+
 } // namespace llvm::sandboxir
 
 #endif // LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_REGION_H
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h
index e3d6ecae836fce..0e3a89eea9948c 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h
@@ -14,6 +14,7 @@
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/SandboxIR/Pass.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Region.h"
 
 #include <memory>
 
diff --git a/llvm/lib/SandboxIR/CMakeLists.txt b/llvm/lib/SandboxIR/CMakeLists.txt
index 3ec53b04b046f9..6a1b53e2749bda 100644
--- a/llvm/lib/SandboxIR/CMakeLists.txt
+++ b/llvm/lib/SandboxIR/CMakeLists.txt
@@ -8,7 +8,6 @@ add_llvm_component_library(LLVMSandboxIR
   Module.cpp
   Pass.cpp
   PassManager.cpp
-  Region.cpp
   Tracker.cpp
   Type.cpp
   User.cpp
diff --git a/llvm/lib/SandboxIR/PassManager.cpp b/llvm/lib/SandboxIR/PassManager.cpp
index aaa49e0f6912b6..a7194bd68b6135 100644
--- a/llvm/lib/SandboxIR/PassManager.cpp
+++ b/llvm/lib/SandboxIR/PassManager.cpp
@@ -20,14 +20,4 @@ bool FunctionPassManager::runOnFunction(Function &F, const Analyses &A) {
   return Change;
 }
 
-bool RegionPassManager::runOnRegion(Region &R, const Analyses &A) {
-  bool Change = false;
-  for (auto &Pass : Passes) {
-    Change |= Pass->runOnRegion(R, A);
-    // TODO: run the verifier.
-  }
-  // TODO: Check ChangeAll against hashes before/after.
-  return Change;
-}
-
 } // namespace llvm::sandboxir
diff --git a/llvm/lib/Transforms/Vectorize/CMakeLists.txt b/llvm/lib/Transforms/Vectorize/CMakeLists.txt
index d769d5100afd23..70bb54d0376d5c 100644
--- a/llvm/lib/Transforms/Vectorize/CMakeLists.txt
+++ b/llvm/lib/Transforms/Vectorize/CMakeLists.txt
@@ -8,6 +8,7 @@ add_llvm_component_library(LLVMVectorize
   SandboxVectorizer/Legality.cpp
   SandboxVectorizer/Passes/BottomUpVec.cpp
   SandboxVectorizer/Passes/RegionsFromMetadata.cpp
+  SandboxVectorizer/Region.cpp
   SandboxVectorizer/SandboxVectorizer.cpp
   SandboxVectorizer/SandboxVectorizerPassBuilder.cpp
   SandboxVectorizer/Scheduler.cpp
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp
index 8e3f5b77429c5a..6d69bcd6e9c871 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.cpp
@@ -8,7 +8,7 @@
 
 #include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/RegionsFromMetadata.h"
 
-#include "llvm/SandboxIR/Region.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Region.h"
 #include "llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizerPassBuilder.h"
 
 namespace llvm::sandboxir {
diff --git a/llvm/lib/SandboxIR/Region.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Region.cpp
similarity index 87%
rename from llvm/lib/SandboxIR/Region.cpp
rename to llvm/lib/Transforms/Vectorize/SandboxVectorizer/Region.cpp
index 1455012440f90c..c1556a461a0fc7 100644
--- a/llvm/lib/SandboxIR/Region.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Region.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/SandboxIR/Region.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Region.h"
 #include "llvm/SandboxIR/Function.h"
 
 namespace llvm::sandboxir {
@@ -81,4 +81,14 @@ SmallVector<std::unique_ptr<Region>> Region::createRegionsFromMD(Function &F) {
   return Regions;
 }
 
+bool RegionPassManager::runOnRegion(Region &R, const Analyses &A) {
+  bool Change = false;
+  for (auto &Pass : Passes) {
+    Change |= Pass->runOnRegion(R, A);
+    // TODO: run the verifier.
+  }
+  // TODO: Check ChangeAll against hashes before/after.
+  return Change;
+}
+
 } // namespace llvm::sandboxir
diff --git a/llvm/unittests/SandboxIR/CMakeLists.txt b/llvm/unittests/SandboxIR/CMakeLists.txt
index b20ef829ed0c95..b4b7a7c580770f 100644
--- a/llvm/unittests/SandboxIR/CMakeLists.txt
+++ b/llvm/unittests/SandboxIR/CMakeLists.txt
@@ -8,7 +8,6 @@ set(LLVM_LINK_COMPONENTS
 add_llvm_unittest(SandboxIRTests
   IntrinsicInstTest.cpp
   PassTest.cpp
-  RegionTest.cpp
   OperatorTest.cpp
   SandboxIRTest.cpp
   TrackerTest.cpp
diff --git a/llvm/unittests/SandboxIR/PassTest.cpp b/llvm/unittests/SandboxIR/PassTest.cpp
index 751aedefd8fe2d..e18d34f867fcef 100644
--- a/llvm/unittests/SandboxIR/PassTest.cpp
+++ b/llvm/unittests/SandboxIR/PassTest.cpp
@@ -13,7 +13,6 @@
 #include "llvm/SandboxIR/Context.h"
 #include "llvm/SandboxIR/Function.h"
 #include "llvm/SandboxIR/PassManager.h"
-#include "llvm/SandboxIR/Region.h"
 #include "llvm/Support/SourceMgr.h"
 #include "gtest/gtest.h"
 
@@ -87,68 +86,6 @@ define void @foo() {
 #endif
 }
 
-TEST_F(PassTest, RegionPass) {
-  auto *F = parseFunction(R"IR(
-define i8 @foo(i8 %v0, i8 %v1) {
-  %t0 = add i8 %v0, 1
-  %t1 = add i8 %t0, %v1, !sandboxvec !0
-  %t2 = add i8 %t1, %v1, !sandboxvec !0
-  ret i8 %t1
-}
-
-!0 = distinct !{!"sandboxregion"}
-)IR",
-                          "foo");
-
-  class TestPass final : public RegionPass {
-    unsigned &InstCount;
-
-  public:
-    TestPass(unsigned &InstCount)
-        : RegionPass("test-pass"), InstCount(InstCount) {}
-    bool runOnRegion(Region &R, const Analyses &A) final {
-      for ([[maybe_unused]] auto &Inst : R) {
-        ++InstCount;
-      }
-      return false;
-    }
-  };
-  unsigned InstCount = 0;
-  TestPass TPass(InstCount);
-  // Check getName(),
-  EXPECT_EQ(TPass.getName(), "test-pass");
-  // Check runOnRegion();
-  llvm::SmallVector<std::unique_ptr<Region>> Regions =
-      Region::createRegionsFromMD(*F);
-  ASSERT_EQ(Regions.size(), 1u);
-  TPass.runOnRegion(*Regions[0], Analyses::emptyForTesting());
-  EXPECT_EQ(InstCount, 2u);
-#ifndef NDEBUG
-  {
-    // Check print().
-    std::string Buff;
-    llvm::raw_string_ostream SS(Buff);
-    TPass.print(SS);
-    EXPECT_EQ(Buff, "test-pass");
-  }
-  {
-    // Check operator<<().
-    std::string Buff;
-    llvm::raw_string_ostream SS(Buff);
-    SS << TPass;
-    EXPECT_EQ(Buff, "test-pass");
-  }
-  // Check pass name assertions.
-  class TestNamePass final : public RegionPass {
-  public:
-    TestNamePass(llvm::StringRef Name) : RegionPass(Name) {}
-    bool runOnRegion(Region &F, const Analyses &A) { return false; }
-  };
-  EXPECT_DEATH(TestNamePass("white space"), ".*whitespace.*");
-  EXPECT_DEATH(TestNamePass("-dash"), ".*start with.*");
-#endif
-}
-
 TEST_F(PassTest, FunctionPassManager) {
   auto *F = parseFunction(R"IR(
 define void @foo() {
@@ -197,65 +134,6 @@ define void @foo() {
 #endif // NDEBUG
 }
 
-TEST_F(PassTest, RegionPassManager) {
-  auto *F = parseFunction(R"IR(
-define i8 @foo(i8 %v0, i8 %v1) {
-  %t0 = add i8 %v0, 1
-  %t1 = add i8 %t0, %v1, !sandboxvec !0
-  %t2 = add i8 %t1, %v1, !sandboxvec !0
-  ret i8 %t1
-}
-
-!0 = distinct !{!"sandboxregion"}
-)IR",
-                          "foo");
-
-  class TestPass1 final : public RegionPass {
-    unsigned &InstCount;
-
-  public:
-    TestPass1(unsigned &InstCount)
-        : RegionPass("test-pass1"), InstCount(InstCount) {}
-    bool runOnRegion(Region &R, const Analyses &A) final {
-      for ([[maybe_unused]] auto &Inst : R)
-        ++InstCount;
-      return false;
-    }
-  };
-  class TestPass2 final : public RegionPass {
-    unsigned &InstCount;
-
-  public:
-    TestPass2(unsigned &InstCount)
-        : RegionPass("test-pass2"), InstCount(InstCount) {}
-    bool runOnRegion(Region &R, const Analyses &A) final {
-      for ([[maybe_unused]] auto &Inst : R)
-        ++InstCount;
-      return false;
-    }
-  };
-  unsigned InstCount1 = 0;
-  unsigned InstCount2 = 0;
-
-  RegionPassManager RPM("test-rpm");
-  RPM.addPass(std::make_unique<TestPass1>(InstCount1));
-  RPM.addPass(std::make_unique<TestPass2>(InstCount2));
-  // Check runOnRegion().
-  llvm::SmallVector<std::unique_ptr<Region>> Regions =
-      Region::createRegionsFromMD(*F);
-  ASSERT_EQ(Regions.size(), 1u);
-  RPM.runOnRegion(*Regions[0], Analyses::emptyForTesting());
-  EXPECT_EQ(InstCount1, 2u);
-  EXPECT_EQ(InstCount2, 2u);
-#ifndef NDEBUG
-  // Check dump().
-  std::string Buff;
-  llvm::raw_string_ostream SS(Buff);
-  RPM.print(SS);
-  EXPECT_EQ(Buff, "test-rpm(test-pass1,test-pass2)");
-#endif // NDEBUG
-}
-
 TEST_F(PassTest, SetPassPipeline) {
   auto *F = parseFunction(R"IR(
 define void @f() {
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt
index df689767b77245..70a2d7f1c872b9 100644
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/CMakeLists.txt
@@ -11,7 +11,8 @@ add_llvm_unittest(SandboxVectorizerTests
   DependencyGraphTest.cpp
   IntervalTest.cpp
   LegalityTest.cpp
+  RegionTest.cpp
   SchedulerTest.cpp
-  SeedCollectorTest.cpp	
+  SeedCollectorTest.cpp
   VecUtilsTest.cpp
 )
diff --git a/llvm/unittests/SandboxIR/RegionTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/RegionTest.cpp
similarity index 64%
rename from llvm/unittests/SandboxIR/RegionTest.cpp
rename to llvm/unittests/Transforms/Vectorize/SandboxVectorizer/RegionTest.cpp
index 47368f93a32c0c..e2aeadd140079f 100644
--- a/llvm/unittests/SandboxIR/RegionTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/RegionTest.cpp
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "llvm/SandboxIR/Region.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Region.h"
 #include "llvm/AsmParser/Parser.h"
 #include "llvm/SandboxIR/Context.h"
 #include "llvm/SandboxIR/Function.h"
@@ -237,3 +237,130 @@ define i8 @foo(i8 %v0, i8 %v1) {
   EXPECT_EQ(Rgn, *Regions[0].get());
 #endif
 }
+
+TEST_F(RegionTest, RegionPass) {
+  parseIR(C, R"IR(
+define i8 @foo(i8 %v0, i8 %v1) {
+  %t0 = add i8 %v0, 1
+  %t1 = add i8 %t0, %v1, !sandboxvec !0
+  %t2 = add i8 %t1, %v1, !sandboxvec !0
+  ret i8 %t1
+}
+
+!0 = distinct !{!"sandboxregion"}
+)IR");
+  llvm::Function *LLVMF = &*M->getFunction("foo");
+  sandboxir::Context Ctx(C);
+  auto *F = Ctx.createFunction(LLVMF);
+
+  class TestPass final : public sandboxir::RegionPass {
+    unsigned &InstCount;
+
+  public:
+    TestPass(unsigned &InstCount)
+        : RegionPass("test-pass"), InstCount(InstCount) {}
+    bool runOnRegion(sandboxir::Region &R, const sandboxir::Analyses &A) final {
+      for ([[maybe_unused]] auto &Inst : R) {
+        ++InstCount;
+      }
+      return false;
+    }
+  };
+  unsigned InstCount = 0;
+  TestPass TPass(InstCount);
+  // Check getName(),
+  EXPECT_EQ(TPass.getName(), "test-pass");
+  // Check runOnRegion();
+  llvm::SmallVector<std::unique_ptr<sandboxir::Region>> Regions =
+      sandboxir::Region::createRegionsFromMD(*F);
+  ASSERT_EQ(Regions.size(), 1u);
+  TPass.runOnRegion(*Regions[0], sandboxir::Analyses::emptyForTesting());
+  EXPECT_EQ(InstCount, 2u);
+#ifndef NDEBUG
+  {
+    // Check print().
+    std::string Buff;
+    llvm::raw_string_ostream SS(Buff);
+    TPass.print(SS);
+    EXPECT_EQ(Buff, "test-pass");
+  }
+  {
+    // Check operator<<().
+    std::string Buff;
+    llvm::raw_string_ostream SS(Buff);
+    SS << TPass;
+    EXPECT_EQ(Buff, "test-pass");
+  }
+  // Check pass name assertions.
+  class TestNamePass final : public sandboxir::RegionPass {
+  public:
+    TestNamePass(llvm::StringRef Name) : RegionPass(Name) {}
+    bool runOnRegion(sandboxir::Region &F, const sandboxir::Analyses &A) {
+      return false;
+    }
+  };
+  EXPECT_DEATH(TestNamePass("white space"), ".*whitespace.*");
+  EXPECT_DEATH(TestNamePass("-dash"), ".*start with.*");
+#endif
+}
+
+TEST_F(RegionTest, RegionPassManager) {
+  parseIR(C, R"IR(
+define i8 @foo(i8 %v0, i8 %v1) {
+  %t0 = add i8 %v0, 1
+  %t1 = add i8 %t0, %v1, !sandboxvec !0
+  %t2 = add i8 %t1, %v1, !sandboxvec !0
+  ret i8 %t1
+}
+
+!0 = distinct !{!"sandboxregion"}
+)IR");
+  llvm::Function *LLVMF = &*M->getFunction("foo");
+  sandboxir::Context Ctx(C);
+  auto *F = Ctx.createFunction(LLVMF);
+
+  class TestPass1 final : public sandboxir::RegionPass {
+    unsigned &InstCount;
+
+  public:
+    TestPass1(unsigned &InstCount)
+        : RegionPass("test-pass1"), InstCount(InstCount) {}
+    bool runOnRegion(sandboxir::Region &R, const sandboxir::Analyses &A) final {
+      for ([[maybe_unused]] auto &Inst : R)
+        ++InstCount;
+      return false;
+    }
+  };
+  class TestPass2 final : public sandboxir::RegionPass {
+    unsigned &InstCount;
+
+  public:
+    TestPass2(unsigned &InstCount)
+        : RegionPass("test-pass2"), InstCount(InstCount) {}
+    bool runOnRegion(sandboxir::Region &R, const sandboxir::Analyses &A) final {
+      for ([[maybe_unused]] auto &Inst : R)
+        ++InstCount;
+      return false;
+    }
+  };
+  unsigned InstCount1 = 0;
+  unsigned InstCount2 = 0;
+
+  sandboxir::RegionPassManager RPM("test-rpm");
+  RPM.addPass(std::make_unique<TestPass1>(InstCount1));
+  RPM.addPass(std::make_unique<TestPass2>(InstCount2));
+  // Check runOnRegion().
+  llvm::SmallVector<std::unique_ptr<sandboxir::Region>> Regions =
+      sandboxir::Region::createRegionsFromMD(*F);
+  ASSERT_EQ(Regions.size(), 1u);
+  RPM.runOnRegion(*Regions[0], sandboxir::Analyses::emptyForTesting());
+  EXPECT_EQ(InstCount1, 2u);
+  EXPECT_EQ(InstCount2, 2u);
+#ifndef NDEBUG
+  // Check dump().
+  std::string Buff;
+  llvm::raw_string_ostream SS(Buff);
+  RPM.print(SS);
+  EXPECT_EQ(Buff, "test-rpm(test-pass1,test-pass2)");
+#endif // NDEBUG
+}

@github-actions
Copy link

github-actions bot commented Nov 21, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

#endif
};

/// A pass that runs on a Region.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Declaring the RegionPass class in Region.h is a bit strange. Wouldn't it make more sense to move Pass.* to the vectorizer too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Including things like FunctionPass that operate on non-vectorizer-specific SandboxIR concepts like Function? I guess the question is whether we believe SandboxVectorizer is the only user that will create SandboxIR sub-pass pipelines or they can be useful elsewhere.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as it is not vectorizer specific, I would move it into SandboxIR ( also for modularity).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the Region-related stuff is currently already in SandboxIR, so "move it into SandboxIR" is just dropping this PR (which is, of course, an option). Or were you talking about something different?

@slackito slackito closed this Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants