Use an Identifier instead of an OperationName internally for OpPassManager identification (NFC)

[mehdi_amini]

Summary

This allows to defers the check for traits to the execution instead of forcing it on the pipeline creation.
In particular, this is making our pipeline creation tolerant to dialects not being loaded in the context yet.

[mehdi_amini]

mlir/lib/Pass/Pass.cpp

342

Check is delayed here now FYI

[GMNGeoffrey]

Thanks!

mlir/lib/Pass/Pass.cpp

342

I think the distinction in the error message between "not registered" and "not IsolatedFromAbove" was useful and should be preserved. I predict someone will otherwise spend a while trying to figure out why it doesn't like their operation that is marked "IsolatedFromAbove"

500–501

While you're here, fix this typo?

[rriddle]

mlir/include/mlir/Pass/PassInstrumentation.h

17–18

These should be sorted.

46

These can be changed to value-typed now.

mlir/include/mlir/Pass/PassManager.h

31–32

These should be sorted.

73

This should be value-typed.

96

Same here.

mlir/lib/Pass/Pass.cpp

338

Return the error directly, it converts to failure

500–501

It wasn't a typo, iff means if and only if. It's a logical conjunction.

mlir/unittests/Pass/PassManagerTest.cpp

79

nit: Remove the double public and just make this a struct.

86

The explicit StringRef seems unnecessary here.

88

Virtual here is unnecessary.

[GMNGeoffrey]

mlir/lib/Pass/Pass.cpp

500–501

That wasn't the typo I was referring to. I meant "the any" :-D

[rriddle]

mlir/lib/Pass/Pass.cpp

500–501

Nice!

[mehdi_amini]

mlir/lib/Pass/Pass.cpp

342

