[MLIR] Factor pass timing out into a dedicated timing manager

[fabianschuiki]

Summary

This factors out the pass timing code into a separate TimingManager that can be plugged into the PassManager from the outside. Users are able to provide their own implementation of this manager, and use it to time additional code paths outside of the pass manager. Also allows for multiple PassManagers to run and contribute to a single timing report.

More specifically, moves most of the existing infrastructure in Pass/PassTiming.cpp into a new Support/Timing.cpp file and adds a public interface in Support/Timing.h. The PassTiming instrumentation becomes a wrapper around the new timing infrastructure which adapts the instrumentation callbacks to the new timers.

This is a concrete suggestion towards https://llvm.discourse.group/t/add-entries-to-pass-timing-from-outside-the-passmanager/3115.

[mehdi_amini]

I support making the timing infrastructure usable and pluggable outside of the pass manager! Thanks for working on this, I looked into it at some point but didn't need it badly enough to finish it.

However I'm not sure about this approach at the moment: it seems that you're trying to keep the PM in charge of all the timing, and using it as a "timing manager" for things that aren't related to pass management.
I would think that it'd be better modeled by extracting the timing manager as a separate entities usable outside of the PM, and have the PM report to "timing manager" (through injection or similar).

[rriddle]

In D100647#2695523, @mehdi_amini wrote:

I support making the timing infrastructure usable and pluggable outside of the pass manager! Thanks for working on this, I looked into it at some point but didn't need it badly enough to finish it.

However I'm not sure about this approach at the moment: it seems that you're trying to keep the PM in charge of all the timing, and using it as a "timing manager" for things that aren't related to pass management.
I would think that it'd be better modeled by extracting the timing manager as a separate entities usable outside of the PM, and have the PM report to "timing manager" (through injection or similar).

+1 to what Mehdi says here. We want to have general infra related to timing (that ideally users can inject their own implementations of), with the pass manager simply hooking into that.

[lattner]

I agree with Mehdi and River. it seems like the passmgr should be a client of the more general shared thing.

I'd also recommend changing the top level flyer:
... Pass execution timing report ... to just ... Execution time report ...

[fabianschuiki]

Thanks for the great feedback! I also had a more general timing manager in mind that we could inject into the PM, but opted for this simpler change to get a discussion rolling. Let me factor out the timing interface into some abstract base class and the current implementation of time keeping into a concrete subclass. That would allow us to default to an MLIR-provided TimingManager class as a convenience (and to not break existing code maybe?), but still allow the user to provide a custom implementation if desired.

[fabianschuiki]

• Moved most of the timing infrastructure in Pass/PassTiming.cpp into a new Support/Timing.cpp file
• Added public interfaces for the timing stuff in Support/Timing.h
• Made the pass manager use the timers provided by the new Timing.h
• Added a new TimingManager that holds the root timer and prints timing reports (similar to the role the PassTiming instrumentation had previously)
• Added TimingManagerOptions and corresponding registerTimingManagerCLOptions and applyTimingManagerCLOptions functions

One problem I ran into when merging async executions of the same pipeline was identifying whether two executed passes were actually "the same" from the user's perspective (i.e. just cloned for the sake of threading). Just using the pass pointer as unique identifier would cause each thread to show up as a separate set of passes. An obvious solution would have been to use pass.getTypeID().getAsOpaquePointer(), but if the user intentionally added the same pass multiple times they would all be collapsed into one. I ended up adding a passPrototype field to Pass itself to track the "pass identity" through cloning inside the PM. (The original pass timing had the same problem and opted for merging the passes in a pipeline by position.)

I'm not sure how far we want to preserve compatibility with respect to CL options. The patch as it is now preserves the old --pass-timing and --pass-timing-display options of the PM, as well as the PM's enableTiming(config) function. If no external timing manager is provided, the PM will just create its own temporary one internally, report there, and print results as before. The new --timing and --timing-display can still be used and if present take precedence over --pass-timing. I'm also happy to throw out the old --pass-timing options, or rename the new ones to --pass-timing instead.

