Skip to content

Conversation

@slackito
Copy link
Collaborator

#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.

…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.
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.
Also made some flag-related globals static.
PassManagers now own the passes they contain, and pipeline parsing is
done by a standalone function in BottomUpVec.cpp.
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.
- A problem with shared library builds where building LLVMPasses results
  in CMake adding to the compiler invocation the Vectorize component but
  not the transitive dependency on SandboxIR, causing "missing vtable"
  link errors.

- Added the new
  llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/PassRegistry.def
  file to `textual_hdrs` in the bazel rule.
@llvmbot llvmbot added vectorizers bazel "Peripheral" support tier build system: utils/bazel llvm:transforms labels Oct 10, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 10, 2024

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-vectorizers

Author: Jorge Gorbe Moya (slackito)

Changes

#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.


Patch is 23.78 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/111772.diff

12 Files Affected:

  • (modified) llvm/include/llvm/SandboxIR/PassManager.h (+46-36)
  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h (+7-1)
  • (added) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h (+19)
  • (modified) llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.h (+17-2)
  • (modified) llvm/lib/SandboxIR/PassManager.cpp (+4-40)
  • (modified) llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp (+42-4)
  • (added) llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/PassRegistry.def (+22)
  • (modified) llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp (+6-36)
  • (modified) llvm/test/Transforms/SandboxVectorizer/default_pass_pipeline.ll (+1-2)
  • (modified) llvm/test/Transforms/SandboxVectorizer/user_pass_pipeline.ll (+4-4)
  • (modified) llvm/unittests/SandboxIR/PassTest.cpp (+49-55)
  • (modified) utils/bazel/llvm-project-overlay/llvm/BUILD.bazel (+3)
diff --git a/llvm/include/llvm/SandboxIR/PassManager.h b/llvm/include/llvm/SandboxIR/PassManager.h
index 54192c6bf1333b..247c43615f5766 100644
--- a/llvm/include/llvm/SandboxIR/PassManager.h
+++ b/llvm/include/llvm/SandboxIR/PassManager.h
@@ -18,6 +18,8 @@
 #ifndef LLVM_SANDBOXIR_PASSMANAGER_H
 #define LLVM_SANDBOXIR_PASSMANAGER_H
 
