[SandboxVec] Add BottomUpVec test flag to build regions from metadata. #111904

slackito · 2024-10-10T19:41:56Z

This allows us to write lit tests for region passes where regions are specified by metadata in textual IR. See added test file for an example.

llvmbot · 2024-10-10T19:42:29Z

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-vectorizers

Author: Jorge Gorbe Moya (slackito)

Changes

This allows us to write lit tests for region passes where regions are specified by metadata in textual IR. See added test file for an example.

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

4 Files Affected:

(added) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCountPass.h (+24)
(modified) llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp (+15)
(modified) llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/PassRegistry.def (+1)
(added) llvm/test/Transforms/SandboxVectorizer/regions-from-metadata.ll (+16)

diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCountPass.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCountPass.h
new file mode 100644
index 00000000000000..d8cfdcb672ccce
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCountPass.h
@@ -0,0 +1,24 @@
+#ifndef LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_PRINTINSTRUCTIONCOUNTPASS_H
+#define LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_PRINTINSTRUCTIONCOUNTPASS_H
+
+#include "llvm/SandboxIR/Pass.h"
+
+namespace llvm::sandboxir {
+
+class Region;
+
+/// A Region pass that prints the instruction count for the region to stdout.
+/// Used to test -sbvec-passes while we don't have any actual optimization
+/// passes.
+class PrintInstructionCountPass final : public RegionPass {
+public:
+  PrintInstructionCountPass() : RegionPass("null") {}
+  bool runOnRegion(Region &R) final {
+    outs() << "InstructionCount: " << std::distance(R.begin(), R.end()) << "\n";
+    return false;
+  }
+};
+
+} // namespace llvm::sandboxir
+
+#endif // LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_PRINTINSTRUCTIONCOUNTPASS_H
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
index 77198f932a3ec0..7743f5c50ff913 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
@@ -10,8 +10,10 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/SandboxIR/Function.h"
 #include "llvm/SandboxIR/Instruction.h"
