[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

Hi @nathanchance, @mgehre, fixed here: https://reviews.llvm.org/D66873, please, 
look at it to commit.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

In D63325#1645892 , @mgehre wrote:

> I'm seeing the same issue `Not all CodeGen sections are inside any Frontend 
> section!` with python 3.7.1. Json: F9863382: check-time-trace-sections.json 
> 


Yes, confirmed that is python version issue, I'm working on it.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

I'm seeing the same issue `Not all CodeGen sections are inside any Frontend 
section!` with python 3.7.1. Json: F9863382: check-time-trace-sections.json 



Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

> Could it be an issue with python? What is the version you are using?

I would assume so...

  $ /usr/bin/python --version
  Python 3.7.4


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

In D63325#1643023 , @nathanchance 
wrote:

> Done, thanks for looking into this!
>
> F9847579: check-time-trace-sections.json 


Hmm, I still didn't manage to reproduce the issue using your json-file:

  [~/llvm] > cat check-time-trace-sections.json | python 
llvm-project/clang/test/Driver/check-time-trace-sections.py
  [~/llvm] > echo $status
  0

The file itself looks good for this test: all Codegen sections are inside any 
Frontend section and all Frontend sections are before all Backend sections.
Also your log shows no error messages, so test should pass ok. Could it be an 
issue with python? What is the version you are using?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

Done, thanks for looking into this!

F9847579: check-time-trace-sections.json 


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

Hi @nathanchance , I could not reproduce your failure using the same revision 
and `build-llvm` script. Could you please attach your json report located at 
`/home/nathan/cbl/git/tc-build/build/llvm/stage1/tools/clang/test/Driver/Output/check-time-trace-sections.json`?


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance added a comment.

This test case has never passed for me with `check-clang`, is there something 
environment related that is needed with this or are others experiencing this 
too?

  [147/148] Running the Clang regression tests
  llvm-lit: 
/home/nathan/cbl/git/tc-build/llvm-project/llvm/utils/lit/lit/llvm/config.py:340:
 note: using clang: /home/nathan/cbl/git/tc-build/build/llvm/stage1/bin/clang
  FAIL: Clang :: Driver/check-time-trace-sections.cpp (4955 of 15278)
   TEST 'Clang :: Driver/check-time-trace-sections.cpp' 
FAILED 
  Script:
  --
  : 'RUN: at line 2';   
/home/nathan/cbl/git/tc-build/build/llvm/stage1/bin/clang --driver-mode=g++ -S 
-ftime-trace -ftime-trace-granularity=0 -o 
/home/nathan/cbl/git/tc-build/build/llvm/stage1/tools/clang/test/Driver/Output/check-time-trace-sections
 
/home/nathan/cbl/git/tc-build/llvm-project/clang/test/Driver/check-time-trace-sections.cpp
  : 'RUN: at line 3';   cat 
/home/nathan/cbl/git/tc-build/build/llvm/stage1/tools/clang/test/Driver/Output/check-time-trace-sections.json
 | "/usr/bin/python" 
/home/nathan/cbl/git/tc-build/llvm-project/clang/test/Driver/check-time-trace-sections.py
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  Time trace json-file dumped to 
/home/nathan/cbl/git/tc-build/build/llvm/stage1/tools/clang/test/Driver/Output/check-time-trace-sections.json
  Use chrome://tracing or Speedscope App (https://www.speedscope.app) for 
flamegraph visualization
  Not all CodeGen sections are inside any Frontend section!
  
  --
  
  
  Testing Time: 38.46s
  
  Failing Tests (1):
  Clang :: Driver/check-time-trace-sections.cpp
  
Expected Passes: 14008
Expected Failures  : 15
Unsupported Tests  : 1254
Unexpected Failures: 1
  FAILED: tools/clang/test/CMakeFiles/check-clang 

This is on bdceb9fb14595d10f7d94e1dd950cf2d94d2f2d3 
 using 
this script 's command: 
`./build-llvm.py --build-stage1-only --check-targets clang --projects clang 
--targets X86`


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL369308: [Support][Time profiler] Make FE codegen blocks to 
be inside frontend blocks (authored by anton-afanasyev, committed by ).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D63325?vs=206187=216011#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325

Files:
  cfe/trunk/lib/CodeGen/CodeGenAction.cpp
  cfe/trunk/test/Driver/check-time-trace-sections.cpp
  cfe/trunk/test/Driver/check-time-trace-sections.py
  llvm/trunk/lib/Support/TimeProfiler.cpp


Index: llvm/trunk/lib/Support/TimeProfiler.cpp
===
--- llvm/trunk/lib/Support/TimeProfiler.cpp
+++ llvm/trunk/lib/Support/TimeProfiler.cpp
@@ -58,8 +58,8 @@
 auto  = Stack.back();
 E.Duration = steady_clock::now() - E.Start;
 
-// Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+// Only include sections longer or equal to TimeTraceGranularity msec.
+if (duration_cast(E.Duration).count() >= 
TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: cfe/trunk/test/Driver/check-time-trace-sections.py
===
--- cfe/trunk/test/Driver/check-time-trace-sections.py
+++ cfe/trunk/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,25 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+def is_before(range1, range2):
+b = range1["ts"] + range1["dur"]; c = range2["ts"]
+return b <= c
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda x: x["name"] == "CodeGen Function", events)
+frontends = filter(lambda x: x["name"] == "Frontend", events)
+backends = filter(lambda x: x["name"] == "Backend", events)
+
+if not all([any([is_inside(codegen, frontend) for frontend in frontends])
+for codegen in codegens]):
+sys.exit("Not all CodeGen sections are inside any Frontend section!")
+
+if not all([all([is_before(frontend, backend) for frontend in frontends])
+for backend in backends]):
+sys.exit("Not all Frontend section are before all Backend sections!")
Index: cfe/trunk/test/Driver/check-time-trace-sections.cpp
===
--- cfe/trunk/test/Driver/check-time-trace-sections.cpp
+++ cfe/trunk/test/Driver/check-time-trace-sections.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: shell
+// RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o 
%T/check-time-trace-sections %s
+// RUN: cat %T/check-time-trace-sections.json | %python 
%S/check-time-trace-sections.py
+
+template 
+void foo(T) {}
+void bar() { foo(0); }
Index: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp
@@ -38,6 +38,7 @@
 #include "llvm/Pass.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -229,6 +230,7 @@
 
 void HandleTranslationUnit(ASTContext ) override {
   {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");
 if (FrontendTimesIsEnabled) {
   LLVMIRGenerationRefCount += 1;


Index: llvm/trunk/lib/Support/TimeProfiler.cpp
===
--- llvm/trunk/lib/Support/TimeProfiler.cpp
+++ llvm/trunk/lib/Support/TimeProfiler.cpp
@@ -58,8 +58,8 @@
 auto  = Stack.back();
 E.Duration = steady_clock::now() - E.Start;
 
-// Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+// Only include sections longer or equal to TimeTraceGranularity msec.
+if (duration_cast(E.Duration).count() >= TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: cfe/trunk/test/Driver/check-time-trace-sections.py
===
--- cfe/trunk/test/Driver/check-time-trace-sections.py
+++ cfe/trunk/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,25 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c 

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Russell Gallop via Phabricator via cfe-commits]

russell.gallop accepted this revision.
russell.gallop added a comment.
This revision is now accepted and ready to land.

I don't know a lot about the structure of clang but it LGTM from the point of 
view of the code and traces coming out.

I'm not very keen on having two "Frontend" sections, but I think it works okay. 
It moves Clang CodeGen under "Frontend". "Total Frontend" and "Total Backend" 
between them now cover almost all of the execution time (so is more accurate 
than without this change).

I think there's always going to be a trade off with time-trace between 
presenting something simple that makes sense to an end user and revealing how 
LLVM/Clang is actually structured inside, which is much more complicated (and 
doesn't nicely fit into scopes).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

Ping!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

Ping!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

Ping!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

Hi @lebedev.ri, could you please lgtm this or elaborate on possible issue with 
this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

Hi all! Could it be accepted or reviewed more?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

Ping!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

F9459646: spirit-c5ceba.json 
F9459650: Снимок экрана от 2019-07-04 18-54-58.png 


I've upload json-file and printscreen of its visualization (used 
https://speedscope.app). This is an example of how this patch works, making two 
`Frontend` sections.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev marked an inline comment as done.
anton-afanasyev added a comment.

Is it ok now? I doubt that main code refactoring is a good way when adding 
support timer code. So ended with more robust solution, though it leads to two 
`Frontend` sections.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev marked 2 inline comments as done.
anton-afanasyev added inline comments.



Comment at: llvm/lib/Support/TimeProfiler.cpp:67
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)

russell.gallop wrote:
> This comment looks wrong since this change. Please can you update or reword 
> it?
Ok, updated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev updated this revision to Diff 206187.
anton-afanasyev added a comment.

Changed comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/Driver/check-time-trace-sections.cpp
  clang/test/Driver/check-time-trace-sections.py
  llvm/lib/Support/TimeProfiler.cpp


Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -64,8 +64,8 @@
 auto  = Stack.back();
 E.Duration = steady_clock::now() - E.Start;
 
-// Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+// Only include sections longer or equal to TimeTraceGranularity msec.
+if (duration_cast(E.Duration).count() >= 
TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-sections.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,25 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+def is_before(range1, range2):
+b = range1["ts"] + range1["dur"]; c = range2["ts"]
+return b <= c
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda x: x["name"] == "CodeGen Function", events)
+frontends = filter(lambda x: x["name"] == "Frontend", events)
+backends = filter(lambda x: x["name"] == "Backend", events)
+
+if not all([any([is_inside(codegen, frontend) for frontend in frontends])
+for codegen in codegens]):
+sys.exit("Not all CodeGen sections are inside any Frontend section!")
+
+if not all([all([is_before(frontend, backend) for frontend in frontends])
+for backend in backends]):
+sys.exit("Not all Frontend section are before all Backend sections!")
Index: clang/test/Driver/check-time-trace-sections.cpp
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: shell
+// RUN: %clangxx -S -ftime-trace -mllvm --time-trace-granularity=0 -o 
%T/check-time-trace-sections %s
+// RUN: cat %T/check-time-trace-sections.json | %python 
%S/check-time-trace-sections.py
+
+template 
+void foo(T) {}
+void bar() { foo(0); }
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -37,6 +37,7 @@
 #include "llvm/Pass.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -228,6 +229,7 @@
 
 void HandleTranslationUnit(ASTContext ) override {
   {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");
 if (FrontendTimesIsEnabled) {
   LLVMIRGenerationRefCount += 1;


Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -64,8 +64,8 @@
 auto  = Stack.back();
 E.Duration = steady_clock::now() - E.Start;
 
-// Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+// Only include sections longer or equal to TimeTraceGranularity msec.
+if (duration_cast(E.Duration).count() >= TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-sections.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,25 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+def is_before(range1, range2):
+b = range1["ts"] + range1["dur"]; c = range2["ts"]
+return b <= c
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda x: x["name"] == "CodeGen Function", events)
+frontends = filter(lambda x: x["name"] == "Frontend", events)
+backends = filter(lambda x: x["name"] == "Backend", 

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Russell Gallop via Phabricator via cfe-commits]

russell.gallop added inline comments.



Comment at: llvm/lib/Support/TimeProfiler.cpp:67
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)

This comment looks wrong since this change. Please can you update or reword it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

Also, first `"Frontend"` section contains `"ParseTemplate"` and 
`"PerformPendingInstantiations"` sections.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

In D63325#1553458 , @lebedev.ri wrote:

> Hm, i started writing previous comment before you posted your last comment, 
> so i didn't see the last update.
>  This looks less intrusive, yes, but two observations:
>
> 1. You now have two `"Frontend"` sections - first one being for lexing time
> 2. That lexing section is not within the `"Frontend"` section, even though it 
> is for sure part of frontend of the compiler. Like i said, i'm not sure what 
> the right solution here is.


Why lexing section is not within `"Frontend"` section. Your p.1 claims is is 
within. For now, first `"Frontend"` section includes lexing as well as frontend 
codegen (called from `Consumer->HandleTopLevelDecl()`. Second `"Frontend"` 
section includes only frontend codegen through `HandleTranslationUnit()` 
calling.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Hm, i started writing previous comment before you posted your last comment, so 
i didn't see the last update.
This looks less intrusive, yes, but two observations:

1. You now have two `"Frontend"` sections - first one being for lexing time
2. That lexing section is not within the `"Frontend"` section, even though it 
is for sure part of frontend of the compiler.

Like i said, i'm not sure what the right solution here is.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri resigned from this revision.
lebedev.ri added a comment.

I'm sorry, i should have noticed that earlier,  the handling of `"Frontend"` 
section
still looks way too intrusive to me, it breaks abstractions.

The only simple solution i see is to

1. Insert a timer for `PrettyStackTraceString CrashInfo("Per-file LLVM IR 
generation");` section
2. Don't stop `"Frontend"` timer within `HandleTranslationUnit` function
3. Naturally, change `"Frontend"` timer name to something else (`"Clang 
invocation"`?)

This is the trivial solution that wouldn't be intrusive, but would not get you 
the `"frontend timer"`
in the sense that it doesn't include the middle-end pipe, backend stuff, etc, 
only the

The alternative raises from the question - why does `CodeGen/CodeGenAction.cpp` 
do all that stuff?
I.e. maybe somehow chop that class into 2 or 3 smaller classes - actual ir 
codegen,
middle-end optimization pipeline + opt-remarks handling, etc.
I don't know if that is possible, or viable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev updated this revision to Diff 205978.
anton-afanasyev added a comment.

Hi @lebedev.ri, I've turned back to the solution with one `TimeTraceScope` for 
"Frontend" inside `BackendConsumer::HandleTranslationUnit()`, since it looks 
more robust. This admits several (two for now) "Frontend" sections, but I see 
no problem with this. Other solutions have to rely on `BackendConsumer` and 
`ASTConsumer` calling correct relations, which are not guaranteed (though it is 
looking correct for now).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/Driver/check-time-trace-sections.cpp
  clang/test/Driver/check-time-trace-sections.py
  llvm/lib/Support/TimeProfiler.cpp


Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -65,7 +65,7 @@
 E.Duration = steady_clock::now() - E.Start;
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+if (duration_cast(E.Duration).count() >= 
TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-sections.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,25 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+def is_before(range1, range2):
+b = range1["ts"] + range1["dur"]; c = range2["ts"]
+return b <= c
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda x: x["name"] == "CodeGen Function", events)
+frontends = filter(lambda x: x["name"] == "Frontend", events)
+backends = filter(lambda x: x["name"] == "Backend", events)
+
+if not all([any([is_inside(codegen, frontend) for frontend in frontends])
+for codegen in codegens]):
+sys.exit("Not all CodeGen sections are inside any Frontend section!")
+
+if not all([all([is_before(frontend, backend) for frontend in frontends])
+for backend in backends]):
+sys.exit("Not all Frontend section are before all Backend sections!")
Index: clang/test/Driver/check-time-trace-sections.cpp
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: shell
+// RUN: %clangxx -S -ftime-trace -mllvm --time-trace-granularity=0 -o 
%T/check-time-trace-sections %s
+// RUN: cat %T/check-time-trace-sections.json | %python 
%S/check-time-trace-sections.py
+
+template 
+void foo(T) {}
+void bar() { foo(0); }
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -37,6 +37,7 @@
 #include "llvm/Pass.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -228,6 +229,7 @@
 
 void HandleTranslationUnit(ASTContext ) override {
   {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");
 if (FrontendTimesIsEnabled) {
   LLVMIRGenerationRefCount += 1;


Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -65,7 +65,7 @@
 E.Duration = steady_clock::now() - E.Start;
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+if (duration_cast(E.Duration).count() >= TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-sections.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,25 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+def is_before(range1, range2):
+b = range1["ts"] + range1["dur"]; c = range2["ts"]
+return b <= c
+
+events = 

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev updated this revision to Diff 205673.
anton-afanasyev added a comment.

Updated, changed test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/test/Driver/check-time-trace-sections.cpp
  clang/test/Driver/check-time-trace-sections.py
  llvm/lib/Support/TimeProfiler.cpp

Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -65,7 +65,7 @@
 E.Duration = steady_clock::now() - E.Start;
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+if (duration_cast(E.Duration).count() >= TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-sections.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,32 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+def is_before(range1, range2):
+b = range1["ts"] + range1["dur"]; c = range2["ts"]
+return b <= c
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda x: x["name"] == "CodeGen Function", events)
+frontends = filter(lambda x: x["name"] == "Frontend", events)
+backends = filter(lambda x: x["name"] == "Backend", events)
+
+if not len(frontends) == 1:
+sys.exit("There should be exactly one Frontend section!")
+
+if not len(backends) == 1:
+sys.exit("There should be exactly one Backend section!")
+
+frontend = frontends[0]
+backend = backends[0]
+
+if not all([is_inside(codegen, frontend) for codegen in codegens]):
+sys.exit("Not all CodeGen sections are inside Frontend section!")
+
+if not is_before(frontend, backend):
+sys.exit("Frontend section are not before Backend section!")
Index: clang/test/Driver/check-time-trace-sections.cpp
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: shell
+// RUN: %clangxx -S -ftime-trace -mllvm --time-trace-granularity=0 -o %T/check-time-trace-sections %s
+// RUN: cat %T/check-time-trace-sections.json | %python %S/check-time-trace-sections.py
+
+template 
+void foo(T) {}
+void bar() { foo(0); }
Index: clang/lib/Parse/ParseAST.cpp
===
--- clang/lib/Parse/ParseAST.cpp
+++ clang/lib/Parse/ParseAST.cpp
@@ -150,8 +150,12 @@
   // after the pragma, there won't be any tokens or a Lexer.
   bool HaveLexer = S.getPreprocessor().getCurrentLexer();
 
+  // Start "Frontend" section finishing inside clang::HandleTranslationUnit()
+  if (llvm::timeTraceProfilerEnabled())
+llvm::timeTraceProfilerBegin("Frontend", StringRef(""));
+
   if (HaveLexer) {
-llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
+llvm::TimeTraceScope TimeScope("Lexing", StringRef(""));
 P.Initialize();
 Parser::DeclGroupPtrTy ADecl;
 for (bool AtEOF = P.ParseFirstTopLevelDecl(ADecl); !AtEOF;
@@ -159,8 +163,11 @@
   // If we got a null return and something *was* parsed, ignore it.  This
   // is due to a top-level semicolon, an action override, or a parse error
   // skipping something.
-  if (ADecl && !Consumer->HandleTopLevelDecl(ADecl.get()))
+  if (ADecl && !Consumer->HandleTopLevelDecl(ADecl.get())) {
+if (llvm::timeTraceProfilerEnabled())
+  llvm::timeTraceProfilerEnd();
 return;
+  }
 }
   }
 
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -37,6 +37,7 @@
 #include "llvm/Pass.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -246,6 +247,12 @@
 IRGenFinished = true;
   }
 
+  // Finish "Frontend" section starting inside clang::ParseAST()
+  // We don't want "Backend" section to turn out within "Frontend" section,
+  // so finishing it here before EmitBackendOutput() and possible return's.
+  if (llvm::timeTraceProfilerEnabled())
+llvm::timeTraceProfilerEnd();
+
   // Silently ignore if we weren't initialized for some reason.
   if (!getModule())
 return;

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev marked an inline comment as done.
anton-afanasyev added inline comments.



Comment at: clang/lib/Parse/ParseAST.cpp:167
   if (ADecl && !Consumer->HandleTopLevelDecl(ADecl.get()))
 return;
 }

@lebedev.ri This `return` should follow after 
`timeTraceProfilerEnd("Frontend",...)`, I'm to fix it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

In D63325#1549316 , @lebedev.ri wrote:

> Hmm, i'm only now noticing how fraglie this looks.
>  How do we know that `clang::ParseAST` will only ever be called with 
> `BackendConsumer` `ASTConsumer`?
>  How do we know that `BackendConsumer` will only ever be `ASTConsumer` from 
> `clang::ParseAST`?


This could be true by design. But I agree it's fragile. We can admit several 
`Frontend` sections though. But with this admission first variant with two 
`Frontend` `TimeTraceScope`'s is more preferrable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Hmm, i'm only now noticing how fraglie this looks.
How do we know that `clang::ParseAST` will only ever be called with 
`BackendConsumer` `ASTConsumer`?
How do we know that `BackendConsumer` will only ever be `ASTConsumer` from 
`clang::ParseAST`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev marked 2 inline comments as done.
anton-afanasyev added inline comments.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:250-252
+  // Finish "Frontend" section starting inside clang::ParseAST()
+  if (llvm::timeTraceProfilerEnabled())
+llvm::timeTraceProfilerEnd();

lebedev.ri wrote:
> anton-afanasyev wrote:
> > lebedev.ri wrote:
> > > I think i'm missing a point here.
> > > Why do we want to end `"Frontend"` section here,  exclude all this 
> > > following diag/optremark/etc stuff?
> > The answer is below, we have `EmitBackendOutput()` there, calling Backend 
> > actions.
> So, because we don't want "backend" timer to be within "frontend" timer.
> Can you please add that as a comment here? :)
Ok, done.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev updated this revision to Diff 205451.
anton-afanasyev marked an inline comment as done.
anton-afanasyev added a comment.

Updated


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/test/Driver/check-time-trace-sections.cpp
  clang/test/Driver/check-time-trace-sections.py
  llvm/lib/Support/TimeProfiler.cpp


Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -65,7 +65,7 @@
 E.Duration = steady_clock::now() - E.Start;
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+if (duration_cast(E.Duration).count() >= 
TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-sections.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,20 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda x: x["name"] == "CodeGen Function", events)
+frontends = filter(lambda x: x["name"] == "Frontend", events)
+
+if not len(frontends) == 1:
+sys.exit("There should be exactly one Frontend section!")
+
+frontend = frontends[0]
+
+if not all([is_inside(codegen, frontend) for codegen in codegens]):
+sys.exit("Not all CodeGen sections are inside Frontend section!")
Index: clang/test/Driver/check-time-trace-sections.cpp
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: shell
+// RUN: %clangxx -S -ftime-trace -mllvm --time-trace-granularity=0 -o 
%T/check-time-trace-sections %s
+// RUN: cat %T/check-time-trace-sections.json | %python 
%S/check-time-trace-sections.py
+
+template 
+void foo(T) {}
+void bar() { foo(0); }
Index: clang/lib/Parse/ParseAST.cpp
===
--- clang/lib/Parse/ParseAST.cpp
+++ clang/lib/Parse/ParseAST.cpp
@@ -150,8 +150,12 @@
   // after the pragma, there won't be any tokens or a Lexer.
   bool HaveLexer = S.getPreprocessor().getCurrentLexer();
 
+  // Start "Frontend" section finishing inside clang::HandleTranslationUnit()
+  if (llvm::timeTraceProfilerEnabled())
+llvm::timeTraceProfilerBegin("Frontend", StringRef(""));
+
   if (HaveLexer) {
-llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
+llvm::TimeTraceScope TimeScope("Lexing", StringRef(""));
 P.Initialize();
 Parser::DeclGroupPtrTy ADecl;
 for (bool AtEOF = P.ParseFirstTopLevelDecl(ADecl); !AtEOF;
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -37,6 +37,7 @@
 #include "llvm/Pass.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -246,6 +247,12 @@
 IRGenFinished = true;
   }
 
+  // Finish "Frontend" section starting inside clang::ParseAST()
+  // We don't want "Backend" section to turn out within "Frontend" section,
+  // so finishing it here before EmitBackendOutput() and possible return's.
+  if (llvm::timeTraceProfilerEnabled())
+llvm::timeTraceProfilerEnd();
+
   // Silently ignore if we weren't initialized for some reason.
   if (!getModule())
 return;


Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -65,7 +65,7 @@
 E.Duration = steady_clock::now() - E.Start;
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+if (duration_cast(E.Duration).count() >= TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-sections.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,20 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added inline comments.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:250-252
+  // Finish "Frontend" section starting inside clang::ParseAST()
+  if (llvm::timeTraceProfilerEnabled())
+llvm::timeTraceProfilerEnd();

anton-afanasyev wrote:
> lebedev.ri wrote:
> > I think i'm missing a point here.
> > Why do we want to end `"Frontend"` section here,  exclude all this 
> > following diag/optremark/etc stuff?
> The answer is below, we have `EmitBackendOutput()` there, calling Backend 
> actions.
So, because we don't want "backend" timer to be within "frontend" timer.
Can you please add that as a comment here? :)



Comment at: clang/test/Driver/check-time-trace-sections.py:1
+#!/usr/bin/env python
+

anton-afanasyev wrote:
> lebedev.ri wrote:
> > Does some other test already require python?
> Sorry, what do you mean? I see many tests inside `clang/test/` and 
> `llvm/test/` using python when Filecheck is not enough. Mostly they are using 
> `python -c ...` as part of the pipe, but there are py-files as well.
I guess that was a dumb question, since lit is python-based.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev updated this revision to Diff 205446.
anton-afanasyev marked 4 inline comments as done.
anton-afanasyev added a comment.

Updated


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/test/Driver/check-time-trace-sections.cpp
  clang/test/Driver/check-time-trace-sections.py
  llvm/lib/Support/TimeProfiler.cpp


Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -65,7 +65,7 @@
 E.Duration = steady_clock::now() - E.Start;
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+if (duration_cast(E.Duration).count() >= 
TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-sections.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,20 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda x: x["name"] == "CodeGen Function", events)
+frontends = filter(lambda x: x["name"] == "Frontend", events)
+
+if not len(frontends) == 1:
+sys.exit("There should be exactly one Frontend section!")
+
+frontend = frontends[0]
+
+if not all([is_inside(codegen, frontend) for codegen in codegens]):
+sys.exit("Not all CodeGen sections are inside Frontend section!")
Index: clang/test/Driver/check-time-trace-sections.cpp
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: shell
+// RUN: %clangxx -S -ftime-trace -mllvm --time-trace-granularity=0 -o 
%T/check-time-trace-sections %s
+// RUN: cat %T/check-time-trace-sections.json | %python 
%S/check-time-trace-sections.py
+
+template 
+void foo(T) {}
+void bar() { foo(0); }
Index: clang/lib/Parse/ParseAST.cpp
===
--- clang/lib/Parse/ParseAST.cpp
+++ clang/lib/Parse/ParseAST.cpp
@@ -150,8 +150,12 @@
   // after the pragma, there won't be any tokens or a Lexer.
   bool HaveLexer = S.getPreprocessor().getCurrentLexer();
 
+  // Start "Frontend" section finishing inside clang::HandleTranslationUnit()
+  if (llvm::timeTraceProfilerEnabled())
+llvm::timeTraceProfilerBegin("Frontend", StringRef(""));
+
   if (HaveLexer) {
-llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
+llvm::TimeTraceScope TimeScope("Lexing", StringRef(""));
 P.Initialize();
 Parser::DeclGroupPtrTy ADecl;
 for (bool AtEOF = P.ParseFirstTopLevelDecl(ADecl); !AtEOF;
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -37,6 +37,7 @@
 #include "llvm/Pass.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -246,6 +247,10 @@
 IRGenFinished = true;
   }
 
+  // Finish "Frontend" section starting inside clang::ParseAST()
+  if (llvm::timeTraceProfilerEnabled())
+llvm::timeTraceProfilerEnd();
+
   // Silently ignore if we weren't initialized for some reason.
   if (!getModule())
 return;


Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -65,7 +65,7 @@
 E.Duration = steady_clock::now() - E.Start;
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+if (duration_cast(E.Duration).count() >= TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-sections.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,20 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+events = 

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev marked 10 inline comments as done.
anton-afanasyev added inline comments.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:250-252
+  // Finish "Frontend" section starting inside clang::ParseAST()
+  if (llvm::timeTraceProfilerEnabled())
+llvm::timeTraceProfilerEnd();

lebedev.ri wrote:
> I think i'm missing a point here.
> Why do we want to end `"Frontend"` section here,  exclude all this following 
> diag/optremark/etc stuff?
The answer is below, we have `EmitBackendOutput()` there, calling Backend 
actions.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:256
   if (!getModule())
 return;
 

Also the function can return here, without finishing `Frontend` section, if it 
is below.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:306
 
   EmitBackendOutput(Diags, HeaderSearchOpts, CodeGenOpts, TargetOpts,
 LangOpts, C.getTargetInfo().getDataLayout(),

@lebedev.ri We have `EmitBackendOutput()` here, calling Backend actions.



Comment at: clang/lib/Parse/ParseAST.cpp:154
   if (HaveLexer) {
-llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 P.Initialize();

lebedev.ri wrote:
> Why not keep this as `"Lexing"`?
Hmm... Thanks, I'm to keep it with this name.



Comment at: clang/lib/Parse/ParseAST.cpp:182
   finalize(S.TemplateInstCallbacks, S);
 
   std::swap(OldCollectStats, S.CollectStats);

lebedev.ri wrote:
> I.e. why do we not want to end it here, where the other stats are printed?
Cause `HandleTranslationUnit()` contains Backend calling.



Comment at: clang/test/Driver/check-time-trace-sections.py:1
+#!/usr/bin/env python
+

lebedev.ri wrote:
> Does some other test already require python?
Sorry, what do you mean? I see many tests inside `clang/test/` and `llvm/test/` 
using python when Filecheck is not enough. Mostly they are using `python -c 
...` as part of the pipe, but there are py-files as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:250-252
+  // Finish "Frontend" section starting inside clang::ParseAST()
+  if (llvm::timeTraceProfilerEnabled())
+llvm::timeTraceProfilerEnd();

I think i'm missing a point here.
Why do we want to end `"Frontend"` section here,  exclude all this following 
diag/optremark/etc stuff?



Comment at: clang/lib/Parse/ParseAST.cpp:154
   if (HaveLexer) {
-llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 P.Initialize();

Why not keep this as `"Lexing"`?



Comment at: clang/lib/Parse/ParseAST.cpp:182
   finalize(S.TemplateInstCallbacks, S);
 
   std::swap(OldCollectStats, S.CollectStats);

I.e. why do we not want to end it here, where the other stats are printed?



Comment at: clang/test/Driver/check-time-trace-sections.py:1
+#!/usr/bin/env python
+

Does some other test already require python?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added inline comments.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:232
   {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");

lebedev.ri wrote:
> This looks more like `Frontend Codegen` to me?
> (With front-end itself being the entirety of clang time up to this point 
> including this step)
`Codegen` sections are not inside `HandleTranslationUnit()` function only. So 
I've changed it other way: started `Frontend` section inside `ParseAST()` and 
finished this section inside `HandleTranslationUnit()`. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev updated this revision to Diff 205297.
anton-afanasyev marked 2 inline comments as done.
anton-afanasyev added a comment.

Updated


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/test/Driver/check-time-trace-sections.cpp
  clang/test/Driver/check-time-trace-sections.py
  llvm/lib/Support/TimeProfiler.cpp


Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -65,7 +65,7 @@
 E.Duration = steady_clock::now() - E.Start;
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+if (duration_cast(E.Duration).count() >= 
TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-sections.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,20 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda x: x["name"] == "CodeGen Function", events)
+frontends = filter(lambda x: x["name"] == "Frontend", events)
+
+if not len(frontends) == 1:
+sys.exit("There should be exactly one Frontend section!")
+
+frontend = frontends[0]
+
+if not all([is_inside(codegen, frontend) for codegen in codegens]):
+sys.exit("Not all CodeGen sections are inside Frontend section!")
Index: clang/test/Driver/check-time-trace-sections.cpp
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: shell
+// RUN: %clangxx -S -ftime-trace -mllvm --time-trace-granularity=0 -o 
%T/check-time-trace-sections %s
+// RUN: cat %T/check-time-trace-sections.json | %python 
%S/check-time-trace-sections.py
+
+template 
+void foo(T) {}
+void bar() { foo(0); }
Index: clang/lib/Parse/ParseAST.cpp
===
--- clang/lib/Parse/ParseAST.cpp
+++ clang/lib/Parse/ParseAST.cpp
@@ -150,8 +150,11 @@
   // after the pragma, there won't be any tokens or a Lexer.
   bool HaveLexer = S.getPreprocessor().getCurrentLexer();
 
+  // Start "Frontend" section finishing inside clang::HandleTranslationUnit()
+  if (llvm::timeTraceProfilerEnabled())
+llvm::timeTraceProfilerBegin("Frontend", StringRef(""));
+
   if (HaveLexer) {
-llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 P.Initialize();
 Parser::DeclGroupPtrTy ADecl;
 for (bool AtEOF = P.ParseFirstTopLevelDecl(ADecl); !AtEOF;
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -37,6 +37,7 @@
 #include "llvm/Pass.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -246,6 +247,10 @@
 IRGenFinished = true;
   }
 
+  // Finish "Frontend" section starting inside clang::ParseAST()
+  if (llvm::timeTraceProfilerEnabled())
+llvm::timeTraceProfilerEnd();
+
   // Silently ignore if we weren't initialized for some reason.
   if (!getModule())
 return;


Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -65,7 +65,7 @@
 E.Duration = steady_clock::now() - E.Start;
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+if (duration_cast(E.Duration).count() >= TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-sections.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-sections.py
@@ -0,0 +1,20 @@
+#!/usr/bin/env python
+
+import json, sys
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda 

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:232
   {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");

This looks more like `Frontend Codegen` to me?
(With front-end itself being the entirety of clang time up to this point 
including this step)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev updated this revision to Diff 204738.
anton-afanasyev added a comment.

Small fix


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63325/new/

https://reviews.llvm.org/D63325

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/Driver/check-time-trace-blocks.cpp
  clang/test/Driver/check-time-trace-blocks.py
  llvm/lib/Support/TimeProfiler.cpp


Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -65,7 +65,7 @@
 E.Duration = steady_clock::now() - E.Start;
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+if (duration_cast(E.Duration).count() >= 
TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-blocks.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-blocks.py
@@ -0,0 +1,16 @@
+#!/usr/bin/env python
+
+import json, sys
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda x: x["name"] == "CodeGen Function", events)
+frontends = filter(lambda x: x["name"] == "Frontend", events)
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+if not all([any([is_inside(codegen, frontend) for frontend in frontends])
+for codegen in codegens]):
+sys.exit("Not all CodeGen blocks are inside any of Frontend blocks!")
Index: clang/test/Driver/check-time-trace-blocks.cpp
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-blocks.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: shell
+// RUN: %clangxx -S -ftime-trace -mllvm --time-trace-granularity=0 -o 
%T/check-time-trace-blocks %s
+// RUN: cat %T/check-time-trace-blocks.json | %python 
%S/check-time-trace-blocks.py
+
+template 
+void foo(T) {}
+void bar() { foo(0); }
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -37,6 +37,7 @@
 #include "llvm/Pass.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -228,6 +229,7 @@
 
 void HandleTranslationUnit(ASTContext ) override {
   {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");
 if (FrontendTimesIsEnabled) {
   LLVMIRGenerationRefCount += 1;


Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -65,7 +65,7 @@
 E.Duration = steady_clock::now() - E.Start;
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+if (duration_cast(E.Duration).count() >= TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-blocks.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-blocks.py
@@ -0,0 +1,16 @@
+#!/usr/bin/env python
+
+import json, sys
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda x: x["name"] == "CodeGen Function", events)
+frontends = filter(lambda x: x["name"] == "Frontend", events)
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+if not all([any([is_inside(codegen, frontend) for frontend in frontends])
+for codegen in codegens]):
+sys.exit("Not all CodeGen blocks are inside any of Frontend blocks!")
Index: clang/test/Driver/check-time-trace-blocks.cpp
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-blocks.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: shell
+// RUN: %clangxx -S -ftime-trace -mllvm --time-trace-granularity=0 -o %T/check-time-trace-blocks %s
+// RUN: cat %T/check-time-trace-blocks.json | %python %S/check-time-trace-blocks.py
+
+template 
+void foo(T) {}
+void bar() { foo(0); }
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- 

[PATCH] D63325: [Support][Time profiler] Make FE codegen blocks to be inside frontend blocks

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev created this revision.
anton-afanasyev added reviewers: russell.gallop, lebedev.ri, thakis.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

Add `Frontend` time trace entry to `HandleTranslationUnit()` function.
Add test to check all codegen blocks are inside frontend blocks.
Also, change `--time-trace-granularity` option a bit to make sure very small
time blocks are outputed to json-file when using `--time-trace-granularity=0`.

This fixes http://llvm.org/pr41969


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63325

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/Driver/check-time-trace-blocks.cpp
  clang/test/Driver/check-time-trace-blocks.py
  llvm/lib/Support/TimeProfiler.cpp


Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -65,7 +65,7 @@
 E.Duration = steady_clock::now() - E.Start;
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+if (duration_cast(E.Duration).count() >= 
TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-blocks.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-blocks.py
@@ -0,0 +1,16 @@
+#!/usr/bin/env python
+
+import json, sys
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda x: x["name"] == "CodeGen Function", events)
+frontends = filter(lambda x: x["name"] == "Frontend", events)
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+if not all([any([is_inside(codegen, frontend) for frontend in frontends])
+for codegen in codegens]):
+sys.exit("Not all CodeGen blocks are inside any of Frontend blocks!")
Index: clang/test/Driver/check-time-trace-blocks.cpp
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-blocks.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: shell
+// RUN: %clangxx -S -ftime-trace -mllvm --time-trace-granularity=0 -o 
%T/check-time-trace-blocks %s
+// RUN: cat %T/check-time-trace-blocks.json | %python 
%s/check-time-trace-blocks.py
+
+template 
+void foo(T) {}
+void bar() { foo(0); }
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -37,6 +37,7 @@
 #include "llvm/Pass.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/ToolOutputFile.h"
 #include "llvm/Support/YAMLTraits.h"
@@ -228,6 +229,7 @@
 
 void HandleTranslationUnit(ASTContext ) override {
   {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 PrettyStackTraceString CrashInfo("Per-file LLVM IR generation");
 if (FrontendTimesIsEnabled) {
   LLVMIRGenerationRefCount += 1;


Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -65,7 +65,7 @@
 E.Duration = steady_clock::now() - E.Start;
 
 // Only include sections longer than TimeTraceGranularity msec.
-if (duration_cast(E.Duration).count() > TimeTraceGranularity)
+if (duration_cast(E.Duration).count() >= TimeTraceGranularity)
   Entries.emplace_back(E);
 
 // Track total time taken by each "name", but only the topmost levels of
Index: clang/test/Driver/check-time-trace-blocks.py
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-blocks.py
@@ -0,0 +1,16 @@
+#!/usr/bin/env python
+
+import json, sys
+
+events = json.loads(sys.stdin.read())["traceEvents"]
+codegens = filter(lambda x: x["name"] == "CodeGen Function", events)
+frontends = filter(lambda x: x["name"] == "Frontend", events)
+
+def is_inside(range1, range2):
+a = range1["ts"]; b = a + range1["dur"]
+c = range2["ts"]; d = c + range2["dur"]
+return (a >= c and a <= d) and (b >= c and b <= d)
+
+if not all([any([is_inside(codegen, frontend) for frontend in frontends])
+for codegen in codegens]):
+sys.exit("Not all CodeGen blocks are inside any of Frontend blocks!")
Index: clang/test/Driver/check-time-trace-blocks.cpp
===
--- /dev/null
+++ clang/test/Driver/check-time-trace-blocks.cpp
@@ -0,0 +1,7 @@
+// REQUIRES: shell