+#include <memory>
+
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/SandboxIR/Pass.h"
@@ -32,25 +34,65 @@ template <typename ParentPass, typename ContainedPass>
 class PassManager : public ParentPass {
 protected:
   /// The list of passes that this pass manager will run.
-  SmallVector<ContainedPass *> Passes;
+  SmallVector<std::unique_ptr<ContainedPass>> Passes;
 
   PassManager(StringRef Name) : ParentPass(Name) {}
   PassManager(const PassManager &) = delete;
+  PassManager(PassManager &&) = default;
   virtual ~PassManager() = default;
   PassManager &operator=(const PassManager &) = delete;
 
 public:
   /// Adds \p Pass to the pass pipeline.
-  void addPass(ContainedPass *Pass) {
+  void addPass(std::unique_ptr<ContainedPass> Pass) {
     // TODO: Check that Pass's class type works with this PassManager type.
-    Passes.push_back(Pass);
+    Passes.push_back(std::move(Pass));
+  }
+
+  using CreatePassFunc =
+      std::function<std::unique_ptr<ContainedPass>(StringRef)>;
+
+  /// Parses \p Pipeline as a comma-separated sequence of pass names and sets
+  /// the pass pipeline, using \p CreatePass to instantiate passes by name.
+  ///
+  /// After calling this function, the PassManager contains only the specified
+  /// pipeline, any previously added passes are cleared.
+  void setPassPipeline(StringRef Pipeline, CreatePassFunc CreatePass) {
+    static constexpr const char EndToken = '\0';
+    static constexpr const char PassDelimToken = ',';
+
+    assert(Passes.empty() &&
+           "setPassPipeline called on a non-empty sandboxir::PassManager");
+    // Add EndToken to the end to ease parsing.
+    std::string PipelineStr = std::string(Pipeline) + EndToken;
+    int FlagBeginIdx = 0;
+
+    for (auto [Idx, C] : enumerate(PipelineStr)) {
+      // Keep moving Idx until we find the end of the pass name.
+      bool FoundDelim = C == EndToken || C == 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 = CreatePass(PassName);
+      if (Pass == nullptr) {
+        errs() << "Pass '" << PassName << "' not registered!\n";
+        exit(1);
+      }
+      addPass(std::move(Pass));
+    }
   }
+
 #ifndef NDEBUG
   void print(raw_ostream &OS) const override {
     OS << this->getName();
     OS << "(";
     // TODO: This should call Pass->print(OS) because Pass may be a PM.
-    interleave(Passes, OS, [&OS](auto *Pass) { OS << Pass->getName(); }, ",");
+    interleave(Passes, OS, [&OS](auto &Pass) { OS << Pass->getName(); }, ",");
     OS << ")";
   }
   LLVM_DUMP_METHOD void dump() const override {
@@ -79,38 +121,6 @@ class RegionPassManager final : public PassManager<RegionPass, RegionPass> {
   bool runOnRegion(Region &R) final;
 };
 
-/// Owns the passes and provides an API to get a pass by its name.
-class PassRegistry {
-  SmallVector<std::unique_ptr<Pass>, 8> Passes;
-  DenseMap<StringRef, Pass *> NameToPassMap;
-
-public:
-  static constexpr const char PassDelimToken = ',';
-  PassRegistry() = default;
-  /// Registers \p PassPtr and takes ownership.
-  Pass &registerPass(std::unique_ptr<Pass> &&PassPtr) {
-    auto &PassRef = *PassPtr.get();
-    NameToPassMap[PassRef.getName()] = &PassRef;
-    Passes.push_back(std::move(PassPtr));
-    return PassRef;
-  }
-  /// \Returns the pass with name \p Name, or null if not registered.
-  Pass *getPassByName(StringRef Name) const {
-    auto It = NameToPassMap.find(Name);
-    return It != NameToPassMap.end() ? It->second : nullptr;
-  }
-  /// Creates a pass pipeline and returns the first pass manager.
-  FunctionPassManager &parseAndCreatePassPipeline(StringRef Pipeline);
-
-#ifndef NDEBUG
-  void print(raw_ostream &OS) const {
-    for (const auto &PassPtr : Passes)
-      OS << PassPtr->getName() << "\n";
-  }
-  LLVM_DUMP_METHOD void dump() const;
-#endif
-};
-
 } // 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 a2108f07c28e50..02abdf0a1ef0df 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h
@@ -15,18 +15,24 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/SandboxIR/Constant.h"
 #include "llvm/SandboxIR/Pass.h"
+#include "llvm/SandboxIR/PassManager.h"
 #include "llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h"
 
 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);
 
+  // The PM containing the pipeline of region passes.
+  RegionPassManager RPM;
+
 public:
-  BottomUpVec() : FunctionPass("bottom-up-vec") {}
+  BottomUpVec();
   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..b7cb418c00326a 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.h
@@ -8,7 +8,10 @@
 #ifndef LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_SANDBOXVECTORIZER_H
 #define LLVM_TRANSFORMS_VECTORIZE_SANDBOXVECTORIZER_SANDBOXVECTORIZER_H
 
+#include <memory>
+
 #include "llvm/IR/PassManager.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h"
 
 namespace llvm {
 
@@ -17,10 +20,22 @@ class TargetTransformInfo;
 class SandboxVectorizerPass : public PassInfoMixin<SandboxVectorizerPass> {
   TargetTransformInfo *TTI = nullptr;
 
-public:
-  PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
+  // The main vectorizer pass.
+  sandboxir::BottomUpVec BottomUpVecPass;
 
   bool runImpl(Function &F);
+
+public:
+  // Make sure the constructors/destructors are out-of-line. This works around a
+  // problem with -DBUILD_SHARED_LIBS=on where components that depend on the
+  // Vectorizer component can't find the vtable for classes like
+  // sandboxir::Pass. This way we don't have to make LLVMPasses add a direct
+  // dependency on SandboxIR.
+  SandboxVectorizerPass();
+  SandboxVectorizerPass(SandboxVectorizerPass &&);
+  ~SandboxVectorizerPass();
+
+  PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
 };
 
 } // namespace llvm
diff --git a/llvm/lib/SandboxIR/PassManager.cpp b/llvm/lib/SandboxIR/PassManager.cpp
index 95bc5e56bb3ec9..3a1cfa1d367a2a 100644
--- a/llvm/lib/SandboxIR/PassManager.cpp
+++ b/llvm/lib/SandboxIR/PassManager.cpp
@@ -8,11 +8,11 @@
 
 #include "llvm/SandboxIR/PassManager.h"
 
-using namespace llvm::sandboxir;
+namespace llvm::sandboxir {
 
 bool FunctionPassManager::runOnFunction(Function &F) {
   bool Change = false;
-  for (FunctionPass *Pass : Passes) {
+  for (auto &Pass : Passes) {
     Change |= Pass->runOnFunction(F);
     // TODO: run the verifier.
   }
@@ -22,7 +22,7 @@ bool FunctionPassManager::runOnFunction(Function &F) {
 
 bool RegionPassManager::runOnRegion(Region &R) {
   bool Change = false;
-  for (RegionPass *Pass : Passes) {
+  for (auto &Pass : Passes) {
     Change |= Pass->runOnRegion(R);
     // TODO: run the verifier.
   }
@@ -30,40 +30,4 @@ bool RegionPassManager::runOnRegion(Region &R) {
   return Change;
 }
 
-FunctionPassManager &
-PassRegistry::parseAndCreatePassPipeline(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;
-  // Start with a FunctionPassManager.
-  auto &InitialPM = static_cast<FunctionPassManager &>(
-      registerPass(std::make_unique<FunctionPassManager>("init-fpm")));
-
-  for (auto [Idx, C] : enumerate(PipelineStr)) {
-    // Keep moving Idx until we find the end of the pass name.
-    bool FoundDelim = C == EndToken || C == 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 = getPassByName(PassName);
-    if (Pass == nullptr) {
-      errs() << "Pass '" << PassName << "' not registered!\n";
-      exit(1);
-    }
-    // TODO: This is safe for now, but would require proper upcasting once we
-    // add more Pass sub-classes.
-    InitialPM.addPass(static_cast<FunctionPass *>(Pass));
-  }
-  return InitialPM;
-}
-#ifndef NDEBUG
-void PassRegistry::dump() const {
-  print(dbgs());
-  dbgs() << "\n";
-}
-#endif // NDEBUG
+} // namespace llvm::sandboxir
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
index c59abd09d43629..77198f932a3ec0 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
@@ -10,10 +10,41 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/SandboxIR/Function.h"
 #include "llvm/SandboxIR/Instruction.h"
-
-using namespace llvm::sandboxir;
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/NullPass.h"
 
 namespace llvm::sandboxir {
+
+static cl::opt<bool>
+    PrintPassPipeline("sbvec-print-pass-pipeline", cl::init(false), cl::Hidden,
+                      cl::desc("Prints the pass pipeline and returns."));
+
+/// A magic string for the default pass pipeline.
+static const char *DefaultPipelineMagicStr = "*";
+
+static cl::opt<std::string> UserDefinedPassPipeline(
+    "sbvec-passes", cl::init(DefaultPipelineMagicStr), cl::Hidden,
+    cl::desc("Comma-separated list of vectorizer passes. If not set "
+             "we run the predefined pipeline."));
+
+static std::unique_ptr<RegionPass> createRegionPass(StringRef Name) {
+#define REGION_PASS(NAME, CREATE_PASS)                                         \
+  if (Name == NAME)                                                            \
+    return std::make_unique<decltype(CREATE_PASS)>(CREATE_PASS);
+#include "PassRegistry.def"
+  return nullptr;
+}
+
+BottomUpVec::BottomUpVec() : FunctionPass("bottom-up-vec"), RPM("rpm") {
+  // Create a pipeline to be run on each Region created by BottomUpVec.
+  if (UserDefinedPassPipeline == DefaultPipelineMagicStr) {
+    // TODO: Add default passes to RPM.
+  } else {
+    // Create the user-defined pipeline.
+    RPM.setPassPipeline(UserDefinedPassPipeline, createRegionPass);
+  }
+}
+
 // TODO: This is a temporary function that returns some seeds.
 //       Replace this with SeedCollector's function when it lands.
 static llvm::SmallVector<Value *, 4> collectSeeds(BasicBlock &BB) {
@@ -34,8 +65,6 @@ static SmallVector<Value *, 4> getOperand(ArrayRef<Value *> Bndl,
   return Operands;
 }
 
-} // namespace llvm::sandboxir
-
 void BottomUpVec::vectorizeRec(ArrayRef<Value *> Bndl) {
   auto LegalityRes = Legality.canVectorize(Bndl);
   switch (LegalityRes.getSubclassID()) {
@@ -53,14 +82,23 @@ void BottomUpVec::vectorizeRec(ArrayRef<Value *> Bndl) {
 void BottomUpVec::tryVectorize(ArrayRef<Value *> Bndl) { vectorizeRec(Bndl); }
 
 bool BottomUpVec::runOnFunction(Function &F) {
+  if (PrintPassPipeline) {
+    RPM.printPipeline(outs());
+    return false;
+  }
+
   Change = false;
   // TODO: Start from innermost BBs first
   for (auto &BB : 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);
   }
   return Change;
 }
+
+} // namespace llvm::sandboxir
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/PassRegistry.def b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/PassRegistry.def
new file mode 100644
index 00000000000000..bbb0dcba1ea516
--- /dev/null
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/PassRegistry.def
@@ -0,0 +1,22 @@
+//===- PassRegistry.def - Registry of passes --------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file is used as the registry of sub-passes that are part of the
+// SandboxVectorizer pass.
+//
+//===----------------------------------------------------------------------===//
+
+// NOTE: NO INCLUDE GUARD DESIRED!
+
+#ifndef REGION_PASS
+#define REGION_PASS(NAME, CREATE_PASS)
+#endif
+
+REGION_PASS("null", NullPass())
+
+#undef REGION_PASS
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp
index 80afcb499a2c22..ba4899cc624e99 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.cpp
@@ -9,8 +9,6 @@
 #include "llvm/Transforms/Vectorize/SandboxVectorizer/SandboxVectorizer.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/SandboxIR/Constant.h"
-#include "llvm/SandboxIR/PassManager.h"
-#include "llvm/Support/CommandLine.h"
 #include "llvm/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.h"
 
 using namespace llvm;
@@ -18,17 +16,12 @@ using namespace llvm;
 #define SV_NAME "sandbox-vectorizer"
 #define DEBUG_TYPE SV_NAME
 
-cl::opt<bool>
-    PrintPassPipeline("sbvec-print-pass-pipeline", cl::init(false), cl::Hidden,
-                      cl::desc("Prints the pass pipeline and returns."));
+SandboxVectorizerPass::SandboxVectorizerPass() = default;
 
-/// A magic string for the default pass pipeline.
-const char *DefaultPipelineMagicStr = "*";
+SandboxVectorizerPass::SandboxVectorizerPass(SandboxVectorizerPass &&) =
+    default;
 
-cl::opt<std::string> UserDefinedPassPipeline(
-    "sbvec-passes", cl::init(DefaultPipelineMagicStr), cl::Hidden,
-    cl::desc("Comma-separated list of vectorizer passes. If not set "
-             "we run the predefined pipeline."));
+SandboxVectorizerPass::~SandboxVectorizerPass() = default;
 
 PreservedAnalyses SandboxVectorizerPass::run(Function &F,
                                              FunctionAnalysisManager &AM) {
@@ -56,31 +49,8 @@ bool SandboxVectorizerPass::runImpl(Function &LLVMF) {
     return false;
   }
 
+  // Create SandboxIR for LLVMF and run BottomUpVec on it.
   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());
-    return false;
-  }
-
-  // Run the pass pipeline.
-  bool Change = PM->runOnFunction(F);
-  return Change;
+  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
 }
diff --git a/llvm/unittests/SandboxIR/PassTest.cpp b/llvm/unittests/SandboxIR/PassTest.cpp
index b380ae9fd475ab..ae7284ecf2deb9 100644
--- a/llvm/unittests/SandboxIR/PassTest.cpp
+++ b/llvm/unittests/SandboxIR/PassTest.cpp
@@ -180,12 +180,10 @@ define void @foo() {
   };
   unsigned BBCnt1 = 0;
   unsigned BBCnt2 = 0;
-  TestPass1 TPass1(BBCnt1);
-  TestPass2 TPass2(BBCnt2);
 
   FunctionPassManager FPM("test-fpm");
-  FPM.addPass(&TPass1);
-  FPM.addPass(&TPass2);
+  FPM.addPass(std::make_unique<TestPass1>(BBCnt1));
+  FPM.addPass(std::make_unique<TestPass2>(BBCnt2));
   // Check runOnFunction().
   FPM.runOnFunction(*F);
   EXPECT_EQ(BBCnt1, 1u);
@@ -238,12 +236,10 @@ define i8 @foo(i8 %v0, i8 %v1) {
   };
   unsigned InstCount1 = 0;
   unsigned InstCount2 = 0;
-  TestPass1 TPass1(InstCount1);
-  TestPass2 TPass2(InstCount2);
 
   RegionPassManager RPM("test-rpm");
-  RPM.addPass(&TPass1);
-  RPM.addPass(&TPass2);
+  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);
@@ -260,62 +256,60 @@ define i8 @foo(i8 %v0, i8 %v1) {
 #endif // NDEBUG
 }
 
-TEST_F(PassTest, PassRegistry) {
-  class TestPass1 final : public FunctionPass {
-  public:
-    TestPass1() : FunctionPass("test-pass1") {}
-    bool runOnFunction(Function &F) final { return false; }
-  };
-  class TestPass2 final : public FunctionPass {
-  public:
-    TestPass2() : FunctionPass("test-pass2") {}
-    bool runOnFuncti...
[truncated]

@slackito slackito merged commit 756ec99 into llvm:main Oct 10, 2024
12 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Oct 10, 2024

LLVM Buildbot has detected a new failure on builder openmp-offload-amdgpu-runtime running on omp-vega20-0 while building llvm,utils at step 7 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/7954

Here is the relevant piece of the build log for the reference
Step 7 (Add check check-offload) failure: test (failure)
...
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug53727.cpp (870 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug47654.cpp (871 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/test_libc.cpp (872 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/wtime.c (873 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu :: offloading/bug49021.cpp (874 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu :: offloading/std_complex_arithmetic.cpp (875 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/complex_reduction.cpp (876 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug49021.cpp (877 of 879)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/std_complex_arithmetic.cpp (878 of 879)
TIMEOUT: libomptarget :: amdgcn-amd-amdhsa :: offloading/parallel_offloading_map.cpp (879 of 879)
******************** TEST 'libomptarget :: amdgcn-amd-amdhsa :: offloading/parallel_offloading_map.cpp' FAILED ********************
Exit Code: -9
Timeout: Reached timeout of 100 seconds

Command Output (stdout):
--
# RUN: at line 1
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang++ -fopenmp    -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src  -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib  -fopenmp-targets=amdgcn-amd-amdhsa /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/offloading/parallel_offloading_map.cpp -o /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/offloading/Output/parallel_offloading_map.cpp.tmp /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a && /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/offloading/Output/parallel_offloading_map.cpp.tmp | /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/offloading/parallel_offloading_map.cpp
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang++ -fopenmp -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -fopenmp-targets=amdgcn-amd-amdhsa /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/offloading/parallel_offloading_map.cpp -o /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/offloading/Output/parallel_offloading_map.cpp.tmp /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a
# note: command had no output on stdout or stderr
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/offloading/Output/parallel_offloading_map.cpp.tmp
# note: command had no output on stdout or stderr
# error: command failed with exit status: -9
# error: command reached timeout: True
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/offloading/parallel_offloading_map.cpp
# note: command had no output on stdout or stderr
# error: command failed with exit status: -9
# error: command reached timeout: True

--

********************
Slowest Tests:
--------------------------------------------------------------------------
100.05s: libomptarget :: amdgcn-amd-amdhsa :: offloading/parallel_offloading_map.cpp
17.28s: libomptarget :: amdgcn-amd-amdhsa :: offloading/bug49021.cpp
13.30s: libomptarget :: amdgcn-amd-amdhsa :: offloading/parallel_target_teams_reduction_min.cpp
12.34s: libomptarget :: amdgcn-amd-amdhsa :: offloading/parallel_target_teams_reduction_max.cpp
11.57s: libomptarget :: amdgcn-amd-amdhsa :: offloading/complex_reduction.cpp
10.32s: libomptarget :: amdgcn-amd-amdhsa :: jit/empty_kernel_lvl2.c
9.31s: libomptarget :: x86_64-unknown-linux-gnu :: offloading/bug49021.cpp
7.80s: libomptarget :: x86_64-unknown-linux-gnu :: offloading/std_complex_arithmetic.cpp
7.45s: libomptarget :: amdgcn-amd-amdhsa :: offloading/barrier_fence.c
7.33s: libomptarget :: amdgcn-amd-amdhsa :: offloading/ompx_saxpy_mixed.c
6.87s: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug49021.cpp
6.62s: libomptarget :: x86_64-unknown-linux-gnu :: offloading/complex_reduction.cpp
6.35s: libomptarget :: amdgcn-amd-amdhsa :: offloading/parallel_target_teams_reduction.cpp
5.51s: libomptarget :: amdgcn-amd-amdhsa :: offloading/default_thread_limit.c
5.21s: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/std_complex_arithmetic.cpp

@slackito slackito deleted the region branch October 10, 2024 16:44
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bazel "Peripheral" support tier build system: utils/bazel llvm:transforms vectorizers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants