-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[SandboxVectorizer] Use sbvec-passes flag to create a pipeline of Region passes after BottomUpVec. #111223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
[SandboxVectorizer] Use sbvec-passes flag to create a pipeline of Region passes after BottomUpVec. #111223
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
1c1145a
[SandboxVectorizer] Use sbvec-passes flag to create a pipeline of Reg…
slackito 674c91e
clang-format
slackito ee3e3d3
Moved the region pass pipeline to BottomUpVec.
slackito 4b4c063
Make BottomUpVec own its RegionPassManager.
slackito 1772712
Remove PassRegistry, use a .def file for vectorizer RegionPasses.
slackito fe5f7d8
Added cleanup in PassRegistry.def
slackito 196b167
Moved pipeline creation to a PassManager method.
slackito 314bf8f
Assert on multiple calls to setPassPipeline. Update unit test.
slackito File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
19 changes: 19 additions & 0 deletions
19
llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| #ifndef LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_NULLPASS_H | ||
| #define LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_NULLPASS_H | ||
|
|
||
| #include "llvm/SandboxIR/Pass.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: | ||
| NullPass() : RegionPass("null") {} | ||
| bool runOnRegion(Region &R) final { return false; } | ||
| }; | ||
|
|
||
| } // namespace llvm::sandboxir | ||
|
|
||
| #endif // LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_PASSES_NULLPASS_H |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,12 @@ | ||
| ; RUN: opt -passes=sandbox-vectorizer -sbvec-print-pass-pipeline -sbvec-passes=bottom-up-vec,bottom-up-vec %s -disable-output | FileCheck %s | ||
| ; RUN: opt -passes=sandbox-vectorizer -sbvec-print-pass-pipeline -sbvec-passes=null,null %s -disable-output | FileCheck %s | ||
|
|
||
| ; !!!WARNING!!! This won't get updated by update_test_checks.py ! | ||
|
|
||
| ; This checks the user defined pass pipeline. | ||
| define void @pipeline() { | ||
| ; CHECK: pm | ||
| ; CHECK: bottom-up-vec | ||
| ; CHECK: bottom-up-vec | ||
| ; CHECK: rpm | ||
| ; CHECK: null | ||
| ; CHECK: null | ||
| ; CHECK-EMPTY: | ||
| ret void | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this function going to be used for non RegionPasses too? If not, should it be a member function of RPM?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be a member of RPM. The moral equivalent of this function for
FunctionPassManageris currently a member function ofPassRegistry:If we change
PassRegistry::parseAndCreatePassPipelineto get the pass manager as an argument rather than creating it inside the function, I think we could easily make it generic to avoid duplicating the pipeline parsing code:What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(and maybe change the
Createpart of theparseAndCreatePassPipelinename to something that better describes the new behavior)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm then perhaps we should deprecate the PassRegistry in favor of a PassManager that owns the passes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need something to hold the mapping between names and passes so we can dynamically create pass pipelines based on the flag value. Right now the PassRegistry is that something. What are you proposing exactly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why we need a parsePassPipeline() in more than one place. Is the PassRegistry's
parsePassPipeline()still being used after this patch?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it is, no. That's why I proposed templatizing it so we can create pipelines of sandbox IR function passes, region passes, etc with a single parsing function. In the future, if there is more than one sandbox IR function pass, we'll probably want to create arbitrary pipelines out of them too. But I can also just delete that member function if we think we aren't going to need it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah templatizing it is fine, we can do it when needed.
That's what I was thinking of. It seems that we are not going to need it.
Also if the PassRegistry's role ends up being just a mapping from name to pass class, then we could use a .def file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about this?