[SandboxVectorizer] Use sbvec-passes flag to create a pipeline of Region passes after BottomUpVec. #111223
Conversation
…ion passes after BottomUpVec. The main change is that the main SandboxVectorizer pass no longer has a pipeline of function passes. Now it consists of a BottomUpVec pass and a pipeline of region passes after it. BottomUpVec now takes a RegionPassManager as an argument to the constructor. This argument is currently stored and not used, but the idea is that after BottomUpVec vectorizes a region of a function, we'll run successive optimization passes on that region. I've also moved creation of the pass pipeline out of the `run` method to the SandboxVectorizer constructor.
slackito
requested review from
alinas,
vporpo,
tschuett,
aeubanks,
tmsri and
Sterling-Augustine
|
I'm not sure this is the way it should work but if not at least I hope to get some clarity out of the review :) |
|
@llvm/pr-subscribers-llvm-transforms Author: Jorge Gorbe Moya (slackito) ChangesThe main change is that the main SandboxVectorizer pass no longer has a pipeline of function passes. Now it consists of a BottomUpVec pass and a pipeline of region passes after it. BottomUpVec now takes a RegionPassManager as an argument to the constructor. This argument is currently stored and not used, but the idea is that after BottomUpVec vectorizes a region of a function, we'll run successive optimization passes on that region. I've also moved creation of the pass pipeline out of the Full diff: https://github.com/llvm/llvm-project/pull/111223.diff 7 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
index a2108f07c28e50..e3a35b09d0e296 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
@@ -19,14 +19,19 @@
namespace llvm::sandboxir {
+class RegionPassManager;
+
class BottomUpVec final : public FunctionPass {
bool Change = false;
LegalityAnalysis Legality;
void vectorizeRec(ArrayRef<Value *> Bndl);
void tryVectorize(ArrayRef<Value *> Seeds);
+ [[maybe_unused]] RegionPassManager *RPM;
+
public:
- BottomUpVec() : FunctionPass("bottom-up-vec") {}
+ BottomUpVec(RegionPassManager *RPM)
+ : FunctionPass("bottom-up-vec"), RPM(RPM) {}
bool runOnFunction(Function &F) final;
};
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h
new file mode 100644
index 00000000000000..75b9f42520156c
--- /dev/null
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h
@@ -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
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.h
index dd9f02d3272643..4547217773cd01 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.h
@@ -8,7 +8,11 @@
#ifndef LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_SANDBOXVECTORIZER_H
#define LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_SANDBOXVECTORIZER_H
+#include <memory>
+
#include "llvm/IR/PassManager.h"
+#include "llvm/SandboxIR/PassManager.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h"
namespace llvm {
@@ -17,10 +21,21 @@ class TargetTransformInfo;
class SandboxVectorizerPass : public PassInfoMixin<SandboxVectorizerPass> {
TargetTransformInfo *TTI = nullptr;
-public:
- PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
+ // Used to build a RegionPass pipeline to be run on Regions created by the
+ // bottom-up vectorization pass.
+ sandboxir::PassRegistry PR;
+
+ // The main vectorizer pass.
+ std::unique_ptr<sandboxir::BottomUpVec> BottomUpVecPass;
+
+ // The PM containing the pipeline of region passes. It's owned by the pass
+ // registry.
+ sandboxir::RegionPassManager *RPM;
bool runImpl(Function &F);
+public:
+ SandboxVectorizerPass();
+ PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
};
} // namespace llvm
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
index c59abd09d43629..69ddfaf7784eae 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
@@ -59,6 +59,8 @@ bool BottomUpVec::runOnFunction(Function &F) {
// TODO: Replace with proper SeedCollector function.
auto Seeds = collectSeeds(BB);
// TODO: Slice Seeds into smaller chunks.
+ // TODO: If vectorization succeeds, run the RegionPassManager on the
+ // resulting region.
if (Seeds.size() >= 2)
tryVectorize(Seeds);
}
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp
index 80afcb499a2c22..1192cc5d9bb243 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp
@@ -12,6 +12,7 @@
#include "llvm/SandboxIR/PassManager.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h"
using namespace llvm;
@@ -30,6 +31,57 @@ cl::opt<std::string> UserDefinedPassPipeline(
cl::desc("Comma-separated list of vectorizer passes. If not set "
"we run the predefined pipeline."));
+static void registerAllRegionPasses(sandboxir::PassRegistry &PR) {
+ PR.registerPass(std::make_unique<sandboxir::NullPass>());
+}
+
+static sandboxir::RegionPassManager &
+parseAndCreatePassPipeline(sandboxir::PassRegistry &PR, StringRef Pipeline) {
+ static constexpr const char EndToken = '\0';
+ // Add EndToken to the end to ease parsing.
+ std::string PipelineStr = std::string(Pipeline) + EndToken;
+ int FlagBeginIdx = 0;
+ auto &RPM = static_cast<sandboxir::RegionPassManager &>(
+ PR.registerPass(std::make_unique<sandboxir::RegionPassManager>("rpm")));
+
+ for (auto [Idx, C] : enumerate(PipelineStr)) {
+ // Keep moving Idx until we find the end of the pass name.
+ bool FoundDelim = C == EndToken || C == PR.PassDelimToken;
+ if (!FoundDelim)
+ continue;
+ unsigned Sz = Idx - FlagBeginIdx;
+ std::string PassName(&PipelineStr[FlagBeginIdx], Sz);
+ FlagBeginIdx = Idx + 1;
+
+ // Get the pass that corresponds to PassName and add it to the pass manager.
+ auto *Pass = PR.getPassByName(PassName);
+ if (Pass == nullptr) {
+ errs() << "Pass '" << PassName << "' not registered!\n";
+ exit(1);
+ }
+ // TODO: Add a type check here. The downcast is correct as long as
+ // registerAllRegionPasses only registers regions passes.
+ RPM.addPass(static_cast<sandboxir::RegionPass *>(Pass));
+ }
+ return RPM;
+}
+
+SandboxVectorizerPass::SandboxVectorizerPass() {
+ registerAllRegionPasses(PR);
+
+ // Create a pipeline to be run on each Region created by BottomUpVec.
+ if (UserDefinedPassPipeline == DefaultPipelineMagicStr) {
+ // Create the default pass pipeline.
+ RPM = &static_cast<sandboxir::RegionPassManager &>(PR.registerPass(
+ std::make_unique<sandboxir::FunctionPassManager>("rpm")));
+ // TODO: Add passes to the default pipeline.
+ } else {
+ // Create the user-defined pipeline.
+ RPM = &parseAndCreatePassPipeline(PR, UserDefinedPassPipeline);
+ }
+ BottomUpVecPass = std::make_unique<sandboxir::BottomUpVec>(RPM);
+}
+
PreservedAnalyses SandboxVectorizerPass::run(Function &F,
FunctionAnalysisManager &AM) {
TTI = &AM.getResult<TargetIRAnalysis>(F);
@@ -56,31 +108,13 @@ bool SandboxVectorizerPass::runImpl(Function &LLVMF) {
return false;
}
- sandboxir::Context Ctx(LLVMF.getContext());
- // Create SandboxIR for `LLVMF`.
- sandboxir::Function &F = *Ctx.createFunction(&LLVMF);
- // Create the passes and register them with the PassRegistry.
- sandboxir::PassRegistry PR;
- auto &BottomUpVecPass = static_cast<sandboxir::FunctionPass &>(
- PR.registerPass(std::make_unique<sandboxir::BottomUpVec>()));
-
- sandboxir::FunctionPassManager *PM = nullptr;
- if (UserDefinedPassPipeline == DefaultPipelineMagicStr) {
- // Create the default pass pipeline.
- PM = &static_cast<sandboxir::FunctionPassManager &>(PR.registerPass(
- std::make_unique<sandboxir::FunctionPassManager>("pm")));
- PM->addPass(&BottomUpVecPass);
- } else {
- // Create the user-defined pipeline.
- PM = &PR.parseAndCreatePassPipeline(UserDefinedPassPipeline);
- }
-
if (PrintPassPipeline) {
- PM->printPipeline(outs());
+ RPM->printPipeline(outs());
return false;
}
- // Run the pass pipeline.
- bool Change = PM->runOnFunction(F);
- return Change;
+ // Create SandboxIR for LLVMF and run BottomUpVec on it.
+ sandboxir::Context Ctx(LLVMF.getContext());
+ sandboxir::Function &F = *Ctx.createFunction(&LLVMF);
+ return BottomUpVecPass->runOnFunction(F);
}
diff --git a/llvm/test/Transforms/SandboxVectorizer/default_pass_pipeline.ll b/llvm/test/Transforms/SandboxVectorizer/default_pass_pipeline.ll
index 5ccd64d9f487a3..86bfbee6364788 100644
--- a/llvm/test/Transforms/SandboxVectorizer/default_pass_pipeline.ll
+++ b/llvm/test/Transforms/SandboxVectorizer/default_pass_pipeline.ll
@@ -4,8 +4,7 @@
; This checks the default pass pipeline for the sandbox vectorizer.
define void @pipeline() {
-; CHECK: pm
-; CHECK: bottom-up-vec
+; CHECK: rpm
; CHECK-EMPTY:
ret void
}
diff --git a/llvm/test/Transforms/SandboxVectorizer/user_pass_pipeline.ll b/llvm/test/Transforms/SandboxVectorizer/user_pass_pipeline.ll
index 2879fbba1b9c00..2e6dab0aa29c74 100644
--- a/llvm/test/Transforms/SandboxVectorizer/user_pass_pipeline.ll
+++ b/llvm/test/Transforms/SandboxVectorizer/user_pass_pipeline.ll
@@ -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
}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| sandboxir::PassRegistry PR; | ||
|
|
||
| // The main vectorizer pass. | ||
| std::unique_ptr<sandboxir::BottomUpVec> BottomUpVecPass; |
There was a problem hiding this comment.
Why is this a member variable? It's created and used only within runOnFunction() at the moment, so making it a class member implies that it used across multiple member functions of SandboxVectorizerPass. Are you planning to move it to a separate class that SandboxVectorizerPass inherits from?
There was a problem hiding this comment.
if we move away from cl::opt("sbvec-passes") and to proper pass parameters, we would want the LLVM pass to hold the region pass manager in the class, rather than keep it local to runImpl, because we'd specify the region pass pipeline via the constructor
but with cl::opt I don't think it really matters
There was a problem hiding this comment.
actually, I thought that BottomUpVecPass would own the region pass manager so it could invoke it when it creates a region
There was a problem hiding this comment.
making it a class member implies that it used across multiple member functions of SandboxVectorizerPass
Not the only reason possible to make something a member. Making it local to runImpl means constructing the BottomUpVec pass every time, which seems like a waste to me.
Also, conceptually I see the SandboxVectorizer pass as a wrapper around BottomUpVec that handles the creation of sandbox IR. I don't see anything wrong with the wrapper owning the thing it's wrapping.
There was a problem hiding this comment.
Not the only reason possible to make something a member. Making it local to runImpl means constructing the BottomUpVec pass every time, which seems like a waste to me.
Ah yes, I forgot that we were building the pass pipeline over and over again.
There was a problem hiding this comment.
actually, I thought that BottomUpVecPass would own the region pass manager so it could invoke it when it creates a region
I missed BottomUpVec also having a reference to the region pass manager
|
|
||
| // The PM containing the pipeline of region passes. It's owned by the pass | ||
| // registry. | ||
| sandboxir::RegionPassManager *RPM; |
| registerAllRegionPasses(PR); | ||
|
|
||
| // Create a pipeline to be run on each Region created by BottomUpVec. | ||
| if (UserDefinedPassPipeline == DefaultPipelineMagicStr) { |
There was a problem hiding this comment.
Same for this stuff. Not sure why they should be in the pass constructor.
There was a problem hiding this comment.
Why recreate the pipeline for each function we ever run the vectorizer on, if it's defined by a command-line flag and won't change during a compiler invocation?
There was a problem hiding this comment.
Yeah totally forgot that we were rebuilding it every time.
| sandboxir::PassRegistry PR; | ||
|
|
||
| // The main vectorizer pass. | ||
| std::unique_ptr<sandboxir::BottomUpVec> BottomUpVecPass; |
There was a problem hiding this comment.
actually, I thought that BottomUpVecPass would own the region pass manager so it could invoke it when it creates a region
I missed BottomUpVec also having a reference to the region pass manager
| // Create the user-defined pipeline. | ||
| PM = &PR.parseAndCreatePassPipeline(UserDefinedPassPipeline); | ||
| } | ||
|
|
||
| if (PrintPassPipeline) { |
There was a problem hiding this comment.
oh, is this the only reason SandboxVectorizerPass has a reference to the region pass manager? I think ideally only the BoUpVectorizer has a reference to it, and we can call BottomUpVecPass->printPipeline(), which forwards to RPM->printPipeline()
There was a problem hiding this comment.
Yeah, I'm fine with BottomUpVec owning the region pass manager instead. I considered it and took the lazy way out because it made it easier to print the pipeline here :)
I'll change it.
There was a problem hiding this comment.
Done. I'm not sure I'm completely happy with the pattern where the RegionPassManager object is actually owned by the PassRegistry, but I don't think it will cause any problems either and if we go with it we can always change it afterwards.
There was a problem hiding this comment.
yeah, all the non-owning pointers make me sus
Everything related to the pass pipeline is now in BottomUpVec.cpp, including pipeline creation and printing and the flags to control it. I have also changed the `BottomUpVecPass` in `SandboxVectorizer` from a `unique_ptr` to a plain old member, as I don't think we need that level of indirection for anything.
| /// A magic string for the default pass pipeline. | ||
| const char *DefaultPipelineMagicStr = "*"; | ||
|
|
||
| cl::opt<std::string> UserDefinedPassPipeline( |
There was a problem hiding this comment.
this can be static/anonymous namespace
| // Create the user-defined pipeline. | ||
| PM = &PR.parseAndCreatePassPipeline(UserDefinedPassPipeline); | ||
| } | ||
|
|
||
| if (PrintPassPipeline) { |
There was a problem hiding this comment.
yeah, all the non-owning pointers make me sus
Also made some flag-related globals static.
Got rid of the non-owning pointer. Now the region pass manager is a simple field of |
|
|
||
| /// Adds to `RPM` a sequence of passes described by `Pipeline` from the `PR` | ||
| /// pass registry. | ||
| static void parseAndCreatePassPipeline(RegionPassManager &RPM, PassRegistry &PR, |
There was a problem hiding this comment.
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.
It could be a member of RPM. The moral equivalent of this function for FunctionPassManager is currently a member function of PassRegistry:
class PassRegistry {
...
FunctionPassManager &parseAndCreatePassPipeline(StringRef Pipeline);
};
If we change PassRegistry::parseAndCreatePassPipeline to 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:
class PassRegistry {
...
template <typename PassManagerType>
void parseAndCreatePassPipeline(PassManagerType &PM, StringRef Pipeline);
};
What do you think?
There was a problem hiding this comment.
(and maybe change the Create part of the parseAndCreatePassPipeline name to something that better describes the new behavior)
There was a problem hiding this comment.
Hmm then perhaps we should deprecate the PassRegistry in favor of a PassManager that owns the passes?
There was a problem hiding this comment.
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.
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.
Is the PassRegistry's
parsePassPipeline()still being used after this patch?
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.
Yeah templatizing it is fine, we can do it when needed.
But I can also just delete that member function if we think we aren't going to need it.
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.
PassManagers now own the passes they contain, and pipeline parsing is done by a standalone function in BottomUpVec.cpp.
| #define REGION_PASS(NAME, CREATE_PASS) | ||
| #endif | ||
|
|
||
| REGION_PASS("null", NullPass()) |
There was a problem hiding this comment.
I think we also need some cleanup code at the end like:
#ifdef REGION_PASS
#undef REGION_PASS
#endif
to avoid getting a warning of redefining REGION_PASS if we ever decide to include the .def file more than once
There was a problem hiding this comment.
Added #undef REGION_PASS. By the time we get to the end the macro is always defined, either to something useful by the user or to an empty macro above.
| #define REGION_PASS(NAME, CREATE_PASS) | ||
| #endif | ||
|
|
||
| REGION_PASS("null", NullPass()) |
There was a problem hiding this comment.
Also why use NullPass() and not just the class name NullPass in REGION_PASS("null", NullPass()). Isn't the class name easier to work with?
There was a problem hiding this comment.
I just followed the pattern for the existing FUNCTION_PASS in llvm/lib/Passes/PassRegistry.def. I have no preference one way or the other.
| } | ||
|
|
||
| /// Adds to `RPM` a sequence of region passes described by `Pipeline`. | ||
| static void parseAndCreatePassPipeline(RegionPassManager &RPM, |
There was a problem hiding this comment.
Now that it's the pass managers that own the passes it looks a bit out of place that populating them is done with a static function, because the only users of it are the pass managers.
Wdyt about moving this to the PassManager base class?
There was a problem hiding this comment.
If we do this, then sandboxir::PassManager has to know about vectorizer-specific passes. Is this a desired outcome?
There was a problem hiding this comment.
I meant making parseAndCreatePassPipeline a member function of PassManager such that we call:
RPM.parseAndCreatePassPipeline(UserDefinedPassPipeline);
instead of:
parseAndCreatePassPipeline(RPM, UserDefinedPassPipeline);
There was a problem hiding this comment.
Right. But parseAndCreatePassPipeline needs to know about the existing vectorizer passes in order to instantiate them.
Do you want llvm/lib/SandboxIR/PassManager.cpp to include all the passes from llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/* ? Should we move RegionPassManager back into the vectorizer?
There was a problem hiding this comment.
Well, conceptually parsing the pass pipeline string and building passes should not be a static function in BottomUpVec.cpp because it is a pass-manager task, not a vectorization-specific task.
Oh yes, the creator function would need to have access to the passes. Hmm could we perhaps keep the creator function here in a lambda createPass(Name) and pass the lambda as an argument to the parse function?
There was a problem hiding this comment.
Done. Left createPass as a static function in BottomUpVec.h instead of a lambda because the includes and defines needed for the .def file messed up indentation and I preferred to keep that ugliness contained in its own function.
I also added a new unit test, similar to the now deleted PassRegistry test, that checks the pipeline parsing function (we also have some very basic coverage from lit tests)
The new method receives a CreatePass function as an argument to avoid the PassManager class having to "know" about existing passes in, for example, SandboxVectorizer. Also added back a unit test similar to the now deleted PassRegistry test that checks PassManager::setPassPipeline.
| FPM.runOnFunction(*F); | ||
| EXPECT_EQ(Str, "foobarfoo"); | ||
|
|
||
| // Check that a second call to setPassPipeline clears the previous pipeline. |
There was a problem hiding this comment.
Should we allow a second call to setPassPipeline() ? Shouldn't we crash if we try to do so instead?
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/89/builds/8127 Here is the relevant piece of the build log for the reference |
…e of Region passes after BottomUpVec." (#111727) Reverts #111223 It broke one of the build bots: LLVM Buildbot has detected a new failure on builder flang-aarch64-libcxx running on linaro-flang-aarch64-libcxx while building llvm at step 5 "build-unified-tree". Full details are available at: https://lab.llvm.org/buildbot/#/builders/89/builds/8127
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/160/builds/6516 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/180/builds/6514 Here is the relevant piece of the build log for the reference |
…egion passes after BottomUpVec. (#111223)" (#111772) #111223 was reverted because of a build failure with `-DBUILD_SHARED_LIBS=on`. The Passes component depends on Vectorizer (because PassBuilder needs to be able to instantiate SandboxVectorizerPass). This resulted in CMake doing this 1. when it builds lib/libLLVMVectorize.so.20.0git it adds lib/libLLVMSandboxIR.so.20.0git to the command line, because it's listed as a dependency (as expected) 2. when it's trying to build lib/libLLVMPasses.so.20.0git it adds lib/libLLVMVectorize.so.20.0git to the command line, because it's listed as a dependency (also as expected). But not libLLVMSandboxIR.so. When SandboxVectorizerPass has its ctors/dtors defined inline, this caused "undefined reference to vtable" linker errors. This change works around that by moving ctors/dtors out of line. Also fix a bazel build problem by adding the new `llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/PassRegistry.def` as a textual header in the Vectorizer target.
…egion passes after BottomUpVec. (llvm#111223)" (llvm#111772) llvm#111223 was reverted because of a build failure with `-DBUILD_SHARED_LIBS=on`. The Passes component depends on Vectorizer (because PassBuilder needs to be able to instantiate SandboxVectorizerPass). This resulted in CMake doing this 1. when it builds lib/libLLVMVectorize.so.20.0git it adds lib/libLLVMSandboxIR.so.20.0git to the command line, because it's listed as a dependency (as expected) 2. when it's trying to build lib/libLLVMPasses.so.20.0git it adds lib/libLLVMVectorize.so.20.0git to the command line, because it's listed as a dependency (also as expected). But not libLLVMSandboxIR.so. When SandboxVectorizerPass has its ctors/dtors defined inline, this caused "undefined reference to vtable" linker errors. This change works around that by moving ctors/dtors out of line. Also fix a bazel build problem by adding the new `llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/PassRegistry.def` as a textual header in the Vectorizer target.
The main change is that the main SandboxVectorizer pass no longer has a pipeline of function passes. Now it consists of a BottomUpVec pass and a pipeline of region passes after it.
BottomUpVec now takes a RegionPassManager as an argument to the constructor. This argument is currently stored and not used, but the idea is that after BottomUpVec vectorizes a region of a function, we'll run successive optimization passes on that region.
I've also moved creation of the pass pipeline out of the
runmethod to the SandboxVectorizer constructor.