Finally I also added reporting for the "remaining" time not covered by dedicated timers, as suggested by @lattner. It is currently only shown for the top of the hierarchy. It's easy to add for all nested timers that have children, but that adds *a lot* of entries to the pipeline.

---

The output from the CIRCT project's firtool with the new timing manager looks as follows:

===-------------------------------------------------------------------------===
                        ... Execution timing report ...
===-------------------------------------------------------------------------===
  Total Execution Time: 0.0067 seconds

  ----User Time----  ----Wall Time----  ----Name----
    0.0010 ( 13.4%)    0.0010 ( 14.9%)  Parser
    0.0012 ( 15.7%)    0.0012 ( 17.5%)  'firrtl.circuit' Pipeline
    0.0004 (  6.0%)    0.0004 (  6.6%)    LowerFIRRTLTypes
    0.0006 (  7.7%)    0.0006 (  8.5%)    'firrtl.module' Pipeline
    0.0002 (  2.1%)    0.0002 (  2.3%)      CSE
    0.0000 (  0.1%)    0.0000 (  0.1%)        (A) DominanceInfo
    0.0004 (  5.3%)    0.0004 (  6.0%)      Canonicalizer
    0.0014 ( 19.3%)    0.0014 ( 21.5%)  LowerFIRRTLToRTL
    0.0002 (  2.2%)    0.0002 (  2.5%)  RTLMemSimImpl
    0.0011 ( 14.1%)    0.0004 (  5.5%)  'rtl.module' Pipeline
    0.0003 (  4.5%)    0.0002 (  2.3%)    RTLCleanup
    0.0002 (  3.2%)    0.0001 (  1.4%)    CSE
    0.0000 (  0.2%)    0.0000 (  0.1%)      (A) DominanceInfo
    0.0003 (  4.3%)    0.0001 (  1.6%)    Canonicalizer
    0.0005 (  6.5%)    0.0005 (  7.3%)  RTLLegalizeNames
    0.0002 (  2.2%)    0.0002 (  2.4%)    (A) circt::sv::LegalNamesAnalysis
    0.0008 ( 10.6%)    0.0008 ( 11.8%)  Output
    0.0008 ( 11.2%)    0.0008 ( 12.5%)  Rest
    0.0075 (100.0%)    0.0067 (100.0%)  Total

Of these Parser and Output are timers run before and after the PM, and Rest indicates the time that is not covered by the top-level timers.

[rriddle]

mlir/include/mlir/Pass/Pass.h

154 ↗