Sure, I just added it back.
Note that it is hard to get a non-registered op at this point (you'd need to programmatically build the pipeline *and* pass --allow-unregistered-dialects I think)
The previous error was needed because it would have caught misconfigurations.

This is some check that we're losing by the way: with the dialects registered "late" we would have to do more work to catch this.

500–501

Since the upgrade comments can be made on a specific subset of a line, if you hover over @GMNGeoffrey 's comment you see the highlighted section :)

mlir/unittests/Pass/PassManagerTest.cpp

86

no known conversion from 'const char [11]' to 'Optional<llvm::StringRef>

Any alternative?

[rriddle]

mlir/unittests/Pass/PassManagerTest.cpp

86

Ah right, forgot about the Optional. Keeping it is fine.

[mehdi_amini]

Address comments

[mehdi_amini]

Remove forward declaration after adding header include

[rriddle]

mlir/include/mlir/Pass/PassInstrumentation.h

12

Why do you need the include in this file?

46

nit: Drop the const on all of these.

[rriddle]

mlir/unittests/Pass/PassManagerTest.cpp

81

Is this actually necessary to provide for this test?

119

Why the change between expect and assert here?

[mehdi_amini]

mlir/include/mlir/Pass/PassInstrumentation.h

12

ERROR: mlir/include/mlir/Pass/PassInstrumentation.h:46:51 variable has incomplete type 'const mlir::Identifier'
  virtual void runBeforePipeline(const Identifier name,

mlir/unittests/Pass/PassManagerTest.cpp

81

It's not, I'll remove.

119

If I don't assert, the test continues and it'll crash on the next line.

[mehdi_amini]

Rebase on D86994, reduce the amount of Identifier::get() calls.

[mehdi_amini]

Rebase

mlir/include/mlir/Pass/PassInstrumentation.h

 //===- PassInstrumentation.h ------------------------------------*- 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef MLIR_PASS_PASSINSTRUMENTATION_H_
 #define MLIR_PASS_PASSINSTRUMENTATION_H_
 
+#include "mlir/IR/Identifier.h"
 #include "mlir/Support/LLVM.h"
 #include "mlir/Support/TypeID.h"
 
 namespace mlir {
 class Operation;
-class OperationName;
 class Pass;
 
 namespace detail {
 struct PassInstrumentorImpl;
 } // end namespace detail
 
 /// PassInstrumentation provides several entry points into the pass manager
 /// infrastructure. Instrumentations should be added directly to a PassManager
 /// before running a pipeline.
struct PipelineParentInfo {
     Pass *parentPass;
   };
 
   virtual ~PassInstrumentation() = 0;
 
   /// A callback to run before a pass pipeline is executed. This function takes
   /// the name of the operation type being operated on, and information related
   /// to the parent that spawned this pipeline.
-  virtual void runBeforePipeline(const OperationName &name,
+  virtual void runBeforePipeline(Identifier name,
                                  const PipelineParentInfo &parentInfo) {}
 
   /// A callback to run after a pass pipeline has executed. This function takes
   /// the name of the operation type being operated on, and information related
   /// to the parent that spawned this pipeline.
-  virtual void runAfterPipeline(const OperationName &name,
+  virtual void runAfterPipeline(Identifier name,
                                 const PipelineParentInfo &parentInfo) {}
 
   /// A callback to run before a pass is executed. This function takes a pointer
   /// to the pass to be executed, as well as the current operation being
   /// operated on.
   virtual void runBeforePass(Pass *pass, Operation *op) {}
 
   /// A callback to run after a pass is successfully executed. This function

 public:
   PassInstrumentor();
   PassInstrumentor(PassInstrumentor &&) = delete;
   PassInstrumentor(const PassInstrumentor &) = delete;
   ~PassInstrumentor();
 
   /// See PassInstrumentation::runBeforePipeline for details.
   void
-  runBeforePipeline(const OperationName &name,
+  runBeforePipeline(Identifier name,
                     const PassInstrumentation::PipelineParentInfo &parentInfo);
 
   /// See PassInstrumentation::runAfterPipeline for details.
   void
-  runAfterPipeline(const OperationName &name,
+  runAfterPipeline(Identifier name,
                    const PassInstrumentation::PipelineParentInfo &parentInfo);
 
   /// See PassInstrumentation::runBeforePass for details.
   void runBeforePass(Pass *pass, Operation *op);
 
   /// See PassInstrumentation::runAfterPass for details.
   void runAfterPass(Pass *pass, Operation *op);
 

mlir/include/mlir/Pass/PassManager.h

 #include <vector>
 
 namespace llvm {
 class Any;
 } // end namespace llvm
 
 namespace mlir {
 class AnalysisManager;
+class Identifier;
 class MLIRContext;
 class ModuleOp;
-class OperationName;
 class Operation;
 class Pass;
 class PassInstrumentation;
 class PassInstrumentor;
 
 namespace detail {
 struct OpPassManagerImpl;
 } // end namespace detail
 
 //===----------------------------------------------------------------------===//
 // OpPassManager
 //===----------------------------------------------------------------------===//
 
 /// This class represents a pass manager that runs passes on a specific
 /// operation type. This class is not constructed directly, but nested within
 /// other OpPassManagers or the top-level PassManager.
 class OpPassManager {
 public:
-  OpPassManager(OperationName name, bool verifyPasses);
+  OpPassManager(Identifier name, MLIRContext *context, bool verifyPasses);
   OpPassManager(OpPassManager &&rhs);
   OpPassManager(const OpPassManager &rhs);
   ~OpPassManager();
   OpPassManager &operator=(const OpPassManager &rhs);
 
   /// Iterator over the passes in this pass manager.
   using pass_iterator =
       llvm::pointee_iterator<MutableArrayRef<std::unique_ptr<Pass>>::iterator>;
   pass_iterator begin();
   pass_iterator end();
   iterator_range<pass_iterator> getPasses() { return {begin(), end()}; }
 
   using const_pass_iterator =
       llvm::pointee_iterator<ArrayRef<std::unique_ptr<Pass>>::const_iterator>;
   const_pass_iterator begin() const;
   const_pass_iterator end() const;
   iterator_range<const_pass_iterator> getPasses() const {
     return {begin(), end()};
   }
 
   /// Nest a new operation pass manager for the given operation kind under this
   /// pass manager.
-  OpPassManager &nest(const OperationName &nestedName);
+  OpPassManager &nest(Identifier nestedName);
   OpPassManager &nest(StringRef nestedName);
   template <typename OpT> OpPassManager &nest() {
-    return nest(OpT::getOperationName());
+    return nest(Identifier::get(OpT::getOperationName(), getContext()));
   }
 
   /// Add the given pass to this pass manager. If this pass has a concrete
   /// operation type, it must be the same type as this pass manager.
   void addPass(std::unique_ptr<Pass> pass);
 
   /// Add the given pass to a nested pass manager for the given operation kind
   /// `OpT`.
   template <typename OpT> void addNestedPass(std::unique_ptr<Pass> pass) {
     nest<OpT>().addPass(std::move(pass));
   }
 
   /// Returns the number of passes held by this manager.
   size_t size() const;
 
   /// Return an instance of the context.
   MLIRContext *getContext() const;
 
   /// Return the operation name that this pass manager operates on.
-  const OperationName &getOpName() const;
+  Identifier getOpName() const;
 
   /// Returns the internal implementation instance.
   detail::OpPassManagerImpl &getImpl();
 
   /// Prints out the passes of the pass manager as the textual representation
   /// of pipelines.
   /// Note: The quality of the string representation depends entirely on the
   /// the correctness of per-pass overrides of Pass::printAsTextualPipeline.

mlir/lib/Pass/Pass.cpp

 
 //===----------------------------------------------------------------------===//
 // OpPassManagerImpl
 //===----------------------------------------------------------------------===//
 
 namespace mlir {
 namespace detail {
 struct OpPassManagerImpl {
-  OpPassManagerImpl(OperationName name, bool verifyPasses)
-      : name(name), verifyPasses(verifyPasses) {}
+  OpPassManagerImpl(Identifier name, MLIRContext *ctx, bool verifyPasses)
+      : name(name), context(ctx), verifyPasses(verifyPasses) {}
 
   /// Merge the passes of this pass manager into the one provided.
   void mergeInto(OpPassManagerImpl &rhs);
 
   /// Nest a new operation pass manager for the given operation kind under this
   /// pass manager.
-  OpPassManager &nest(const OperationName &nestedName);
+  OpPassManager &nest(Identifier nestedName);
   OpPassManager &nest(StringRef nestedName) {
-    return nest(OperationName(nestedName, getContext()));
+    return nest(Identifier::get(nestedName, getContext()));
   }
 
   /// Add the given pass to this pass manager. If this pass has a concrete
   /// operation type, it must be the same type as this pass manager.
   void addPass(std::unique_ptr<Pass> pass);
 
   /// Coalesce adjacent AdaptorPasses into one large adaptor. This runs
   /// recursively through the pipeline graph.
   void coalesceAdjacentAdaptorPasses();
 
   /// Split all of AdaptorPasses such that each adaptor only contains one leaf
   /// pass.
   void splitAdaptorPasses();
 
   /// Return an instance of the context.
-  MLIRContext *getContext() const {
-    return name.getAbstractOperation()->dialect.getContext();
-  }
+  MLIRContext *getContext() const { return context; }
 
   /// The name of the operation that passes of this pass manager operate on.
-  OperationName name;
+  Identifier name;
+
+  /// The current context for this pass manager
+  MLIRContext *context;
 
   /// Flag that specifies if the IR should be verified after each pass has run.
   bool verifyPasses : 1;
 
   /// The set of passes to run as part of this pass manager.
   std::vector<std::unique_ptr<Pass>> passes;
 };
 } // end namespace detail
 } // end namespace mlir
 
 void OpPassManagerImpl::mergeInto(OpPassManagerImpl &rhs) {
   assert(name == rhs.name && "merging unrelated pass managers");
   for (auto &pass : passes)
     rhs.passes.push_back(std::move(pass));
   passes.clear();
 }
 
-OpPassManager &OpPassManagerImpl::nest(const OperationName &nestedName) {
-  OpPassManager nested(nestedName, verifyPasses);
+OpPassManager &OpPassManagerImpl::nest(Identifier nestedName) {
+  OpPassManager nested(nestedName, getContext(), verifyPasses);
   auto *adaptor = new OpToOpPassAdaptor(std::move(nested));
   addPass(std::unique_ptr<Pass>(adaptor));
   return adaptor->getPassManagers().front();
 }
 
 void OpPassManagerImpl::addPass(std::unique_ptr<Pass> pass) {
   // If this pass runs on a different operation than this pass manager, then
   // implicitly nest a pass manager for this operation.
   auto passOpName = pass->getOpName();
-  if (passOpName && passOpName != name.getStringRef())
+  if (passOpName && passOpName != name.strref())
     return nest(*passOpName).addPass(std::move(pass));
 
   passes.emplace_back(std::move(pass));
   if (verifyPasses)
     passes.emplace_back(std::make_unique<VerifierPass>());
 }
 
 void OpPassManagerImpl::coalesceAdjacentAdaptorPasses() {
for (std::unique_ptr<Pass> &pass : oldPasses) {
     }
   }
 }
 
 //===----------------------------------------------------------------------===//
 // OpPassManager
 //===----------------------------------------------------------------------===//
 
-OpPassManager::OpPassManager(OperationName name, bool verifyPasses)
-    : impl(new OpPassManagerImpl(name, verifyPasses)) {
-  assert(name.getAbstractOperation() &&
-         "OpPassManager can only operate on registered operations");
-  assert(name.getAbstractOperation()->hasProperty(
-             OperationProperty::IsolatedFromAbove) &&
-         "OpPassManager only supports operating on operations marked as "
-         "'IsolatedFromAbove'");
-}
+OpPassManager::OpPassManager(Identifier name, MLIRContext *context,
+                             bool verifyPasses)
+    : impl(new OpPassManagerImpl(name, context, verifyPasses)) {}
 OpPassManager::OpPassManager(OpPassManager &&rhs) : impl(std::move(rhs.impl)) {}
 OpPassManager::OpPassManager(const OpPassManager &rhs) { *this = rhs; }
 OpPassManager &OpPassManager::operator=(const OpPassManager &rhs) {
-  impl.reset(new OpPassManagerImpl(rhs.impl->name, rhs.impl->verifyPasses));
+  impl.reset(new OpPassManagerImpl(rhs.impl->name, rhs.impl->getContext(),
+                                   rhs.impl->verifyPasses));
   for (auto &pass : rhs.impl->passes)
     impl->passes.emplace_back(pass->clone());
   return *this;
 }
 
 OpPassManager::~OpPassManager() {}
 
 OpPassManager::pass_iterator OpPassManager::begin() {
   return MutableArrayRef<std::unique_ptr<Pass>>{impl->passes}.begin();
 }
 OpPassManager::pass_iterator OpPassManager::end() {
   return MutableArrayRef<std::unique_ptr<Pass>>{impl->passes}.end();
 }
 
 OpPassManager::const_pass_iterator OpPassManager::begin() const {
   return ArrayRef<std::unique_ptr<Pass>>{impl->passes}.begin();
 }
 OpPassManager::const_pass_iterator OpPassManager::end() const {
   return ArrayRef<std::unique_ptr<Pass>>{impl->passes}.end();
 }
 
 /// Nest a new operation pass manager for the given operation kind under this
 /// pass manager.
-OpPassManager &OpPassManager::nest(const OperationName &nestedName) {
+OpPassManager &OpPassManager::nest(Identifier nestedName) {
   return impl->nest(nestedName);
 }
 OpPassManager &OpPassManager::nest(StringRef nestedName) {
   return impl->nest(nestedName);
 }
 
 /// Add the given pass to this pass manager. If this pass has a concrete
 /// operation type, it must be the same type as this pass manager.
 void OpPassManager::addPass(std::unique_ptr<Pass> pass) {
   impl->addPass(std::move(pass));
 }
 
 /// Returns the number of passes held by this manager.
 size_t OpPassManager::size() const { return impl->passes.size(); }
 
 /// Returns the internal implementation instance.
 OpPassManagerImpl &OpPassManager::getImpl() { return *impl; }
 
 /// Return an instance of the context.
 MLIRContext *OpPassManager::getContext() const { return impl->getContext(); }
 
 /// Return the operation name that this pass manager operates on.
-const OperationName &OpPassManager::getOpName() const { return impl->name; }
+Identifier OpPassManager::getOpName() const { return impl->name; }
 
 /// Prints out the given passes as the textual representation of a pipeline.
 static void printAsTextualPipeline(ArrayRef<std::unique_ptr<Pass>> passes,
                                    raw_ostream &os) {
   // Filter out passes that are not part of the public pipeline.
   auto filteredPasses =
       llvm::make_filter_range(passes, [](const std::unique_ptr<Pass> &pass) {
         return !isa<VerifierPass>(pass);

 }
 
 //===----------------------------------------------------------------------===//
 // OpToOpPassAdaptor
 //===----------------------------------------------------------------------===//
 
 LogicalResult OpToOpPassAdaptor::run(Pass *pass, Operation *op,
                                      AnalysisManager am) {
+  if (!op->getName().getAbstractOperation())
+    return op->emitOpError()
+           << "trying to schedule a pass on an unregistered operation";
+  if (!op->getName().getAbstractOperation()->hasProperty(
+          OperationProperty::IsolatedFromAbove))
+    return op->emitOpError() << "trying to schedule a pass on an operation not "
+                                "marked as 'IsolatedFromAbove'";
+
   pass->passState.emplace(op, am);
 
   // Instrument before the pass has run.
   PassInstrumentor *pi = am.getPassInstrumentor();
   if (pi)
     pi->runBeforePass(pass, op);
 
   // Invoke the virtual runOnOperation method.
if (failed(run(&pass, op, am)))
       return failure();
 
   return success();
 }
 
 /// Find an operation pass manager that can operate on an operation of the given
 /// type, or nullptr if one does not exist.
 static OpPassManager *findPassManagerFor(MutableArrayRef<OpPassManager> mgrs,
-                                         const OperationName &name) {
+                                         Identifier name) {
   auto it = llvm::find_if(
       mgrs, [&](OpPassManager &mgr) { return mgr.getOpName() == name; });
   return it == mgrs.end() ? nullptr : &*it;
 }
 
 OpToOpPassAdaptor::OpToOpPassAdaptor(OpPassManager &&mgr) {
   mgrs.emplace_back(std::move(mgr));
 }
if (auto *existingPM = findPassManagerFor(rhs.mgrs, pm.getOpName())) {
       rhs.mgrs.emplace_back(std::move(pm));
     }
   }
   mgrs.clear();
 
   // After coalescing, sort the pass managers within rhs by name.
   llvm::array_pod_sort(rhs.mgrs.begin(), rhs.mgrs.end(),
                        [](const OpPassManager *lhs, const OpPassManager *rhs) {
-                         return lhs->getOpName().getStringRef().compare(
-                             rhs->getOpName().getStringRef());
+                         return lhs->getOpName().strref().compare(
+                             rhs->getOpName().strref());
                        });
 }
 
 /// Returns the adaptor pass name.
 std::string OpToOpPassAdaptor::getAdaptorName() {
   std::string name = "Pipeline Collection : [";
   llvm::raw_string_ostream os(name);
   llvm::interleaveComma(getPassManagers(), os, [&](OpPassManager &pm) {

 void OpToOpPassAdaptor::runOnOperationImpl() {
   auto am = getAnalysisManager();
   PassInstrumentation::PipelineParentInfo parentInfo = {llvm::get_threadid(),
                                                         this};
   auto *instrumentor = am.getPassInstrumentor();
   for (auto &region : getOperation()->getRegions()) {
     for (auto &block : region) {
       for (auto &op : block) {
-        auto *mgr = findPassManagerFor(mgrs, op.getName());
+        auto *mgr = findPassManagerFor(mgrs, op.getName().getIdentifier());
         if (!mgr)
           continue;
 
         // Run the held pipeline over the current operation.
         if (instrumentor)
           instrumentor->runBeforePipeline(mgr->getOpName(), parentInfo);
         auto result = runPipeline(mgr->getPasses(), &op, am.nest(&op));
         if (instrumentor)
asyncExecutors.assign(llvm::hardware_concurrency().compute_thread_count(),
                           mgrs);
 
   // Run a prepass over the module to collect the operations to execute over.
   // This ensures that an analysis manager exists for each operation, as well as
   // providing a queue of operations to execute over.
   std::vector<std::pair<Operation *, AnalysisManager>> opAMPairs;
   for (auto &region : getOperation()->getRegions()) {
     for (auto &block : region) {
       for (auto &op : block) {
-        // Add this operation iff the name matches the any of the pass managers.
-        if (findPassManagerFor(mgrs, op.getName()))
+        // Add this operation iff the name matches any of the pass managers.
+        if (findPassManagerFor(mgrs, op.getName().getIdentifier()))
           opAMPairs.emplace_back(&op, am.nest(&op));
       }
     }
   }
 
   // A parallel diagnostic handler that provides deterministic diagnostic
   // ordering.
   ParallelDiagnosticHandler diagHandler(&getContext());
llvm::parallelForEach(
           if (nextID >= e)
             break;
 
           // Set the order id for this thread in the diagnostic handler.
           diagHandler.setOrderIDForThread(nextID);
 
           // Get the pass manager for this operation and execute it.
           auto &it = opAMPairs[nextID];
-          auto *pm = findPassManagerFor(pms, it.first->getName());
+          auto *pm =
+              findPassManagerFor(pms, it.first->getName().getIdentifier());
           assert(pm && "expected valid pass manager for operation");
 
           if (instrumentor)
             instrumentor->runBeforePipeline(pm->getOpName(), parentInfo);
           auto pipelineResult =
               runPipeline(pm->getPasses(), it.first, it.second);
           if (instrumentor)
             instrumentor->runAfterPipeline(pm->getOpName(), parentInfo);
return module.emitError()
          << *crashReproducerFileName << "'";
 }
 
 //===----------------------------------------------------------------------===//
 // PassManager
 //===----------------------------------------------------------------------===//
 
 PassManager::PassManager(MLIRContext *ctx, bool verifyPasses)
-    : OpPassManager(OperationName(ModuleOp::getOperationName(), ctx),
+    : OpPassManager(Identifier::get(ModuleOp::getOperationName(), ctx), ctx,
                     verifyPasses),
       passTiming(false), localReproducer(false) {}
 
 PassManager::~PassManager() {}
 
 /// Run the passes within this manager on the provided module.
 LogicalResult PassManager::run(ModuleOp module) {
   // Before running, make sure to coalesce any adjacent pass adaptors in the

 } // end namespace detail
 } // end namespace mlir
 
 PassInstrumentor::PassInstrumentor() : impl(new PassInstrumentorImpl()) {}
 PassInstrumentor::~PassInstrumentor() {}
 
 /// See PassInstrumentation::runBeforePipeline for details.
 void PassInstrumentor::runBeforePipeline(
-    const OperationName &name,
+    Identifier name,
     const PassInstrumentation::PipelineParentInfo &parentInfo) {
   llvm::sys::SmartScopedLock<true> instrumentationLock(impl->mutex);
   for (auto &instr : impl->instrumentations)
     instr->runBeforePipeline(name, parentInfo);
 }
 
 /// See PassInstrumentation::runAfterPipeline for details.
 void PassInstrumentor::runAfterPipeline(
-    const OperationName &name,
+    Identifier name,
     const PassInstrumentation::PipelineParentInfo &parentInfo) {
   llvm::sys::SmartScopedLock<true> instrumentationLock(impl->mutex);
   for (auto &instr : llvm::reverse(impl->instrumentations))
     instr->runAfterPipeline(name, parentInfo);
 }
 
 /// See PassInstrumentation::runBeforePass for details.
 void PassInstrumentor::runBeforePass(Pass *pass, Operation *op) {

mlir/lib/Pass/PassStatistics.cpp

if (auto *adaptor = dyn_cast<OpToOpPassAdaptor>(pass)) {
       auto mgrs = adaptor->getPassManagers();
       if (mgrs.size() > 1) {
         printPassEntry(os, indent, adaptor->getAdaptorName());
         indent += 2;
       }
 
       // Print each of the children passes.
       for (OpPassManager &mgr : mgrs) {
-        auto name = ("'" + mgr.getOpName().getStringRef() + "' Pipeline").str();
+        auto name = ("'" + mgr.getOpName().strref() + "' Pipeline").str();
         printPassEntry(os, indent, name);
         for (Pass &pass : mgr.getPasses())
           printPass(indent + 2, &pass);
       }
       return;
     }
 
     // Otherwise, we print the statistics for this pass.

mlir/lib/Pass/PassTiming.cpp

 };
 
 struct PassTiming : public PassInstrumentation {
   PassTiming(std::unique_ptr<PassManager::PassTimingConfig> config)
       : config(std::move(config)) {}
   ~PassTiming() override { print(); }
 
   /// Setup the instrumentation hooks.
-  void runBeforePipeline(const OperationName &name,
+  void runBeforePipeline(Identifier name,
                          const PipelineParentInfo &parentInfo) override;
-  void runAfterPipeline(const OperationName &name,
+  void runAfterPipeline(Identifier name,
                         const PipelineParentInfo &parentInfo) override;
   void runBeforePass(Pass *pass, Operation *) override { startPassTimer(pass); }
   void runAfterPass(Pass *pass, Operation *) override;
   void runAfterPassFailed(Pass *pass, Operation *op) override {
     runAfterPass(pass, op);
   }
   void runBeforeAnalysis(StringRef name, TypeID id, Operation *) override {
     startAnalysisTimer(name, id);
struct PassTiming : public PassInstrumentation {
 
   /// A mapping of pipeline timers that need to be merged into the parent
   /// collection. The timers are mapped to the parent info to merge into.
   DenseMap<PipelineParentInfo, SmallVector<Timer::ChildrenMap::value_type, 4>>
       pipelinesToMerge;
 };
 } // end anonymous namespace
 
-void PassTiming::runBeforePipeline(const OperationName &name,
+void PassTiming::runBeforePipeline(Identifier name,
                                    const PipelineParentInfo &parentInfo) {
   // We don't actually want to time the pipelines, they gather their total
   // from their held passes.
   getTimer(name.getAsOpaquePointer(), TimerKind::Pipeline,
-           [&] { return ("'" + name.getStringRef() + "' Pipeline").str(); });
+           [&] { return ("'" + name.strref() + "' Pipeline").str(); });
 }
 
-void PassTiming::runAfterPipeline(const OperationName &name,
+void PassTiming::runAfterPipeline(Identifier name,
                                   const PipelineParentInfo &parentInfo) {
   // Pop the timer for the pipeline.
   auto tid = llvm::get_threadid();
   auto &activeTimers = activeThreadTimers[tid];
   assert(!activeTimers.empty() && "expected active timer");
   activeTimers.pop_back();
 
   // If the current thread is the same as the parent, there is nothing left to

mlir/unittests/Pass/PassManagerTest.cpp

for (FuncOp func : module->getOps<FuncOp>()) {
     EXPECT_TRUE(func.getAttr("isFunc").cast<BoolAttr>().getValue());
 
     bool isSecret = func.getName() == "secret";
     ASSERT_TRUE(func.getAttr("isSecret").isa<BoolAttr>());
     EXPECT_EQ(func.getAttr("isSecret").cast<BoolAttr>().getValue(), isSecret);
   }
 }
 
+namespace {
+struct InvalidPass : Pass {
+  InvalidPass() : Pass(TypeID::get<InvalidPass>(), StringRef("invalid_op")) {}
+  StringRef getName() const override { return "Invalid Pass"; }
+  void runOnOperation() override {}
+
+  /// A clone method to create a copy of this pass.
+  std::unique_ptr<Pass> clonePass() const override {
+    return std::make_unique<InvalidPass>(
+        *static_cast<const InvalidPass *>(this));
+  }
+};
+} // anonymous namespace
+
+TEST(PassManagerTest, InvalidPass) {
+  MLIRContext context;
+
+  // Create a module
+  OwningModuleRef module(ModuleOp::create(UnknownLoc::get(&context)));
+
+  // Add a single "invalid_op" operation
+  OpBuilder builder(&module->getBodyRegion());
+  OperationState state(UnknownLoc::get(&context), "invalid_op");
+  builder.insert(Operation::create(state));
+
+  // Register a diagnostic handler to capture the diagnostic so that we can
+  // check it later.
+  std::unique_ptr<Diagnostic> diagnostic;
+  context.getDiagEngine().registerHandler([&](Diagnostic &diag) {
+    diagnostic.reset(new Diagnostic(std::move(diag)));
+  });
+
+  // Instantiate and run our pass.
+  PassManager pm(&context);
+  pm.addPass(std::make_unique<InvalidPass>());
+  LogicalResult result = pm.run(module.get());
+  EXPECT_TRUE(failed(result));
+  ASSERT_TRUE(diagnostic.get() != nullptr);
+  EXPECT_EQ(
+      diagnostic->str(),
+      "'invalid_op' op trying to schedule a pass on an unregistered operation");
+}
+
 } // end namespace