[Timers] Use the pass argument name for JSON keys in time-passes

[thegameg]

Summary

When using clang --save-stats -mllvm -time-passes, both timers and stats end up in the same json file.

We could end up with things like:

{
  "asm-printer.EmittedInsts": 1,
  "time.pass.Virtual Register Map.wall": 2.9015541076660156e-04,
  "time.pass.Virtual Register Map.user": 2.0500000000000379e-04,
  "time.pass.Virtual Register Map.sys": 8.5000000000001741e-05,
}

This patch makes use of the pass argument name (if available) in the JSON key to end up with things like:

{
  "asm-printer.EmittedInsts": 1,
  "time.pass.virtregmap.wall": 2.9015541076660156e-04,
  "time.pass.virtregmap.user": 2.0500000000000379e-04,
  "time.pass.virtregmap.sys": 8.5000000000001741e-05,
}

This also helps avoiding to write another JSON printer to handle all the cases that we could have in our pass names.

[lebedev.ri]

I guess this makes sense.
But what about new pass manager?

[lebedev.ri]

Adding some reviewers from the differential (D25583) that added the LHS of the diff.

[paquette]

This would make much more sense IMO. Especially for passes with long names. (For example, I'd much rather see "patchable-function" than "Implement the 'patchable-function' attribute" in the JSON)

In D48109#1130700, @lebedev.ri wrote:

I guess this makes sense.
But what about new pass manager?

A quick grep shows that Timers are currently only used in the legacy PM.

Is timing info implemented in the new PM yet? If not, I think it would make sense to just move forward with this; the change is pretty simple, and I can't really see it blocking or increasing the work that'd have to go into a new PM implementation.

Other than the new PM question, this LGTM.

[lebedev.ri]

In D48109#1131244, @paquette wrote:

This would make much more sense IMO. Especially for passes with long names. (For example, I'd much rather see "patchable-function" than "Implement the 'patchable-function' attribute" in the JSON)

In D48109#1130700, @lebedev.ri wrote:

I guess this makes sense.
But what about new pass manager?

A quick grep shows that Timers are currently only used in the legacy PM.

Is timing info implemented in the new PM yet? If not, I think it would make sense to just move forward with this; the change is pretty simple, and I can't really see it blocking or increasing the work that'd have to go into a new PM implementation.

No idea.
I'm simply noting that this only touches only one of the two pm's.
If the new one does not have this functionality at all, then yes, this is good as-is.

Other than the new PM question, this LGTM.

Same.

[thegameg]

@chandlerc @jlebar can you confirm that the new PM doesn't have timer support?

[MatzeB]

In D48109#1131367, @thegameg wrote:

@chandlerc @jlebar can you confirm that the new PM doesn't have timer support?

I'm pretty sure I only added the timer->statistics support for the "legacy" pass manager.

LGTM.

I suspect there aren't too many people tracking those stats yet so renaming should be fine.

(I also wish pass names and argument names would just be the same, but this is fine for the status quo).

[Lekensteyn]

FYI, this caused a regression: http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/49533/testReport/junit/LLVM/Other/statistic_ll/

/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/test/Other/statistic.ll:11:17: error: expected string not found in input
; JSONTIME-DAG: "time.pass.Remove redundant instructions.wall"
                ^
<stdin>:2:2: note: scanning from here
 "instsimplify.NumSimplified": 1,
 ^

[thegameg]

Reverted in r334651.

[thegameg]

Re-landed in r334657.

lib/IR/LegacyPassManager.cpp

public:
   Timer *getPassTimer(Pass *P) {
     if (P->getAsPMDataManager())
       return nullptr;
 
     sys::SmartScopedLock<true> Lock(*TimingInfoMutex);
     Timer *&T = TimingData[P];
     if (!T) {
       StringRef PassName = P->getPassName();
-      T = new Timer(PassName, PassName, TG);
+      StringRef PassArgument;
+      if (const PassInfo *PI = Pass::lookupPassInfo(P->getPassID()))
+        PassArgument = PI->getPassArgument();
+      T = new Timer(PassArgument.empty() ? PassName : PassArgument, PassName,
+                    TG);
     }
     return T;
   }
 };
 
 } // End of anon namespace
 
 static TimingInfo *TheTimeInfo;

test/CodeGen/X86/time-passes-json-stats.ll

+; RUN: llc -mtriple=x86_64-- -stats-json=true -stats -time-passes %s -o /dev/null 2>&1 | FileCheck %s
+
+; Verify that we use the argument pass name instead of the full name as a json
+; key for timers.
+;
+; CHECK: {
+; CHECK-NEXT: "asm-printer.EmittedInsts":
+; CHECK-NOT: Virtual Register Map
+; CHECK: "time.pass.virtregmap.wall":
+; CHECK: "time.pass.virtregmap.user":
+; CHECK: "time.pass.virtregmap.sys":
+; CHECK: Virtual Register Map
+
+define void @test_stats() { ret void }