[Support] Time profiler: support new PassManager

[anton-afanasyev]

Summary

Add time tracing of backend passes invoked by new PassManager.

[lebedev.ri]

I know this is in line with what is done in old-pm, but i find the middle-end names to be very useless.
In particular, why is the actual optimization pass name is always in the description, and the actual timeslice name is always the same generic "Opt???"?
IMHO the actual optimization pass name should be the Name, not Detail.

[anton-afanasyev]

In D62067#1506690, @lebedev.ri wrote:

I know this is in line with what is done in old-pm, but i find the middle-end names to be very useless.
In particular, why is the actual optimization pass name is always in the description, and the actual timeslice name is always the same generic "Opt???"?
IMHO the actual optimization pass name should be the Name, not Detail.

We can change this in a separate commits in both new and old pm.
Same name "Opt???" makes the same color in a flamechart. Btw, have you tried Speedscope (https://www.speedscope.app) for visualization? It works better than chrome://tracing, trying to show details in chart boxes.

[lebedev.ri]

In D62067#1506714, @anton-afanasyev wrote:

In D62067#1506690, @lebedev.ri wrote:

I know this is in line with what is done in old-pm, but i find the middle-end names to be very useless.
In particular, why is the actual optimization pass name is always in the description, and the actual timeslice name is always the same generic "Opt???"?
IMHO the actual optimization pass name should be the Name, not Detail.

We can change this in a separate commits in both new and old pm.

Same name "Opt???" makes the same color in a flamechart.

Yes, indeed.
I *believe* it also makes double-clicking on one select *all* the slices with that "Opt???" name.
In other words, how do i group by the pass name?
It can be argued either way which is more important, sure.

Btw, have you tried Speedscope (https://www.speedscope.app) for visualization? It works better than chrome://tracing, trying to show details in chart boxes.

No, i know it exists, but i didn't try it..

[lebedev.ri]

I'm not really familiar with new-PM.
Can this perhaps have a test?

[philip.pfaffe]

I might have fallen a bit out of the loop, but can you tell me a bit more details about the use case here? There is no in-tree support for new-PM backend passes currently, is this for an out-of-tree user?
What about all the other IRUnit nestings beyond function passes in module pipelines?

Those questions aside, I'm not happy with the implementation. It uses global state, which is bad on its own, but in a way that isn't remotely threadsafe. It also cannot be easily made threadsafe without bigger redesign. Therefore, this isn't going to work in the new PM.

In the new PassManager we have a quite nice instrumentation infrastructure, which is perfectly suited to hook tracers such as this one into the pipeline execution. I suggest implementing -ftime-trace on top of that infrastructure. That being said, it appears as if there's quite some overlap between this tracer and the pass timing things. Why is the existing mechanism insufficient? Is the difference really just the output format? In which case, why not make that customizable instead of rolling out a second timing instrumentation?

[anton-afanasyev]

In D62067#1508281, @philip.pfaffe wrote:

I might have fallen a bit out of the loop, but can you tell me a bit more details about the use case here? There is no in-tree support for new-PM backend passes currently, is this for an out-of-tree user?
What about all the other IRUnit nestings beyond function passes in module pipelines?

Those questions aside, I'm not happy with the implementation. It uses global state, which is bad on its own, but in a way that isn't remotely threadsafe. It also cannot be easily made threadsafe without bigger redesign. Therefore, this isn't going to work in the new PM.

In the new PassManager we have a quite nice instrumentation infrastructure, which is perfectly suited to hook tracers such as this one into the pipeline execution. I suggest implementing -ftime-trace on top of that infrastructure. That being said, it appears as if there's quite some overlap between this tracer and the pass timing things. Why is the existing mechanism insufficient? Is the difference really just the output format? In which case, why not make that customizable instead of rolling out a second timing instrumentation?

Hmm, thank you for noting this all, I'm not familiar with new PM. Could you please describe instrumentation infrastructure you are suggesting?

[philip.pfaffe]

In essence, the infrastructure is a set of callbacks that you can register with PassBuilder. Your callbacks then get invoked at various points withing pipelines built by PB.

See StandardInstrumentations as well as its use in opt's NewPMDriver for an example.

[lebedev.ri]

With new-pm soon being the default, i'm curious what's the plan with -ftime-trace?
Will it simply turn into a pumpkin? Is there an upcoming patch (along the lines of D62067#1511128)?

[anton-afanasyev]

In D62067#1672835, @lebedev.ri wrote:

With new-pm soon being the default, i'm curious what's the plan with -ftime-trace?
Will it simply turn into a pumpkin? Is there an upcoming patch (along the lines of D62067#1511128)?

It still will be actual for FE part of profiler timers. What is the planned date of switching to new PM?

[lebedev.ri]

In D62067#1673829, @anton-afanasyev wrote:

In D62067#1672835, @lebedev.ri wrote:

With new-pm soon being the default, i'm curious what's the plan with -ftime-trace?
Will it simply turn into a pumpkin? Is there an upcoming patch (along the lines of D62067#1511128)?

It still will be actual for FE part of profiler timers.

What is the planned date of switching to new PM?

"As soon as possible"?
I think it is all but guaranteed to finally happen this release cycle
https://lists.llvm.org/pipermail/llvm-dev/2019-August/134326.html
There's patch i think, but i can't find it.

[anton-afanasyev]

Starting from D71059 time profiler supports multiple threads, this is a step to support new PassManager.
Regarding note about "why not to use new PM infrastructure for whole time profiler?" The current implementation of time profiler is independent and used by frontend (clang) and ThinLTO (lld). It is rather simple to use by adding just one line at the start of the profiled scope. I'm not sure new PM infrastructure is appropriate for it.

[anton-afanasyev]

Here is the another patch for new pm support: D74516

llvm/include/llvm/IR/PassManager.h

 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/TinyPtrVector.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/PassInstrumentation.h"
 #include "llvm/IR/PassManagerInternal.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/TypeName.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
 #include <cstring>
 #include <iterator>
 #include <list>
 #include <memory>
for (unsigned Idx = 0, Size = Passes.size(); Idx != Size; ++Idx) {
                << "\n";
 
       // Check the PassInstrumentation's BeforePass callbacks before running the
       // pass, skip its execution completely if asked to (callback returns
       // false).
       if (!PI.runBeforePass<IRUnitT>(*P, IR))
         continue;
 
+      llvm::TimeTraceScope PassScope("RunPass", P->name());
+
       PreservedAnalyses PassPA = P->run(IR, AM, ExtraArgs...);
 
       // Call onto PassInstrumentation's AfterPass callbacks immediately after
       // running the pass.
       PI.runAfterPass<IRUnitT>(*P, IR);
 
       // Update the analysis manager as each pass runs and potentially
       // invalidates analyses.
public:
   explicit ModuleToFunctionPassAdaptor(FunctionPassT Pass)
       : Pass(std::move(Pass)) {}
 
   /// Runs the function pass across every function in the module.
   PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM) {
     FunctionAnalysisManager &FAM =
         AM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
 
+    llvm::TimeTraceScope ModuleScope("OptModule", M.getName());
+
     // Request PassInstrumentation from analysis manager, will use it to run
     // instrumenting callbacks for the passes later.
     PassInstrumentation PI = AM.getResult<PassInstrumentationAnalysis>(M);
 
     PreservedAnalyses PA = PreservedAnalyses::all();
     for (Function &F : M) {
       if (F.isDeclaration())
         continue;
 
       // Check the PassInstrumentation's BeforePass callbacks before running the
       // pass, skip its execution completely if asked to (callback returns
       // false).
       if (!PI.runBeforePass<Function>(Pass, F))
         continue;
+
+      llvm::TimeTraceScope FunctionScope("OptFunction", F.getName());
+
       PreservedAnalyses PassPA = Pass.run(F, FAM);
 
       PI.runAfterPass(Pass, F);
 
       // We know that the function pass couldn't have invalidated any other
       // function's analyses (that's the contract of a function pass), so
       // directly handle the function analysis manager's invalidation here.
       FAM.invalidate(F, PassPA);