+#include "llvm/SandboxIR/Region.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/PrintInstructionCountPass.h"
 
 namespace llvm::sandboxir {
 
@@ -19,6 +21,11 @@ static cl::opt<bool>
     PrintPassPipeline("sbvec-print-pass-pipeline", cl::init(false), cl::Hidden,
                       cl::desc("Prints the pass pipeline and returns."));
 
+static cl::opt<bool> UseRegionsFromMetadata(
+    "sbvec-use-regions-from-metadata", cl::init(false), cl::Hidden,
+    cl::desc("Skips bottom-up vectorization, builds regions from metadata "
+             "already present in the IR and runs the region pass pipeline."));
+
 /// A magic string for the default pass pipeline.
 static const char *DefaultPipelineMagicStr = "*";
 
@@ -86,6 +93,14 @@ bool BottomUpVec::runOnFunction(Function &F) {
     RPM.printPipeline(outs());
     return false;
   }
+  if (UseRegionsFromMetadata) {
+    SmallVector<std::unique_ptr<Region>> Regions =
+        Region::createRegionsFromMD(F);
+    for (auto &R : Regions) {
+      RPM.runOnRegion(*R);
+    }
+    return false;
+  }
 
   Change = false;
   // TODO: Start from innermost BBs first
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/PassRegistry.def b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/PassRegistry.def
index bbb0dcba1ea516..c0ad1710d06b91 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/PassRegistry.def
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/PassRegistry.def
@@ -18,5 +18,6 @@
 #endif
 
 REGION_PASS("null", NullPass())
+REGION_PASS("print-instruction-count", PrintInstructionCountPass())
 
 #undef REGION_PASS
diff --git a/llvm/test/Transforms/SandboxVectorizer/regions-from-metadata.ll b/llvm/test/Transforms/SandboxVectorizer/regions-from-metadata.ll
new file mode 100644
index 00000000000000..3874876996f8c0
--- /dev/null
+++ b/llvm/test/Transforms/SandboxVectorizer/regions-from-metadata.ll
@@ -0,0 +1,16 @@
+; RUN: opt -disable-output --passes=sandbox-vectorizer \
+; RUN:    -sbvec-passes=print-instruction-count \
+; RUN:    -sbvec-use-regions-from-metadata %s | FileCheck %s
+
+define i8 @foo(i8 %v0, i8 %v1) {
+  %t0 = add i8 %v0, 1, !sandboxvec !0
+  %t1 = add i8 %t0, %v1, !sandboxvec !1
+  %t2 = add i8 %t1, %v1, !sandboxvec !1
+  ret i8 %t2
+}
+
+!0 = distinct !{!"sandboxregion"}
+!1 = distinct !{!"sandboxregion"}
+
+; CHECK: InstructionCount: 1
+; CHECK: InstructionCount: 2

aeubanks

I thought we were going to have a separate function pass for the purposes of testing, rather than hacking this into BoUpVec

printing instruction count is good

llvm/test/Transforms/SandboxVectorizer/regions-from-metadata.ll

slackito · 2024-10-10T21:43:02Z

I thought we were going to have a separate function pass for the purposes of testing, rather than hacking this into BoUpVec

I wasn't feeling comfortable with the idea of adding a new pass to the overall list just to test the internals of the vectorizer.

…rizer class.

slackito · 2024-10-10T23:51:28Z

Chatted offline about moving it to the main SandboxVectorizer pass rather than BottomUpVec. What do you think about this?

aeubanks · 2024-10-11T19:53:18Z

llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h

@@ -29,10 +29,11 @@ class BottomUpVec final : public FunctionPass {
  void tryVectorize(ArrayRef<Value *> Seeds);

  // The PM containing the pipeline of region passes.
-  RegionPassManager RPM;
+  // TODO: Remove maybe_unused once we build regions to run passes on.
+  [[maybe_unused]] RegionPassManager *RPM;


can this be a reference?

tmsri · 2024-10-11T20:23:43Z

llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp

+    for (auto &R : Regions) {
+      RPM.runOnRegion(*R);
+    }
+    return false;


I am not following the various discussions. Why can't this be another function pass called "TestsFuncPass" which then runs the various regions?

It can. I'm just having some trouble figuring out what we want.

…dboxVec pass pipelines. My previous attempt (llvm#111904) hacked creation of Regions from metadata into the bottom-up vectorizer. I got some feedback that it should be its own pass. So now we have two SandboxIR function passes (`BottomUpVec` and `RegionsFromMetadata`) that are interchangeable, and we could have other SandboxIR function passes doing other kinds of transforms, so this commit revamps pipeline creation and parsing. First, `sandboxir::PassManager::setPassPipeline` now accepts pass arguments in angle brackets. Pass arguments are arbitrary strings that must be parsed by each pass, the only requirement is that nested angle bracket pairs must be balanced, to allow for nested pipelines with more arguments. For example: ``` bottom-up-vec<region-pass-1,region-pass-2<arg>,region-pass-3> ``` This has complicated the parser a little bit (the loop over pipeline characters now contains a small state machine), and we now have some new test cases to exercise the new features. The main SandboxVectorizerPass now contains a customizable pipeline of SandboxIR function passes, defined by the `sbvec-passes` flag. Region passes for the bottom-up vectorizer pass are now in pass arguments (like in the example above). Because we have now several classes that can build sub-pass pipelines, I've moved the logic that interacts with PassRegistry.def into its own files (PassBuilder.{h,cpp} so it can be easily reused. Finally, I've added a `RegionsFromMetadata` function pass, which will allow us to run region passes in isolation from lit tests without relying on the bottom-up vectorizer, and a new lit test that does exactly this. Note that the new pipeline parser now allows empty pipelines. This is useful for testing. For example, if we use ``` -sbvec-passes="bottom-up-vec<>" ``` SandboxVectorizer converts LLVM IR to SandboxIR and runs the bottom-up vectorizer, but no region passes afterwards. ``` -sbvec-passes="" ``` SandboxVectorizer converts LLVM IR to SandboxIR and runs no passes on it. This is useful to exercise SandboxIR conversion on its own.

…dboxVec pass pipelines. (#112288) My previous attempt (#111904) hacked creation of Regions from metadata into the bottom-up vectorizer. I got some feedback that it should be its own pass. So now we have two SandboxIR function passes (`BottomUpVec` and `RegionsFromMetadata`) that are interchangeable, and we could have other SandboxIR function passes doing other kinds of transforms, so this commit revamps pipeline creation and parsing. First, `sandboxir::PassManager::setPassPipeline` now accepts pass arguments in angle brackets. Pass arguments are arbitrary strings that must be parsed by each pass, the only requirement is that nested angle bracket pairs must be balanced, to allow for nested pipelines with more arguments. For example: ``` bottom-up-vec<region-pass-1,region-pass-2<arg>,region-pass-3> ``` This has complicated the parser a little bit (the loop over pipeline characters now contains a small state machine), and we now have some new test cases to exercise the new features. The main SandboxVectorizerPass now contains a customizable pipeline of SandboxIR function passes, defined by the `sbvec-passes` flag. Region passes for the bottom-up vectorizer pass are now in pass arguments (like in the example above). Because we have now several classes that can build sub-pass pipelines, I've moved the logic that interacts with PassRegistry.def into its own files (PassBuilder.{h,cpp} so it can be easily reused. Finally, I've added a `RegionsFromMetadata` function pass, which will allow us to run region passes in isolation from lit tests without relying on the bottom-up vectorizer, and a new lit test that does exactly this. Note that the new pipeline parser now allows empty pipelines. This is useful for testing. For example, if we use ``` -sbvec-passes="bottom-up-vec<>" ``` SandboxVectorizer converts LLVM IR to SandboxIR and runs the bottom-up vectorizer, but no region passes afterwards. ``` -sbvec-passes="" ``` SandboxVectorizer converts LLVM IR to SandboxIR and runs no passes on it. This is useful to exercise SandboxIR conversion on its own.

slackito · 2024-10-15T23:10:52Z

Abandoned in favor of #112288. Closing.

…dboxVec pass pipelines. (llvm#112288) My previous attempt (llvm#111904) hacked creation of Regions from metadata into the bottom-up vectorizer. I got some feedback that it should be its own pass. So now we have two SandboxIR function passes (`BottomUpVec` and `RegionsFromMetadata`) that are interchangeable, and we could have other SandboxIR function passes doing other kinds of transforms, so this commit revamps pipeline creation and parsing. First, `sandboxir::PassManager::setPassPipeline` now accepts pass arguments in angle brackets. Pass arguments are arbitrary strings that must be parsed by each pass, the only requirement is that nested angle bracket pairs must be balanced, to allow for nested pipelines with more arguments. For example: ``` bottom-up-vec<region-pass-1,region-pass-2<arg>,region-pass-3> ``` This has complicated the parser a little bit (the loop over pipeline characters now contains a small state machine), and we now have some new test cases to exercise the new features. The main SandboxVectorizerPass now contains a customizable pipeline of SandboxIR function passes, defined by the `sbvec-passes` flag. Region passes for the bottom-up vectorizer pass are now in pass arguments (like in the example above). Because we have now several classes that can build sub-pass pipelines, I've moved the logic that interacts with PassRegistry.def into its own files (PassBuilder.{h,cpp} so it can be easily reused. Finally, I've added a `RegionsFromMetadata` function pass, which will allow us to run region passes in isolation from lit tests without relying on the bottom-up vectorizer, and a new lit test that does exactly this. Note that the new pipeline parser now allows empty pipelines. This is useful for testing. For example, if we use ``` -sbvec-passes="bottom-up-vec<>" ``` SandboxVectorizer converts LLVM IR to SandboxIR and runs the bottom-up vectorizer, but no region passes afterwards. ``` -sbvec-passes="" ``` SandboxVectorizer converts LLVM IR to SandboxIR and runs no passes on it. This is useful to exercise SandboxIR conversion on its own.

[SandboxVec] Add BottomUpVec test flag to build regions from metadata.

61453a0

This allows us to write lit tests for region passes where regions are specified by metadata in textual IR. See added test file for an example.

slackito requested review from alinas, vporpo, aeubanks, tmsri and Sterling-Augustine October 10, 2024 19:41

llvmbot added vectorizers llvm:transforms labels Oct 10, 2024

aeubanks reviewed Oct 10, 2024

View reviewed changes

llvm/test/Transforms/SandboxVectorizer/regions-from-metadata.ll Show resolved Hide resolved

Move pass pipeline and region creation logic to the main SandboxVecto…

10b12d8

…rizer class.

aeubanks reviewed Oct 11, 2024

View reviewed changes

tmsri reviewed Oct 11, 2024

View reviewed changes

slackito mentioned this pull request Oct 14, 2024

[SandboxVec] Add pass to create Regions from metadata. Generalize SandboxVec pass pipelines. #112288

Merged

slackito closed this Oct 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SandboxVec] Add BottomUpVec test flag to build regions from metadata. #111904

[SandboxVec] Add BottomUpVec test flag to build regions from metadata. #111904

Uh oh!

slackito commented Oct 10, 2024

Uh oh!

llvmbot commented Oct 10, 2024 •

edited

Loading

Uh oh!

aeubanks left a comment

Uh oh!

Uh oh!

slackito commented Oct 10, 2024

Uh oh!

slackito commented Oct 10, 2024

Uh oh!

aeubanks Oct 11, 2024

Uh oh!

tmsri Oct 11, 2024

Uh oh!

slackito Oct 11, 2024

Uh oh!

slackito commented Oct 15, 2024

Uh oh!

Uh oh!

[SandboxVec] Add BottomUpVec test flag to build regions from metadata. #111904

[SandboxVec] Add BottomUpVec test flag to build regions from metadata. #111904

Uh oh!

Conversation

slackito commented Oct 10, 2024

Uh oh!

llvmbot commented Oct 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aeubanks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

slackito commented Oct 10, 2024

Uh oh!

slackito commented Oct 10, 2024

Uh oh!

aeubanks Oct 11, 2024

Choose a reason for hiding this comment

Uh oh!

tmsri Oct 11, 2024

Choose a reason for hiding this comment

Uh oh!

slackito Oct 11, 2024

Choose a reason for hiding this comment

Uh oh!

slackito commented Oct 15, 2024

Uh oh!

Uh oh!

llvmbot commented Oct 10, 2024 •

edited

Loading