(On Diff #339144)

You can't rely on this meaning that the pass is a threading sibling with the original pass.

mlir/include/mlir/Support/Timing.h

113 ↗

(On Diff #339144)

nit: Just spell out auto here.

121 ↗

(On Diff #339144)

This display mode looks specific to the current pass manager implementation, I'm not sure this is generally applicable to the other potential timer backends. Couple of related general questions:

• Do we need a general TimingConfig anymore?

This was necessary before because of how the pass manager hid the internal timing instrumentation, but I'm not sure that it is really necessary anymore. How much of it would be shared across different potential timer backends?

• Can you rename TimingManagerBase base to just TimingManager?

I don't think the default implementation should be called TimingManager, it should likely be called something else(not sure the name though). This would establish that TimingManager (as you currently have it) is just an implementation and not the interface.

• Can you add comment blocks? I find these to help separate different concepts within a source file (e.g. https://github.com/llvm/llvm-project/blob/a7f33d720b74a92031958fce0e209794d296393d/mlir/include/mlir/Pass/Pass.h#L310)

• Do you have an idea of what supporting another timing backend would look like?

Not that you have to do the work, but when I've thought about this before ideally I would want to be able to support something like (1) or (2) without needing a new API/interface. Ideally these would all just be implementations that a user can choose from and pass the general infra to use for timer reporting.

(1): https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/TimeProfiler.h
(2): https://github.com/wolfpld/tracy

mlir/lib/Support/Timing.cpp

134 ↗

(On Diff #339144)

The virtual isn't necessary when you have override.

139 ↗

(On Diff #339144)

nit: Drop the else after a return.

171 ↗

(On Diff #339144)

Drop the trivial braces here.

[lattner]

This is looking really great Fabian!

mlir/include/mlir/Pass/PassManager.h

337

Unless this is a widely used typedef that could occur in downstream code, I'd recommend just eliminating the PassTimingConfig name, updating clients to use TimingConfig directly.

mlir/include/mlir/Support/Timing.h

26 ↗

(On Diff #339144)

Please follow clang-tidy's advice here and add a virtual dtor. Also, please declare one method (e.g. the dtor) out of line in a .cpp file to give the vtable a home in a .o file.

161 ↗

(On Diff #339144)

I agree with River's comment that it would be great to eliminate this class.

mlir/lib/Support/Timing.cpp

107 ↗

(On Diff #339144)

As clang tidy suggests please declare this as a class. The difference matters for MSVC

[fabianschuiki]

mlir/include/mlir/Pass/Pass.h

154 ↗

(On Diff #339144)

Renamed this to threadingSibling and moved the tracking of copies from Pass::clone() into the OpPassManager constructor. Since the OpPassManager is only duplicated in OpToOpPassAdaptor::runOnOperationAsyncImpl, that should reliably identify clones that were made for the sake of threading.

[fabianschuiki]

Thanks for the great feedback @rriddle and @lattner! Updated the diff with your suggestions and fixed the check-mlir run.

• @rriddle's question about how easily we can spin up additional implementations for stuff like the LLVM TimeProfiler or tracy got me thinking. The previous design would require you to subclass both TimerBase and TimingManagerBase to hook into the infrastructure. Since you had to hand out TimerBase instances, you had to allocate them somewhere in memory, which was particularly bad if the external timing API you were wrapping was giving you IDs or pointers to its own data structures. This revision now simplifies the design: TimerBase has been replaced by a Timer that is not intended to be subclassed, but packages a pointer to a timing manager and an opaque ID/handle identifying the timer. TimingManagerBase is now called TimingManager and has overridable callback functions for timer start, stop, and nesting which Timer forwards to. That means there is one thing for implementations to subclass (TimingManager), and one thing for library code to interact with (Timer, and by extension TimingScope). Makes things much easier to use. Also, wrappers around external timing libs can just pass around the pointers they get from those libs as opaque handles in Timer.

• Dropped all the TimingConfig business and made the TimingDisplayMode a characteristic of the default timing manager implementation.

• DefaultTimingManager is maybe not the best of names; suggestions are very welcome! Maybe ConsoleTimingManager, BasicTimingManager, SimpleTimingManager?

• The redundancy between the timing options in PassManagerOptions and the new ones in TimingManagerOptions felt awkward. I dropped the timing options from PM altogether in favor of dealing with timing config completely outside of the PM. The pass manager can own a TimingManager and there is now a applyDefaultTimingPassManagerCLOptions() function which generates a dedicated timing manager for the PM with some options in case the user does not want to deal with timing themselves. The intended usage though is through PassManager::enableTiming(TimingManager).

• I renamed the --pass-timing and --pass-timing-display options to --timing and --timing-display, respectively, to reflect that the scope of these has broadened beyond pass timing.

• I updated MlirOptMain and the pass-timing.mlir test to use the new timing infrastructure and options, and to act as an example on how to use them.

[lattner]

This looks really nice Fabian!

[rriddle]

How do you envision this being used deep within the compiler? Should we have a static API that is accessible from anywhere? Should we have something on the context? What are your thoughts here?

Also, please fix the clang tidy issues.

mlir/include/mlir/Support/Timing.h

159 ↗

(On Diff #342412)

nit: Please make bool conversion operators explicit.

[rriddle]

(Apologies for the delay in review, will take a pass over tomorrow)

[fabianschuiki]

Fixed clang-tidy issues and made bool conversion explicit. Sorry about those, having a somewhat hard time to get my local clang-tidy to reproduce the set of upstream checks.

Regarding usage within the compiler: Having to thread TimingScopes through everywhere sounds pretty unergonomic if you just want to time a few things within the compiler. Ideally we would have a per-thread stack of active timing scopes somewhere (Maybe in the context? As a thread-local static?). Pointers to the active TimingScopes would then be kept on that stack, and you could just ask for the currently-active one to nest into. That way you could get at whatever scope within the parent code you're implicitly executing in, without needing to explicitly push scope references down the call chain. Maybe context.getTimingScope(id, nameBuilder) and context.getTimer(id, nameBuilder)?

[mehdi_amini]

In general, I have concerns with any design that would single global data structure for book-keeping. I would look into hooking the storage onto the MLIRContext as needed.

[rriddle]

In D100647#2735576, @fabianschuiki wrote:

Fixed clang-tidy issues and made bool conversion explicit. Sorry about those, having a somewhat hard time to get my local clang-tidy to reproduce the set of upstream checks.

Regarding usage within the compiler: Having to thread TimingScopes through everywhere sounds pretty unergonomic if you just want to time a few things within the compiler. Ideally we would have a per-thread stack of active timing scopes somewhere (Maybe in the context? As a thread-local static?). Pointers to the active TimingScopes would then be kept on that stack, and you could just ask for the currently-active one to nest into. That way you could get at whatever scope within the parent code you're implicitly executing in, without needing to explicitly push scope references down the call chain. Maybe context.getTimingScope(id, nameBuilder) and context.getTimer(id, nameBuilder)?

I would not add timing API to the context itself, but instead hook in a reference to a timing manager. Mehdi touches a bit on the general opinion that MLIR takes about these in terms of static vs non-static, and returning a manager object is inline with the other things hooked into the context. One important piece here though, is that it should generally be zero-cost if there is no timing manager.

mlir/include/mlir/Pass/PassManager.h

392

Why can't this be handled by the timing instrumentation? I don't think we need these fields here.

mlir/include/mlir/Support/Timing.h

16 ↗

(On Diff #342652)

This is an indicator that the dependencies are off. Support/ shouldn't depend on anything in IR/.

332 ↗

(On Diff #342652)

nit: I don't think the llvm:: here is necessary if you include LLVM.h

mlir/lib/Pass/PassTiming.cpp

146

nit: Drop the trivial braces here.

mlir/lib/Support/MlirOptMain.cpp

51 ↗

(On Diff #342652)

I'm not sure about this type of integration. This seems to always enforce timing, which isn't something that I would expect. If we start integrating timing more deeply into the compiler. I would expect it to do nothing unless explicitly enabled.

[fabianschuiki]

I think having a reference to a TimingManager in the context would be great. That would allow us to encode timing enable/disable through the presence/absence of a such a reference (right now it's an explicit field in the TM). For the sake of keeping code that uses the timing API streamlined, we should probably have the getTimingManager() return a NullTimingManager reference that always hands out disabled Timers. Since the latter are just a wrapper around a pointer, and their functions all return immediately if disabled, the runtime cost in code should be equivalent to having manual "is timing enabled" checks.

Do you guys want to tackle the integration into MLIRContext already as part of this diff, or push that off into a separate one?

mlir/include/mlir/Pass/PassManager.h

392

Yeah good point, I'll move that into the timing instrumentation.

mlir/include/mlir/Support/Timing.h

16 ↗

(On Diff #342652)

Good point. Will check for some other/dedicated mechanism to map a string to a opaque pointer identifying the string content. Since we have a TimingManager now, that could just as well go in there.

mlir/lib/Support/MlirOptMain.cpp

51 ↗

(On Diff #342652)

The timing is disabled in the manager upon construction. The call to applyDefaultTimingManagerCLOptions(tm) on the next line enables timing if the options are present on the command line. If the timing remains disabled, getRootScope() below returns a disabled TimingScope that has a fast path to do nothing (just a pointer comparison).

With some integration into MLIRContext and a stack of running timers, you could remove the timing variable below and instead have things like:

DefaultTimingManager tm;
applyDefaultTimingManagerCLOptions(tm);
context->setTimingManager(tm); // or some ownership transfer

auto parserTiming = context->getTimingManager()->addTimingScope("Parser");

parserTiming.stop()

auto outputTiming = context->getTimingManager()->addTimingScope("Output");

[rriddle]

In D100647#2738046, @fabianschuiki wrote:

I think having a reference to a TimingManager in the context would be great. That would allow us to encode timing enable/disable through the presence/absence of a such a reference (right now it's an explicit field in the TM). For the sake of keeping code that uses the timing API streamlined, we should probably have the getTimingManager() return a NullTimingManager reference that always hands out disabled Timers. Since the latter are just a wrapper around a pointer, and their functions all return immediately if disabled, the runtime cost in code should be equivalent to having manual "is timing enabled" checks.

Do you guys want to tackle the integration into MLIRContext already as part of this diff, or push that off into a separate one?

I'm okay with shifting that a follow up, so that it is easier to review the diff.

Can you also draft up some documentation for the timing infra? with examples on users would interact with it, and how it could be used to inject custom timers? I'm fine with that being a follow up as well.

This looks really great! I'm looking forward to being able to use it.

mlir/include/mlir/Support/Timing.h

86–87 ↗

(On Diff #342652)

/// Implementations override this method to provide access to the top-level timer.

This method isn't virtual, what do you mean by the comment above?

mlir/lib/Support/MlirOptMain.cpp

51 ↗

(On Diff #342652)

Ah, I completely missed that. Thanks for pointing it out! I like what you have here.

[fabianschuiki]

mlir/include/mlir/Support/Timing.h

86–87 ↗

(On Diff #342652)

Oops, my bad -- that's a leftover from when rootTimer() below wasn't a thing.

[fabianschuiki]

• Moved the owned timing manager and scope into PassTiming, dropping those fields from PassManager.
• Added a TimingIdentifier to remove the dependency on IR/Identifier.h. This new identifier is a version of Identifier/MLIRContext but stripped-down to cover only the functionality needed by the TM.
• Other cleanup

[rriddle]

Looks good to me with comments resolved.

(Also should clarify when I said docs, I was thinking something in mlir/docs/)

mlir/include/mlir/Support/Timing.h

79 ↗

(On Diff #342966)

demarcate

230 ↗

(On Diff #342966)

Can you change all of the uses of std::function<> && to function_ref? std::function generally ends up allocating memory, which we don't really want to do. Given that we aren't storing the function, we only really need a reference anyways.

402 ↗

(On Diff #342966)

Is this something we could make non-virtual? I think it would be nice if we could avoid an indirect call in the case where timing is disabled.

mlir/lib/Support/Timing.cpp

33 ↗

(On Diff #342966)

This shouldn't be necessary, StringRef is exported in the mlir namespace (see LLVM.h)

542 ↗

(On Diff #342966)

Can you prefix these with mlir-? I'm always hesitant about adding very commonly named things to the global command line namespace.

[fabianschuiki]

mlir/include/mlir/Support/Timing.h

402 ↗

(On Diff #342966)

The way you use the TM at the moment is roughly:

DefaultTimingManager tm;
TimingScope ts = tm.getRootScope();
// all further timing calls go through `ts`

If timing is disabled, ts is marked as diabled through a nullptr in its tm field, which causes all subsequent interaction to short-circuit into no-ops. So in its current implementation, if timing is disabled there is just one virtual function call for the duration of the program -- that should not be visible in practice.

I do agree that it would be nicer if the setup-related interaction with the TM would be non-virtual. Especially if we start to keep per-thread stacks of active timing scopes, that should all be directly accessible. We could promote setEnabled and isEnabled into the TimingManager base class such that it can just never ask for the rootTimer if disabled. Pretty sure that all TM impls can get behind the concept of enabling/disabling.

Do you want to tackle this in this diff, or push that off to a follow-up diff alongside tracking a TM in the context?

mlir/lib/Support/Timing.cpp

542 ↗

(On Diff #342966)

Yeah that's a good idea. --timing did seem awfully brief.

[fabianschuiki]

Applied suggestions by @rriddle.

[rriddle]

mlir/include/mlir/Support/Timing.h

402 ↗

(On Diff #342966)

Followup is alright with me, thanks!

[fabianschuiki]

Awesome, thanks! What are the next steps (sorry, first time Phabricator user)?

[lattner]

git pull --rebase
ninja check-mlir # sanity check
git push

if needed:
git pull --rebase
git push

[fabianschuiki]

Thanks @lattner! Wasn't sure whether the ready-to-land is the green light to push to the repo myself. Done!

[lattner]

Thanks again Fabian. It would also be good to make sure this gets mentioned in MLIR biweekly

[mehdi_amini]

In D100647#2754448, @lattner wrote:

Thanks again Fabian. It would also be good to make sure this gets mentioned in MLIR biweekly

Yes please! :)

(here is the link: https://llvm.discourse.group/t/work-in-progress-next-mlir-news-33rd-edition-5-1-5-14-2021/3417 )

[fabianschuiki]

In D100647#2755459, @mehdi_amini wrote:

In D100647#2754448, @lattner wrote:

Thanks again Fabian. It would also be good to make sure this gets mentioned in MLIR biweekly

Yes please! :)

(here is the link: https://llvm.discourse.group/t/work-in-progress-next-mlir-news-33rd-edition-5-1-5-14-2021/3417 )

Done! :-)

mlir/include/mlir/Pass/PassManager.h

 class Pass;
 class PassInstrumentation;
 class PassInstrumentor;
 
 namespace detail {
 struct OpPassManagerImpl;
 class OpToOpPassAdaptor;
 struct PassExecutionState;
+class PassTiming;
 } // 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
public:
     virtual ~PassTimingConfig();
 
     /// A hook that may be overridden by a derived config to control the
     /// printing. The callback is supplied by the framework and the config is
     /// responsible to call it back with a stream for the output.
     virtual void printTiming(PrintCallbackFn printCallback);
 
     /// Return the `PassDisplayMode` this config was created with.
     PassDisplayMode getDisplayMode() { return displayMode; }
 
   private:
     PassDisplayMode displayMode;
   };
 
   /// Add an instrumentation to time the execution of passes and the computation
   /// of analyses.
   /// Note: Timing should be enabled after all other instrumentations to avoid
   /// any potential "ghost" timing from other instrumentations being
   /// unintentionally included in the timing results.
   void enableTiming(std::unique_ptr<PassTimingConfig> config = nullptr);
 
+  /// A helper to time code paths outside the pass manager.
+  class ExternalTimer {
+  public:
+    virtual ~ExternalTimer() {}
+
+    /// Start the timer.
+    virtual void start() = 0;
+
+    /// Stop the timer.
+    virtual void stop() = 0;
+  };
+
+  /// Get a timer that can be used to include things outside of the pass manager
+  /// in the timing statistics.
+  std::unique_ptr<ExternalTimer> getExternalTimer(Identifier name);
+  std::unique_ptr<ExternalTimer> getExternalTimer(StringRef name) {
+    return getExternalTimer(Identifier::get(name, context));
+  }
+
   /// Prompts the pass manager to print the statistics collected for each of the
   /// held passes after each call to 'run'.
   void
   enableStatistics(PassDisplayMode displayMode = PassDisplayMode::Pipeline);
 
 private:
   /// Dump the statistics of the passes within this pass manager.
   void dumpStatistics();
 
   /// Run the pass manager with crash recover enabled.
   LogicalResult runWithCrashRecovery(Operation *op, AnalysisManager am);
   /// Run the given passes with crash recover enabled.
   LogicalResult
   runWithCrashRecovery(MutableArrayRef<std::unique_ptr<Pass>> passes,
                        Operation *op, AnalysisManager am);
 
   /// Context this PassManager was initialized with.
   MLIRContext *context;
 
   /// Flag that specifies if pass statistics should be dumped.
   Optional<PassDisplayMode> passStatisticsMode;
 
   /// A manager for pass instrumentations.
   std::unique_ptr<PassInstrumentor> instrumentor;
 
   /// An optional factory to use when generating a crash reproducer if valid.
   ReproducerStreamFactory crashReproducerStreamFactory;
 
   /// A hash key used to detect when reinitialization is necessary.
   llvm::hash_code initializationKey;
 
-  /// Flag that specifies if pass timing is enabled.
-  bool passTiming : 1;
+  /// Pointer to the pass timing instrumentation, if it is enabled.
+  detail::PassTiming *passTiming;
 
   /// Flag that specifies if the generated crash reproducer should be local.
   bool localReproducer : 1;
 
   /// A flag that indicates if the IR should be verified in between passes.
   bool verifyPasses : 1;
 };
 

mlir/lib/Pass/Pass.cpp

 //===----------------------------------------------------------------------===//
 // PassManager
 //===----------------------------------------------------------------------===//
 
 PassManager::PassManager(MLIRContext *ctx, Nesting nesting,
                          StringRef operationName)
     : OpPassManager(Identifier::get(operationName, ctx), nesting), context(ctx),
       initializationKey(DenseMapInfo<llvm::hash_code>::getTombstoneKey()),
-      passTiming(false), localReproducer(false), verifyPasses(true) {}
+      passTiming(nullptr), localReproducer(false), verifyPasses(true) {}
 
 PassManager::~PassManager() {}
 
 void PassManager::enableVerifier(bool enabled) { verifyPasses = enabled; }
 
 /// Run the passes within this manager on the provided operation.
 LogicalResult PassManager::run(Operation *op) {
   MLIRContext *context = getContext();

mlir/lib/Pass/PassTiming.cpp

struct Timer {
   /// Merge in the given child timer and id into this timer.
   void mergeChild(ChildrenMap::value_type &&childIt) {
     auto &child = children[childIt.first];
     if (!child)
       child = std::move(childIt.second);
     else
       child->merge(std::move(*childIt.second));
   }
 
   /// Raw timing information.
   std::chrono::time_point<std::chrono::system_clock> startTime;
   std::chrono::nanoseconds wallTime = std::chrono::nanoseconds(0);
   std::chrono::nanoseconds userTime = std::chrono::nanoseconds(0);
 
   /// A map of unique identifiers to child timers.
   ChildrenMap children;
 
   /// A descriptive name for this timer.
   std::string name;
 
   /// The type of timer this instance represents.
   TimerKind kind;
 };
+} // end anonymous namespace
 
+namespace mlir {
+namespace detail {
 struct PassTiming : public PassInstrumentation {
   PassTiming(std::unique_ptr<PassManager::PassTimingConfig> config)
       : config(std::move(config)) {}
   ~PassTiming() override { print(); }
 
   /// Setup the instrumentation hooks.
   void runBeforePipeline(Identifier name,
                          const PipelineParentInfo &parentInfo) override;
struct PassTiming : public PassInstrumentation {
   void print();
 
   /// Start a new timer for the given pass.
   void startPassTimer(Pass *pass);
 
   /// Start a new timer for the given analysis.
   void startAnalysisTimer(StringRef name, TypeID id);
 
+  /// Start a new timer for the given external code path.
+  void startExternalTimer(Identifier name);
+
+  /// Stop an external timer.
+  void stopExternalTimer(Identifier name);
+
   /// Pop the last active timer for the current thread.
   Timer *popLastActiveTimer() {
     auto tid = llvm::get_threadid();
     auto &activeTimers = activeThreadTimers[tid];
     assert(!activeTimers.empty() && "expected active timer");
     return activeTimers.pop_back_val();
   }
 
struct PassTiming : public PassInstrumentation {
   /// The configuration object to use when printing the timing results.
   std::unique_ptr<PassManager::PassTimingConfig> config;
 
   /// 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
+} // namespace detail
+} // namespace mlir
 
 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.strref() + "' Pipeline").str(); });
 }

 
 /// Start a new timer for the given analysis.
 void PassTiming::startAnalysisTimer(StringRef name, TypeID id) {
   Timer *timer = getTimer(id.getAsOpaquePointer(), TimerKind::PassOrAnalysis,
                           [name] { return "(A) " + name.str(); });
   timer->start();
 }
 
+/// Start a new timer for the given external code path.
+void PassTiming::startExternalTimer(Identifier name) {
+  Timer *timer = getTimer(name.getAsOpaquePointer(), TimerKind::PassOrAnalysis,
+                          [name] { return name.str(); });
+  timer->start();
+}
+
 /// Stop a pass timer.
 void PassTiming::runAfterPass(Pass *pass, Operation *) {
   Timer *timer = popLastActiveTimer();
 
   // Check to see if we need to merge in the timing data for the pipelines
   // running on other threads.
   auto toMerge = pipelinesToMerge.find({llvm::get_threadid(), pass});
   if (toMerge != pipelinesToMerge.end()) {
     for (auto &it : toMerge->second)
       timer->mergeChild(std::move(it));
     pipelinesToMerge.erase(toMerge);
   }
 
   timer->stop();
 }
 
 /// Stop a timer.
 void PassTiming::runAfterAnalysis(StringRef, TypeID, Operation *) {
   popLastActiveTimer()->stop();
 }
 
+/// Stop an external timer.
+void PassTiming::stopExternalTimer(Identifier) { popLastActiveTimer()->stop(); }
+
 /// Utility to print the timer heading information.
 static void printTimerHeader(raw_ostream &os, TimeRecord total) {
   os << "===" << std::string(73, '-') << "===\n";
   // Figure out how many spaces to description name.
   unsigned padding = (80 - kPassTimingDescription.size()) / 2;
   os.indent(padding) << kPassTimingDescription << '\n';
   os << "===" << std::string(73, '-') << "===\n";
 

 /// Add an instrumentation to time the execution of passes and the computation
 /// of analyses.
 void PassManager::enableTiming(std::unique_ptr<PassTimingConfig> config) {
   // Check if pass timing is already enabled.
   if (passTiming)
     return;
   if (!config)
     config = std::make_unique<PassManager::PassTimingConfig>();
-  addInstrumentation(std::make_unique<PassTiming>(std::move(config)));
-  passTiming = true;
+  auto pt = std::make_unique<PassTiming>(std::move(config));
+  passTiming = pt.get();
+  addInstrumentation(std::move(pt));
+}
+
+namespace {
+/// An opaque handle for an external timer. Returned from `getExternalTimer()`
+/// to allow the caller to include code paths outside the PM in the timing
+/// statistic.
+struct ExternalTimerImpl : public PassManager::ExternalTimer {
+  PassTiming *pt;
+  Identifier name;
+
+  ExternalTimerImpl(PassTiming *pt, Identifier name) : pt(pt), name(name) {}
+  virtual ~ExternalTimerImpl() { stop(); }
+
+  void start() override {
+    if (pt)
+      pt->startExternalTimer(name);
+  }
+
+  void stop() override {
+    if (pt)
+      pt->stopExternalTimer(name);
+  }
+};
+} // namespace
+
+/// Get a timer that can be used to include things outside of the pass manager
+/// in the timing statistics.
+std::unique_ptr<PassManager::ExternalTimer>
+PassManager::getExternalTimer(Identifier name) {
+  return std::make_unique<ExternalTimerImpl>(passTiming, name);
 }