[PATCH] D148410: [Parse] Remove TimeTraceScope for "ParseTemplate"

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev accepted this revision.
anton-afanasyev added a comment.
This revision is now accepted and ready to land.

Ok, let's remove it for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148410

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

[PATCH] D139302: [RISCV] Add Syntacore SCR1 CPU model

[Anton Afanasyev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc86a878e8995: [RISCV] Add Syntacore SCR1 CPU model (authored 
by dnpetrov-sc, committed by anton-afanasyev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139302

Files:
  clang/test/Driver/riscv-cpus.c
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/include/llvm/Support/RISCVTargetParser.def
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVSchedSyntacoreSCR1.td

Index: llvm/lib/Target/RISCV/RISCVSchedSyntacoreSCR1.td
===
--- /dev/null
+++ llvm/lib/Target/RISCV/RISCVSchedSyntacoreSCR1.td
@@ -0,0 +1,207 @@
+//==- RISCVSchedSyntacoreSCR1.td - Syntacore SCR1 Scheduling Definitions *- tablegen -*-=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+//===--===//
+
+// SCR1: https://github.com/syntacore/scr1
+
+// This model covers SYNTACORE_SCR1_CFG_RV32IMC_MAX configuration (syntacore-scr1-max).
+// SYNTACORE_SCR1_CFG_RV32IC_BASE (syntacore-scr1-base) configuration has essentially
+// same scheduling characteristics.
+
+// SCR1 is single-issue in-order processor
+def SyntacoreSCR1Model : SchedMachineModel {
+  let MicroOpBufferSize = 0;
+  let IssueWidth = 1;
+  let LoadLatency = 2;
+  let MispredictPenalty = 3;
+  let CompleteModel = 0;
+  let UnsupportedFeatures = [HasStdExtZbkb, HasStdExtZbkc, HasStdExtZbkx,
+ HasStdExtZknd, HasStdExtZkne, HasStdExtZknh,
+ HasStdExtZksed, HasStdExtZksh, HasStdExtZkr,
+ HasVInstructions];
+}
+
+let SchedModel = SyntacoreSCR1Model in {
+
+let BufferSize = 0 in {
+def SCR1_ALU : ProcResource<1>;
+def SCR1_LSU : ProcResource<1>;
+def SCR1_MUL : ProcResource<1>;
+def SCR1_DIV : ProcResource<1>;
+def SCR1_CFU : ProcResource<1>;
+}
+
+// Branching
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+
+// Integer arithmetic and logic
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+
+// Integer multiplication: single-cycle multiplier in SCR1_CFG_RV32IMC_MAX
+def : WriteRes;
+def : WriteRes;
+
+// Integer division: latency 33, inverse throughput 33
+let Latency = 33, ResourceCycles = [33] in {
+def : WriteRes;
+def : WriteRes;
+}
+
+// Load/store instructions on SCR1 have latency 2 and inverse throughput 2
+// (SCR1_CFG_RV32IMC_MAX includes TCM)
+let Latency = 2, ResourceCycles=[2] in {
+// Memory
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+}
+
+let Unsupported = true in {
+// Atomic memory
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+
+// FP load/store
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+
+// FP instructions
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+def : WriteRes;
+
+def : WriteRes;
+}
+
+// Others
+def : WriteRes;
+def : WriteRes;
+
+def : InstRW<[WriteIALU], (instrs COPY)>;
+
+//===--===//
+// Bypasses (none)
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;
+def : ReadAdvance;

[PATCH] D139302: [RISCV] Add Syntacore SCR1 CPU model

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVSchedSCR1.td:14
+// This model covers SCR1_CFG_RV32IMC_MAX configuration (scr1-max).
+// SCR1_CFG_RV32EC_MIN (scr1-min) and SCR1_CFG_RV32IC_BASE (scr1-base)
+// configurations have essentially same scheduling characteristics.

`SCR1_CFG_RV32EC_MIN (scr1-min)` -- this should be removed from comment as well 
since patch doesn't cover `scr1-min`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139302

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

[PATCH] D129065: [RISCV][Driver] Add libm linking to `RISCVToolchain` for C++

[Anton Afanasyev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf553287b5889: [RISCV][Driver] Add libm linking to 
`RISCVToolchain` for C++ (authored by anton-afanasyev).

Changed prior to commit:
  https://reviews.llvm.org/D129065?vs=442069=442402#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129065

Files:
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c


Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -58,7 +58,7 @@
 // CXX-RV64-BAREMETAL-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-linux-gnu/8.0.1{{/|}}crtbegin.o"
 // CXX-RV64-BAREMETAL-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-linux-gnu/8.0.1"
 // CXX-RV64-BAREMETAL-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib"
-// CXX-RV64-BAREMETAL-LP64: "-lstdc++" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
+// CXX-RV64-BAREMETAL-LP64: "-lstdc++" "-lm" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
 // CXX-RV64-BAREMETAL-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-linux-gnu/8.0.1{{/|}}crtend.o"
 
 // RUN: env "PATH=" %clangxx -### %s -fuse-ld= \
@@ -73,7 +73,7 @@
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-linux-gnu/8.0.1{{/|}}crtbegin.o"
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-linux-gnu/8.0.1"
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-linux-gnu/8.0.1/../../..{{/|}}..{{/|}}riscv64-unknown-linux-gnu/lib"
-// CXX-RV64-BAREMETAL-NOSYSROOT-LP64: "-lstdc++" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
+// CXX-RV64-BAREMETAL-NOSYSROOT-LP64: "-lstdc++" "-lm" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-linux-gnu/8.0.1{{/|}}crtend.o"
 
 // RUN: env "PATH=" %clang -### %s -fuse-ld= -no-pie \
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -58,7 +58,7 @@
 // CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-linux-gnu/8.0.1{{/|}}crtbegin.o"
 // CXX-RV32-BAREMETAL-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-linux-gnu/8.0.1"
 // CXX-RV32-BAREMETAL-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib"
-// CXX-RV32-BAREMETAL-ILP32: "-lstdc++" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
+// CXX-RV32-BAREMETAL-ILP32: "-lstdc++" "-lm" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
 // CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-linux-gnu/8.0.1{{/|}}crtend.o"
 
 // RUN: %clangxx -### %s -fuse-ld= \
@@ -73,7 +73,7 @@
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-linux-gnu/8.0.1{{/|}}crtbegin.o"
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-linux-gnu/8.0.1"
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-linux-gnu/8.0.1/../../..{{/|}}..{{/|}}riscv32-unknown-linux-gnu/lib"
-// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-lstdc++" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-lstdc++" "-lm" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-linux-gnu/8.0.1{{/|}}crtend.o"
 
 // RUN: env "PATH=" %clang -### %s -fuse-ld=ld -no-pie \
Index: clang/lib/Driver/ToolChains/RISCVToolchain.cpp
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -201,8 +201,11 @@
 
   if (!Args.hasArg(options::OPT_nostdlib) &&
   !Args.hasArg(options::OPT_nodefaultlibs)) {
-if (ToolChain.ShouldLinkCXXStdlib(Args))
-  ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
+if (D.CCCIsCXX()) {
+  if (ToolChain.ShouldLinkCXXStdlib(Args))
+ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
+  CmdArgs.push_back("-lm");
+}
 CmdArgs.push_back("--start-group");
 CmdArgs.push_back("-lc");
 CmdArgs.push_back("-lgloss");


Index: clang/test/Driver/riscv64-toolchain.c
===
--- 

[PATCH] D129065: [RISCV][Driver] Add libm linking to `RISCVToolchain` for C++

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

In D129065#3631052 , @jrtc27 wrote:

> Commit subject/body should say C++ in it

Sure, updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129065

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

[PATCH] D129065: [RISCV][Driver] Add libm linking to `RISCVToolchain`

[Anton Afanasyev via Phabricator via cfe-commits]

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

Update summary


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129065

Files:
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c


Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -58,7 +58,7 @@
 // CXX-RV64-BAREMETAL-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV64-BAREMETAL-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
 // CXX-RV64-BAREMETAL-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib"
-// CXX-RV64-BAREMETAL-LP64: "-lstdc++" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
+// CXX-RV64-BAREMETAL-LP64: "-lstdc++" "-lm" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
 // CXX-RV64-BAREMETAL-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: env "PATH=" %clangxx -### %s -fuse-ld= \
@@ -73,7 +73,7 @@
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv64-unknown-elf/lib"
-// CXX-RV64-BAREMETAL-NOSYSROOT-LP64: "-lstdc++" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
+// CXX-RV64-BAREMETAL-NOSYSROOT-LP64: "-lstdc++" "-lm" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: env "PATH=" %clang -### %s -fuse-ld= -no-pie \
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -58,7 +58,7 @@
 // CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV32-BAREMETAL-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1"
 // CXX-RV32-BAREMETAL-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib"
-// CXX-RV32-BAREMETAL-ILP32: "-lstdc++" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
+// CXX-RV32-BAREMETAL-ILP32: "-lstdc++" "-lm" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
 // CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: %clangxx -### %s -fuse-ld= \
@@ -73,7 +73,7 @@
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1"
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv32-unknown-elf/lib"
-// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-lstdc++" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-lstdc++" "-lm" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: env "PATH=" %clang -### %s -fuse-ld=ld -no-pie \
Index: clang/lib/Driver/ToolChains/RISCVToolchain.cpp
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -211,8 +211,11 @@
 
   if (!Args.hasArg(options::OPT_nostdlib) &&
   !Args.hasArg(options::OPT_nodefaultlibs)) {
-if (ToolChain.ShouldLinkCXXStdlib(Args))
-  ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
+if (D.CCCIsCXX()) {
+  if (ToolChain.ShouldLinkCXXStdlib(Args))
+ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
+  CmdArgs.push_back("-lm");
+}
 CmdArgs.push_back("--start-group");
 CmdArgs.push_back("-lc");
 CmdArgs.push_back("-lgloss");


Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -58,7 +58,7 @@
 // CXX-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
 // CXX-RV64-BAREMETAL-LP64: 

[PATCH] D129065: [RISCV][Driver] Add libm linking to `RISCVToolchain`

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev created this revision.
anton-afanasyev added reviewers: MaskRay, kito-cheng, asb.
Herald added subscribers: sunshaoce, VincentWu, luke957, StephenFan, vkmr, 
frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, 
benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, 
edward-jones, zzheng, jrtc27, shiva0217, niosHD, sabuasal, simoncook, 
johnrusso, rbar, arichardson.
Herald added a project: All.
anton-afanasyev requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD.
Herald added a project: clang.

GCC automatically links libm by adding `-lm` to linker command line,
add it to `RISCVToochain` as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129065

Files:
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c


Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -58,7 +58,7 @@
 // CXX-RV64-BAREMETAL-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV64-BAREMETAL-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
 // CXX-RV64-BAREMETAL-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib"
-// CXX-RV64-BAREMETAL-LP64: "-lstdc++" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
+// CXX-RV64-BAREMETAL-LP64: "-lstdc++" "-lm" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
 // CXX-RV64-BAREMETAL-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: env "PATH=" %clangxx -### %s -fuse-ld= \
@@ -73,7 +73,7 @@
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv64-unknown-elf/lib"
-// CXX-RV64-BAREMETAL-NOSYSROOT-LP64: "-lstdc++" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
+// CXX-RV64-BAREMETAL-NOSYSROOT-LP64: "-lstdc++" "-lm" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: env "PATH=" %clang -### %s -fuse-ld= -no-pie \
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -58,7 +58,7 @@
 // CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV32-BAREMETAL-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1"
 // CXX-RV32-BAREMETAL-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib"
-// CXX-RV32-BAREMETAL-ILP32: "-lstdc++" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
+// CXX-RV32-BAREMETAL-ILP32: "-lstdc++" "-lm" "--start-group" "-lc" "-lgloss" 
"--end-group" "-lgcc"
 // CXX-RV32-BAREMETAL-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: %clangxx -### %s -fuse-ld= \
@@ -73,7 +73,7 @@
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtbegin.o"
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1"
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv32-unknown-elf/lib"
-// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-lstdc++" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-lstdc++" "-lm" "--start-group" "-lc" 
"-lgloss" "--end-group" "-lgcc"
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: env "PATH=" %clang -### %s -fuse-ld=ld -no-pie \
Index: clang/lib/Driver/ToolChains/RISCVToolchain.cpp
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -211,8 +211,11 @@
 
   if (!Args.hasArg(options::OPT_nostdlib) &&
   !Args.hasArg(options::OPT_nodefaultlibs)) {
-if (ToolChain.ShouldLinkCXXStdlib(Args))
-  ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
+if (D.CCCIsCXX()) {
+  if (ToolChain.ShouldLinkCXXStdlib(Args))
+ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
+  CmdArgs.push_back("-lm");
+}
 CmdArgs.push_back("--start-group");

[PATCH] D128625: [RISCV][Driver] Fix baremetal `GCCInstallation` paths

[Anton Afanasyev via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/RISCVToolchain.cpp:54
+
+  // Set alias for "riscv{64|32}-unknown-unknown-elf"
+  SmallVector TripleAliases;

arichardson wrote:
> This seems like the wrong place to add this workaround, shouldn't the change 
> be in `GCCInstallation::init`? That way targets other than RISC-V also 
> benefit from this fix.
> 
`GCCInstallation` knows nothing about triple equivalence of the specific 
targets, but provides `TripleAliases` init variable for installation target 
callers. Other targets are to do the same initialization as we do here in case 
of different normalized and layout-used triple names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128625

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

[PATCH] D128625: [RISCV][Driver] Fix baremetal `GCCInstallation` paths

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

In D128625#3621030 , @MaskRay wrote:

> The description seems unclear to me. Is a `riscv64-unknown-linux-gnu` GCC 
> installation selected while the requested target triple is 
> `riscv64-unknown-elf`?

Yes, exactly (see precommitted tests 4ee6b7806bc0 
: adding 
one more target directory confuses driver, mixing triples in paths). Adjusting 
description to be more clear.

> This could be an instance of 
> https://discourse.llvm.org/t/rfc-fix-loose-behaviors-of-clang-target/60272 
> (`[RFC] Fix loose behaviors of Clang –target=`) and the right fix may be 
> somewhere upper level.

This looks related but not the same one: this patch fixes the incorrect use of 
completely different targets. I believe this issue could be addressed on top of 
your RFC implemented, need to refactor triple, setting equivalence of triples 
explicitly. Anyway, I'd like to have this patch committed meanwhile: it is more 
related to how `RISCVToolchain` uses baremetal `GCCInstallation` for the 
current state.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128625

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

[PATCH] D128625: [RISCV][Driver] Fix baremetal `GCCInstallation` paths

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev created this revision.
anton-afanasyev added reviewers: MaskRay, kito-cheng, asb.
Herald added subscribers: sunshaoce, VincentWu, luke957, StephenFan, vkmr, 
frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, 
benna, psnobl, abidh, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, 
rogfer01, edward-jones, zzheng, jrtc27, shiva0217, niosHD, sabuasal, simoncook, 
johnrusso, rbar, arichardson.
Herald added a project: All.
anton-afanasyev requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD.
Herald added a project: clang.

  In general gcc toolchain folder contains several targets, for instance,
  baremetal and Linux ones. The precommitted tests address this case by
  adding `riscv64-unknown-linux-gnu/` to toolchain folder. This breaks driver's
  include and lib paths, since riscv baremetal triple is now normalized to
  "riscv{32|64}-unknown-unknown-elf" rather than just to 
"riscv{32|64}-unknown-elf",
  and `GCCInstallation` uses this search priority: 
"riscv{32|64}-unknown-linux-gnu",
  "riscv{32|64}-linux-gnu", "riscv{32|64}-unknown-elf", choosing Linux target.
  
  This patch fixes this issue by passing triple alias for baremetal target.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128625

Files:
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c

Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -21,15 +21,15 @@
 // RUN:   --sysroot=%S/Inputs/basic_riscv64_tree/riscv64-unknown-elf 2>&1 -no-pie \
 // RUN:   | FileCheck -check-prefix=C-RV64-BAREMETAL-LP64 %s
 
-// C-RV64-BAREMETAL-LP64: "{{.*}}Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-linux-gnu/8.0.1/../../..{{/|}}..{{/|}}bin{{/|}}riscv64-unknown-elf-ld"
+// C-RV64-BAREMETAL-LP64: "{{.*}}Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}bin{{/|}}riscv64-unknown-elf-ld"
 // C-RV64-BAREMETAL-LP64: "--sysroot={{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf"
 // C-RV64-BAREMETAL-LP64-SAME: "-X"
 // C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib{{/|}}crt0.o"
-// C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-linux-gnu/8.0.1{{/|}}crtbegin.o"
-// C-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-linux-gnu/8.0.1"
+// C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
+// C-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
 // C-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib"
 // C-RV64-BAREMETAL-LP64: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
-// C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-linux-gnu/8.0.1{{/|}}crtend.o"
+// C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: env "PATH=" %clang -### %s -fuse-ld= \
 // RUN:   --target=riscv64-unknown-elf --rtlib=platform \
@@ -37,13 +37,13 @@
 // RUN:   --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-BAREMETAL-NOSYSROOT-LP64 %s
 
-// C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-linux-gnu/8.0.1/../../..{{/|}}..{{/|}}bin{{/|}}riscv64-unknown-elf-ld"
-// C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-linux-gnu/8.0.1/../../..{{/|}}..{{/|}}riscv64-unknown-linux-gnu/lib{{/|}}crt0.o"
-// C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-linux-gnu/8.0.1{{/|}}crtbegin.o"
-// C-RV64-BAREMETAL-NOSYSROOT-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-linux-gnu/8.0.1"
-// C-RV64-BAREMETAL-NOSYSROOT-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-linux-gnu/8.0.1/../../..{{/|}}..{{/|}}riscv64-unknown-linux-gnu{{/|}}lib"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}bin{{/|}}riscv64-unknown-elf-ld"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../..{{/|}}..{{/|}}riscv64-unknown-elf/lib{{/|}}crt0.o"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: 

[PATCH] D113179: [Passes] Move AggressiveInstCombine after InstCombine

[Anton Afanasyev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc34d157fc739: [Passes] Move AggressiveInstCombine after 
InstCombine (authored by anton-afanasyev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113179

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/Passes/PassBuilderPipelines.cpp
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll

Index: llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll
===
--- llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll
+++ llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll
@@ -6,16 +6,6 @@
 define i32 @main(i32 %a) {
 ; CHECK-LABEL: @main(
 ; CHECK-NEXT:  if.end:
-; CHECK-NEXT:[[TMP0:%.*]] = trunc i32 [[A:%.*]] to i8
-; CHECK-NEXT:[[TMP1:%.*]] = add i8 [[TMP0]], 1
-; CHECK-NEXT:[[CONV:%.*]] = and i8 [[TMP1]], 8
-; CHECK-NEXT:[[CMP_I_NOT:%.*]] = icmp eq i8 [[CONV]], 0
-; CHECK-NEXT:[[SHL_I:%.*]] = select i1 [[CMP_I_NOT]], i8 7, i8 0
-; CHECK-NEXT:[[COND_I:%.*]] = shl i8 [[CONV]], [[SHL_I]]
-; CHECK-NEXT:[[CONV1:%.*]] = sext i8 [[COND_I]] to i32
-; CHECK-NEXT:[[SEXT:%.*]] = mul i32 [[CONV1]], 1355350016
-; CHECK-NEXT:[[TOBOOL:%.*]] = icmp ne i32 [[SEXT]], 65536
-; CHECK-NEXT:tail call void @llvm.assume(i1 [[TOBOOL]])
 ; CHECK-NEXT:ret i32 0
 ;
   %inc = add nsw i32 %a, 1
Index: llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
@@ -95,8 +95,8 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O-NEXT: Running pass: InstCombinePass
+; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O1-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O2-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O3-NEXT: Running pass: LibCallsShrinkWrapPass
Index: llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
@@ -121,7 +121,6 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O-NEXT: Running pass: InstCombinePass
 ; CHECK-O-NEXT: Running analysis: BlockFrequencyAnalysis on foo
 ; These next two can appear in any order since they are accessed as parameters
@@ -129,6 +128,7 @@
 ; CHECK-O-DAG: Running analysis: LoopAnalysis on foo
 ; CHECK-O-DAG: Running analysis: BranchProbabilityAnalysis on foo
 ; CHECK-O-DAG: Running analysis: PostDominatorTreeAnalysis on foo
+; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O1-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O2-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O3-NEXT: Running pass: LibCallsShrinkWrapPass
Index: llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
@@ -100,8 +100,8 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O-NEXT: Running pass: InstCombinePass
+; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O1-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O2-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O3-NEXT: Running pass: LibCallsShrinkWrapPass
Index: llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
@@ -91,8 +91,8 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; 

[PATCH] D113179: [Passes] Move AggressiveInstCombine after InstCombine

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev updated this revision to Diff 391823.
anton-afanasyev added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Update tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113179

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/Passes/PassBuilderPipelines.cpp
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll

Index: llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll
===
--- llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll
+++ llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll
@@ -6,16 +6,6 @@
 define i32 @main(i32 %a) {
 ; CHECK-LABEL: @main(
 ; CHECK-NEXT:  if.end:
-; CHECK-NEXT:[[TMP0:%.*]] = trunc i32 [[A:%.*]] to i8
-; CHECK-NEXT:[[TMP1:%.*]] = add i8 [[TMP0]], 1
-; CHECK-NEXT:[[CONV:%.*]] = and i8 [[TMP1]], 8
-; CHECK-NEXT:[[CMP_I_NOT:%.*]] = icmp eq i8 [[CONV]], 0
-; CHECK-NEXT:[[SHL_I:%.*]] = select i1 [[CMP_I_NOT]], i8 7, i8 0
-; CHECK-NEXT:[[COND_I:%.*]] = shl i8 [[CONV]], [[SHL_I]]
-; CHECK-NEXT:[[CONV1:%.*]] = sext i8 [[COND_I]] to i32
-; CHECK-NEXT:[[SEXT:%.*]] = mul i32 [[CONV1]], 1355350016
-; CHECK-NEXT:[[TOBOOL:%.*]] = icmp ne i32 [[SEXT]], 65536
-; CHECK-NEXT:tail call void @llvm.assume(i1 [[TOBOOL]])
 ; CHECK-NEXT:ret i32 0
 ;
   %inc = add nsw i32 %a, 1
Index: llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
@@ -95,8 +95,8 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O-NEXT: Running pass: InstCombinePass
+; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O1-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O2-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O3-NEXT: Running pass: LibCallsShrinkWrapPass
Index: llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
@@ -121,7 +121,6 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O-NEXT: Running pass: InstCombinePass
 ; CHECK-O-NEXT: Running analysis: BlockFrequencyAnalysis on foo
 ; These next two can appear in any order since they are accessed as parameters
@@ -129,6 +128,7 @@
 ; CHECK-O-DAG: Running analysis: LoopAnalysis on foo
 ; CHECK-O-DAG: Running analysis: BranchProbabilityAnalysis on foo
 ; CHECK-O-DAG: Running analysis: PostDominatorTreeAnalysis on foo
+; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O1-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O2-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O3-NEXT: Running pass: LibCallsShrinkWrapPass
Index: llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
@@ -100,8 +100,8 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O-NEXT: Running pass: InstCombinePass
+; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O1-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O2-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O3-NEXT: Running pass: LibCallsShrinkWrapPass
Index: llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
@@ -91,8 +91,8 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O3-NEXT: Running pass: 

[PATCH] D108625: [Test][Time profiler] Fix test time checking

[Anton Afanasyev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGff780014b296: [Test][Time profiler] Fix test time checking 
(authored by anton-afanasyev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108625

Files:
  clang/test/Driver/check-time-trace-sections.py


Index: clang/test/Driver/check-time-trace-sections.py
===
--- clang/test/Driver/check-time-trace-sections.py
+++ clang/test/Driver/check-time-trace-sections.py
@@ -20,10 +20,8 @@
 beginning_of_time = log_contents["beginningOfTime"] / 100
 seconds_since_epoch = time.time()
 
-# Make sure that the 'beginningOfTime' is not earlier than 10 seconds ago
-# and not later than now.
-if beginning_of_time > seconds_since_epoch or \
-seconds_since_epoch - beginning_of_time > 10:
+# Make sure that the 'beginningOfTime' is not later than now.
+if beginning_of_time > seconds_since_epoch:
 sys.exit("'beginningOfTime' should represent the absolute time when the "
  "process has started")
 


Index: clang/test/Driver/check-time-trace-sections.py
===
--- clang/test/Driver/check-time-trace-sections.py
+++ clang/test/Driver/check-time-trace-sections.py
@@ -20,10 +20,8 @@
 beginning_of_time = log_contents["beginningOfTime"] / 100
 seconds_since_epoch = time.time()
 
-# Make sure that the 'beginningOfTime' is not earlier than 10 seconds ago
-# and not later than now.
-if beginning_of_time > seconds_since_epoch or \
-seconds_since_epoch - beginning_of_time > 10:
+# Make sure that the 'beginningOfTime' is not later than now.
+if beginning_of_time > seconds_since_epoch:
 sys.exit("'beginningOfTime' should represent the absolute time when the "
  "process has started")
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108625: [Test][Time profiler] Fix test time checking

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev created this revision.
anton-afanasyev added reviewers: broadwaylamb, russell.gallop.
anton-afanasyev requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This test sometimes triggers failures during build testing. For instance, see:
https://lab.llvm.org/buildbot/#/builders/52/builds/10161, details: 
https://lab.llvm.org/buildbot/#/builders/52/builds/10161/steps/5/logs/FAIL__Clang__check-time-trace-sections_cpp
 .
AFAICT the time between driver calling and checking its time tracker output
is not guaranteed to be stable and small:

  > head -2 check-time-trace-sections.cpp
  // 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
  > clang -S -ftime-trace -ftime-trace-granularity=0 -o /tmp/check 
check-time-trace-sections.cpp
  > cat /tmp/check.json | python check-time-trace-sections.py
  > sleep 10
  > cat /tmp/check.json | python check-time-trace-sections.py
  'beginningOfTime' should represent the absolute time when the process has 
started
  >

One can change "10 sec" value to something longer, but I believe
it's enough just to check that `beginningOfTime` exists and is
not later than current time.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108625

Files:
  clang/test/Driver/check-time-trace-sections.py


Index: clang/test/Driver/check-time-trace-sections.py
===
--- clang/test/Driver/check-time-trace-sections.py
+++ clang/test/Driver/check-time-trace-sections.py
@@ -20,10 +20,8 @@
 beginning_of_time = log_contents["beginningOfTime"] / 100
 seconds_since_epoch = time.time()
 
-# Make sure that the 'beginningOfTime' is not earlier than 10 seconds ago
-# and not later than now.
-if beginning_of_time > seconds_since_epoch or \
-seconds_since_epoch - beginning_of_time > 10:
+# Make sure that the 'beginningOfTime' is not later than now.
+if beginning_of_time > seconds_since_epoch:
 sys.exit("'beginningOfTime' should represent the absolute time when the "
  "process has started")
 


Index: clang/test/Driver/check-time-trace-sections.py
===
--- clang/test/Driver/check-time-trace-sections.py
+++ clang/test/Driver/check-time-trace-sections.py
@@ -20,10 +20,8 @@
 beginning_of_time = log_contents["beginningOfTime"] / 100
 seconds_since_epoch = time.time()
 
-# Make sure that the 'beginningOfTime' is not earlier than 10 seconds ago
-# and not later than now.
-if beginning_of_time > seconds_since_epoch or \
-seconds_since_epoch - beginning_of_time > 10:
+# Make sure that the 'beginningOfTime' is not later than now.
+if beginning_of_time > seconds_since_epoch:
 sys.exit("'beginningOfTime' should represent the absolute time when the "
  "process has started")
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68161: [TimeProfiler] Fix "OptModule" section and add new "Backend" sections

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev closed this revision.
anton-afanasyev added a comment.

Commited long time ago (https://reviews.llvm.org/rL373142).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68161



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

[PATCH] D71347: Add TimeTraceScope constructor without detail arg to simplify code where no detail required

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev accepted this revision.
anton-afanasyev added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71347



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

[PATCH] D69750: make -ftime-trace also trace time spent creating debug info

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev accepted this revision.
anton-afanasyev added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D69750



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

[PATCH] D68710: Remove time-trace message as it is inconsistent style

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev accepted this revision.
anton-afanasyev added a comment.
This revision is now accepted and ready to land.

Yes, I agree. LGTM.


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

https://reviews.llvm.org/D68710



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

[PATCH] D68260: Decrease the verbosity of the -ftime-trace optionAnd move the relevant information in the doc.

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev accepted this revision.
anton-afanasyev added a comment.
This revision is now accepted and ready to land.

I believe it's better solution, LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68260



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

[PATCH] D68161: [TimeProfiler] Fix "OptModule" section and add new "Backend" sections

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

In D68161#1686632 , @lebedev.ri wrote:

> Oh nice!
>  F10096481: image.png 
>  F10096488: image.png 
>  I like this.
>
> One more related question - there are two `"Frontend"` sections, second one 
> appears to consist only of `"CodeGen Function"s`.
>  Is that correct? Should they both be in an actual front-end section, and 
> should they be renamed to, i dunno,
>  `"Sema"` and `"Codegen"`? I'm not sure about this, just raising a question.


Ok, committing this.

As for two `"Frontend"` sections -- we've discussed this several months ago 
(https://reviews.llvm.org/D63325#1544555), it is not obvious how to join these 
sections to one (start and finish of such section are separated a lot and the 
code for joining is opaque).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68161



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/lib/CodeGen/BackendUtil.cpp:1426-1431
   EmitAssemblyHelper AsmHelper(Diags, HeaderOpts, CGOpts, TOpts, LOpts, M);
 
   if (CGOpts.ExperimentalNewPassManager)
 AsmHelper.EmitAssemblyWithNewPassManager(Action, std::move(OS));
   else
 AsmHelper.EmitAssembly(Action, std::move(OS));

anton-afanasyev wrote:
> lebedev.ri wrote:
> > anton-afanasyev wrote:
> > > lebedev.ri wrote:
> > > > anton-afanasyev wrote:
> > > > > lebedev.ri wrote:
> > > > > > anton-afanasyev wrote:
> > > > > > > anton-afanasyev wrote:
> > > > > > > > lebedev.ri wrote:
> > > > > > > > > This isn't covered by any timer; if you look in 
> > > > > > > > > `BackendUtil.cpp`,
> > > > > > > > > `EmitAssemblyHelper` actually has 
> > > > > > > > > `CodeGenerationTime("codegen", "Code Generation Time")` timer.
> > > > > > > > Thanks, I'm to add it.
> > > > > > > Hmm, I've figured out this isn't needed: such new timer mostly 
> > > > > > > coincides with "Backend" timer (above). Legacy `Timer 
> > > > > > > CodeGenerationTime(...)` is bad example of doing right timing.
> > > > > > "Mostly coincides" may not be the best way to approach fine-grained 
> > > > > > timings, i think? :)
> > > > > > 
> > > > > > I have noticed this because when i looked at the produced time 
> > > > > > flame graph,
> > > > > > there's large section in the end that is covered only by 
> > > > > > `"Backend"` timer,
> > > > > > but nothing else. Now, i'm not going to say whether or not that 
> > > > > > extra section
> > > > > > should or should not be within `"Backend"` timer, but it certainly 
> > > > > > should *also*
> > > > > > be within `"codegen"` timer. Or is there no codegen time spent 
> > > > > > there?
> > > > > > {F10062322}
> > > > > > {F10062316}
> > > > > "Mostly coincides" here means "identical" I believe, the difference 
> > > > > is auxiliary stuff.
> > > > > Please look at `clang::EmitBackendOutput()`, `"Backend"` timer is 
> > > > > outer for `"codegen"` one.
> > > > Then we are talking about different things using the same name.
> > > > There are two distinct codegen steps:
> > > > 1. clang AST -> LLVM IR codegen
> > > > (after that, all the opt passes run)
> > > > 2. LLVM IR -> final assembly. This happens after all the opt middle-end 
> > > > passes.
> > > > 
> > > > Those are *different* codegen's.
> > > Yes, and step 1 is named as "CodeGen Function" whereas step 2 is named 
> > > just "Backend".
> > Aha. So this is what i //expected// to see, apparently: 
> > {F10063128} {F10063119}
> > ```
> > diff --git a/clang/lib/CodeGen/BackendUtil.cpp 
> > b/clang/lib/CodeGen/BackendUtil.cpp
> > index 71ae8fd4fb0..d89d12612f8 100644
> > --- a/clang/lib/CodeGen/BackendUtil.cpp
> > +++ b/clang/lib/CodeGen/BackendUtil.cpp
> > @@ -910,6 +910,7 @@ void EmitAssemblyHelper::EmitAssembly(BackendAction 
> > Action,
> >  
> >{
> >  PrettyStackTraceString CrashInfo("Code generation");
> > +llvm::TimeTraceScope TimeScope("BackendCodegen", StringRef(""));
> >  CodeGenPasses.run(*TheModule);
> >}
> > ```
> > 
> > But that actually brings me to another question - what about 
> > `PrettyStackTraceString CrashInfo("Per-function optimization");`?
> > Should that be wrapped into some section, too?
> > I'm less certain here.
> Ok, you've talked about `Timer CodeGenerationTime`, which corresponds to 
> `Backend` scope.
> As for this `BackendCodegen`, adding this I would prefer to add adjacent 
> `"Per-function optimization"` and `"Per-module optimization passes"` together.
Changes are here: https://reviews.llvm.org/D68161


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58675



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

[PATCH] D68161: [TimeProfiler] Fix "OptModule" section and add new "Backend" sections

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev created this revision.
anton-afanasyev added a reviewer: lebedev.ri.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, mehdi_amini.
Herald added projects: clang, LLVM.

Remove unnecessary "OptModule" section. Add "PerFunctionPasses", 
"PerModulePasses" and "CodeGenPasses" sections under "Backend" section.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68161

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/lib/IR/LegacyPassManager.cpp


Index: llvm/lib/IR/LegacyPassManager.cpp
===
--- llvm/lib/IR/LegacyPassManager.cpp
+++ llvm/lib/IR/LegacyPassManager.cpp
@@ -1680,7 +1680,6 @@
 bool FPPassManager::runOnModule(Module ) {
   bool Changed = false;
 
-  llvm::TimeTraceScope TimeScope("OptModule", M.getName());
   for (Function  : M)
 Changed |= runOnFunction(F);
 
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -894,6 +894,7 @@
 
   {
 PrettyStackTraceString CrashInfo("Per-function optimization");
+llvm::TimeTraceScope TimeScope("PerFunctionPasses", StringRef(""));
 
 PerFunctionPasses.doInitialization();
 for (Function  : *TheModule)
@@ -904,11 +905,13 @@
 
   {
 PrettyStackTraceString CrashInfo("Per-module optimization passes");
+llvm::TimeTraceScope TimeScope("PerModulePasses", StringRef(""));
 PerModulePasses.run(*TheModule);
   }
 
   {
 PrettyStackTraceString CrashInfo("Code generation");
+llvm::TimeTraceScope TimeScope("CodeGenPasses", StringRef(""));
 CodeGenPasses.run(*TheModule);
   }
 


Index: llvm/lib/IR/LegacyPassManager.cpp
===
--- llvm/lib/IR/LegacyPassManager.cpp
+++ llvm/lib/IR/LegacyPassManager.cpp
@@ -1680,7 +1680,6 @@
 bool FPPassManager::runOnModule(Module ) {
   bool Changed = false;
 
-  llvm::TimeTraceScope TimeScope("OptModule", M.getName());
   for (Function  : M)
 Changed |= runOnFunction(F);
 
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -894,6 +894,7 @@
 
   {
 PrettyStackTraceString CrashInfo("Per-function optimization");
+llvm::TimeTraceScope TimeScope("PerFunctionPasses", StringRef(""));
 
 PerFunctionPasses.doInitialization();
 for (Function  : *TheModule)
@@ -904,11 +905,13 @@
 
   {
 PrettyStackTraceString CrashInfo("Per-module optimization passes");
+llvm::TimeTraceScope TimeScope("PerModulePasses", StringRef(""));
 PerModulePasses.run(*TheModule);
   }
 
   {
 PrettyStackTraceString CrashInfo("Code generation");
+llvm::TimeTraceScope TimeScope("CodeGenPasses", StringRef(""));
 CodeGenPasses.run(*TheModule);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/lib/CodeGen/BackendUtil.cpp:1426-1431
   EmitAssemblyHelper AsmHelper(Diags, HeaderOpts, CGOpts, TOpts, LOpts, M);
 
   if (CGOpts.ExperimentalNewPassManager)
 AsmHelper.EmitAssemblyWithNewPassManager(Action, std::move(OS));
   else
 AsmHelper.EmitAssembly(Action, std::move(OS));

lebedev.ri wrote:
> anton-afanasyev wrote:
> > lebedev.ri wrote:
> > > anton-afanasyev wrote:
> > > > lebedev.ri wrote:
> > > > > anton-afanasyev wrote:
> > > > > > anton-afanasyev wrote:
> > > > > > > lebedev.ri wrote:
> > > > > > > > This isn't covered by any timer; if you look in 
> > > > > > > > `BackendUtil.cpp`,
> > > > > > > > `EmitAssemblyHelper` actually has 
> > > > > > > > `CodeGenerationTime("codegen", "Code Generation Time")` timer.
> > > > > > > Thanks, I'm to add it.
> > > > > > Hmm, I've figured out this isn't needed: such new timer mostly 
> > > > > > coincides with "Backend" timer (above). Legacy `Timer 
> > > > > > CodeGenerationTime(...)` is bad example of doing right timing.
> > > > > "Mostly coincides" may not be the best way to approach fine-grained 
> > > > > timings, i think? :)
> > > > > 
> > > > > I have noticed this because when i looked at the produced time flame 
> > > > > graph,
> > > > > there's large section in the end that is covered only by `"Backend"` 
> > > > > timer,
> > > > > but nothing else. Now, i'm not going to say whether or not that extra 
> > > > > section
> > > > > should or should not be within `"Backend"` timer, but it certainly 
> > > > > should *also*
> > > > > be within `"codegen"` timer. Or is there no codegen time spent there?
> > > > > {F10062322}
> > > > > {F10062316}
> > > > "Mostly coincides" here means "identical" I believe, the difference is 
> > > > auxiliary stuff.
> > > > Please look at `clang::EmitBackendOutput()`, `"Backend"` timer is outer 
> > > > for `"codegen"` one.
> > > Then we are talking about different things using the same name.
> > > There are two distinct codegen steps:
> > > 1. clang AST -> LLVM IR codegen
> > > (after that, all the opt passes run)
> > > 2. LLVM IR -> final assembly. This happens after all the opt middle-end 
> > > passes.
> > > 
> > > Those are *different* codegen's.
> > Yes, and step 1 is named as "CodeGen Function" whereas step 2 is named just 
> > "Backend".
> Aha. So this is what i //expected// to see, apparently: 
> {F10063128} {F10063119}
> ```
> diff --git a/clang/lib/CodeGen/BackendUtil.cpp 
> b/clang/lib/CodeGen/BackendUtil.cpp
> index 71ae8fd4fb0..d89d12612f8 100644
> --- a/clang/lib/CodeGen/BackendUtil.cpp
> +++ b/clang/lib/CodeGen/BackendUtil.cpp
> @@ -910,6 +910,7 @@ void EmitAssemblyHelper::EmitAssembly(BackendAction 
> Action,
>  
>{
>  PrettyStackTraceString CrashInfo("Code generation");
> +llvm::TimeTraceScope TimeScope("BackendCodegen", StringRef(""));
>  CodeGenPasses.run(*TheModule);
>}
> ```
> 
> But that actually brings me to another question - what about 
> `PrettyStackTraceString CrashInfo("Per-function optimization");`?
> Should that be wrapped into some section, too?
> I'm less certain here.
Ok, you've talked about `Timer CodeGenerationTime`, which corresponds to 
`Backend` scope.
As for this `BackendCodegen`, adding this I would prefer to add adjacent 
`"Per-function optimization"` and `"Per-module optimization passes"` together.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/lib/CodeGen/BackendUtil.cpp:1426-1431
   EmitAssemblyHelper AsmHelper(Diags, HeaderOpts, CGOpts, TOpts, LOpts, M);
 
   if (CGOpts.ExperimentalNewPassManager)
 AsmHelper.EmitAssemblyWithNewPassManager(Action, std::move(OS));
   else
 AsmHelper.EmitAssembly(Action, std::move(OS));

lebedev.ri wrote:
> anton-afanasyev wrote:
> > lebedev.ri wrote:
> > > anton-afanasyev wrote:
> > > > anton-afanasyev wrote:
> > > > > lebedev.ri wrote:
> > > > > > This isn't covered by any timer; if you look in `BackendUtil.cpp`,
> > > > > > `EmitAssemblyHelper` actually has `CodeGenerationTime("codegen", 
> > > > > > "Code Generation Time")` timer.
> > > > > Thanks, I'm to add it.
> > > > Hmm, I've figured out this isn't needed: such new timer mostly 
> > > > coincides with "Backend" timer (above). Legacy `Timer 
> > > > CodeGenerationTime(...)` is bad example of doing right timing.
> > > "Mostly coincides" may not be the best way to approach fine-grained 
> > > timings, i think? :)
> > > 
> > > I have noticed this because when i looked at the produced time flame 
> > > graph,
> > > there's large section in the end that is covered only by `"Backend"` 
> > > timer,
> > > but nothing else. Now, i'm not going to say whether or not that extra 
> > > section
> > > should or should not be within `"Backend"` timer, but it certainly should 
> > > *also*
> > > be within `"codegen"` timer. Or is there no codegen time spent there?
> > > {F10062322}
> > > {F10062316}
> > "Mostly coincides" here means "identical" I believe, the difference is 
> > auxiliary stuff.
> > Please look at `clang::EmitBackendOutput()`, `"Backend"` timer is outer for 
> > `"codegen"` one.
> Then we are talking about different things using the same name.
> There are two distinct codegen steps:
> 1. clang AST -> LLVM IR codegen
> (after that, all the opt passes run)
> 2. LLVM IR -> final assembly. This happens after all the opt middle-end 
> passes.
> 
> Those are *different* codegen's.
Yes, and step 1 is named as "CodeGen Function" whereas step 2 is named just 
"Backend".


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/lib/CodeGen/BackendUtil.cpp:1426-1431
   EmitAssemblyHelper AsmHelper(Diags, HeaderOpts, CGOpts, TOpts, LOpts, M);
 
   if (CGOpts.ExperimentalNewPassManager)
 AsmHelper.EmitAssemblyWithNewPassManager(Action, std::move(OS));
   else
 AsmHelper.EmitAssembly(Action, std::move(OS));

lebedev.ri wrote:
> anton-afanasyev wrote:
> > anton-afanasyev wrote:
> > > lebedev.ri wrote:
> > > > This isn't covered by any timer; if you look in `BackendUtil.cpp`,
> > > > `EmitAssemblyHelper` actually has `CodeGenerationTime("codegen", "Code 
> > > > Generation Time")` timer.
> > > Thanks, I'm to add it.
> > Hmm, I've figured out this isn't needed: such new timer mostly coincides 
> > with "Backend" timer (above). Legacy `Timer CodeGenerationTime(...)` is bad 
> > example of doing right timing.
> "Mostly coincides" may not be the best way to approach fine-grained timings, 
> i think? :)
> 
> I have noticed this because when i looked at the produced time flame graph,
> there's large section in the end that is covered only by `"Backend"` timer,
> but nothing else. Now, i'm not going to say whether or not that extra section
> should or should not be within `"Backend"` timer, but it certainly should 
> *also*
> be within `"codegen"` timer. Or is there no codegen time spent there?
> {F10062322}
> {F10062316}
"Mostly coincides" here means "identical" I believe, the difference is 
auxiliary stuff.
Please look at `clang::EmitBackendOutput()`, `"Backend"` timer is outer for 
`"codegen"` one.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/lib/CodeGen/BackendUtil.cpp:1426-1431
   EmitAssemblyHelper AsmHelper(Diags, HeaderOpts, CGOpts, TOpts, LOpts, M);
 
   if (CGOpts.ExperimentalNewPassManager)
 AsmHelper.EmitAssemblyWithNewPassManager(Action, std::move(OS));
   else
 AsmHelper.EmitAssembly(Action, std::move(OS));

anton-afanasyev wrote:
> lebedev.ri wrote:
> > This isn't covered by any timer; if you look in `BackendUtil.cpp`,
> > `EmitAssemblyHelper` actually has `CodeGenerationTime("codegen", "Code 
> > Generation Time")` timer.
> Thanks, I'm to add it.
Hmm, I've figured out this isn't needed: such new timer mostly coincides with 
"Backend" timer (above). Legacy `Timer CodeGenerationTime(...)` is bad example 
of doing right timing.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/lib/CodeGen/BackendUtil.cpp:1426-1431
   EmitAssemblyHelper AsmHelper(Diags, HeaderOpts, CGOpts, TOpts, LOpts, M);
 
   if (CGOpts.ExperimentalNewPassManager)
 AsmHelper.EmitAssemblyWithNewPassManager(Action, std::move(OS));
   else
 AsmHelper.EmitAssembly(Action, std::move(OS));

lebedev.ri wrote:
> This isn't covered by any timer; if you look in `BackendUtil.cpp`,
> `EmitAssemblyHelper` actually has `CodeGenerationTime("codegen", "Code 
> Generation Time")` timer.
Thanks, I'm to add it.



Comment at: llvm/trunk/lib/IR/LegacyPassManager.cpp:1686
 
+  llvm::TimeTraceScope TimeScope("OptModule", M.getName());
   for (Function  : M)

lebedev.ri wrote:
> I think this may be the wrong place for this.
> This includes the entirety of the pipeline, including all of llvm back-end 
> stuff.
Ok, I'm just to delete this line, this block is outputted by 
`MPPassManager::runOnModule()` as well. This is just a section to see module 
name rather than to cover proper part of pipeline.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58675



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

[PATCH] D66873: [Test][Time profiler] Fix test for python3

[Anton Afanasyev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370300: [Test][Time profiler] Fix test for python3 (authored 
by anton-afanasyev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66873?vs=217613=217780#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66873

Files:
  cfe/trunk/test/Driver/check-time-trace-sections.py


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
@@ -12,9 +12,9 @@
 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)
+codegens = [event for event in events if event["name"] == "CodeGen Function"]
+frontends = [event for event in events if event["name"] == "Frontend"]
+backends = [event for event in events if event["name"] == "Backend"]
 
 if not all([any([is_inside(codegen, frontend) for frontend in frontends])
 for codegen in codegens]):


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
@@ -12,9 +12,9 @@
 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)
+codegens = [event for event in events if event["name"] == "CodeGen Function"]
+frontends = [event for event in events if event["name"] == "Frontend"]
+backends = [event for event in events if event["name"] == "Backend"]
 
 if not all([any([is_inside(codegen, frontend) for frontend in frontends])
 for codegen in codegens]):
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66873: [Test][Time profiler] Fix test for python3

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

Could you please give LGTM?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66873



___
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, @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] D66873: [Test][Time profiler] Fix test for python3

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev created this revision.
anton-afanasyev added reviewers: mgehre, nathanchance.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix test checking time profiler generates correct tracing json-file.
`filter` works differently for python2 and python3, so unifying this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66873

Files:
  clang/test/Driver/check-time-trace-sections.py


Index: clang/test/Driver/check-time-trace-sections.py
===
--- clang/test/Driver/check-time-trace-sections.py
+++ clang/test/Driver/check-time-trace-sections.py
@@ -12,9 +12,9 @@
 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)
+codegens = [event for event in events if event["name"] == "CodeGen Function"]
+frontends = [event for event in events if event["name"] == "Frontend"]
+backends = [event for event in events if event["name"] == "Backend"]
 
 if not all([any([is_inside(codegen, frontend) for frontend in frontends])
 for codegen in codegens]):


Index: clang/test/Driver/check-time-trace-sections.py
===
--- clang/test/Driver/check-time-trace-sections.py
+++ clang/test/Driver/check-time-trace-sections.py
@@ -12,9 +12,9 @@
 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)
+codegens = [event for event in events if event["name"] == "CodeGen Function"]
+frontends = [event for event in events if event["name"] == "Frontend"]
+backends = [event for event in events if event["name"] == "Backend"]
 
 if not all([any([is_inside(codegen, frontend) for frontend in frontends])
 for codegen in codegens]):
___
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

[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

[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

[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

[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] D65202: [Support] Fix `-ftime-trace-granularity` option

[Anton Afanasyev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366911: [Support] Fix `-ftime-trace-granularity` option 
(authored by anton-afanasyev, committed by ).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D65202?vs=211489=211510#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65202

Files:
  cfe/trunk/docs/ClangCommandLineReference.rst
  cfe/trunk/include/clang/Basic/CodeGenOptions.def
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Frontend/FrontendOptions.h
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/test/Driver/check-time-trace.cpp
  cfe/trunk/tools/driver/cc1_main.cpp
  llvm/trunk/include/llvm/Support/TimeProfiler.h
  llvm/trunk/lib/Support/TimeProfiler.cpp

Index: llvm/trunk/include/llvm/Support/TimeProfiler.h
===
--- llvm/trunk/include/llvm/Support/TimeProfiler.h
+++ llvm/trunk/include/llvm/Support/TimeProfiler.h
@@ -19,7 +19,7 @@
 /// Initialize the time trace profiler.
 /// This sets up the global \p TimeTraceProfilerInstance
 /// variable to be the profiler instance.
-void timeTraceProfilerInitialize();
+void timeTraceProfilerInitialize(unsigned TimeTraceGranularity);
 
 /// Cleanup the time trace profiler, if it was initialized.
 void timeTraceProfilerCleanup();
Index: llvm/trunk/lib/Support/TimeProfiler.cpp
===
--- llvm/trunk/lib/Support/TimeProfiler.cpp
+++ llvm/trunk/lib/Support/TimeProfiler.cpp
@@ -24,12 +24,6 @@
 
 namespace llvm {
 
-static cl::opt TimeTraceGranularity(
-"time-trace-granularity",
-cl::desc(
-"Minimum time granularity (in microseconds) traced by time profiler"),
-cl::init(500));
-
 TimeTraceProfiler *TimeTraceProfilerInstance = nullptr;
 
 typedef duration DurationType;
@@ -161,12 +155,16 @@
   SmallVector Entries;
   StringMap CountAndTotalPerName;
   time_point StartTime;
+
+  // Minimum time granularity (in microseconds)
+  unsigned TimeTraceGranularity;
 };
 
-void timeTraceProfilerInitialize() {
+void timeTraceProfilerInitialize(unsigned TimeTraceGranularity) {
   assert(TimeTraceProfilerInstance == nullptr &&
  "Profiler should not be initialized");
   TimeTraceProfilerInstance = new TimeTraceProfiler();
+  TimeTraceProfilerInstance->TimeTraceGranularity = TimeTraceGranularity;
 }
 
 void timeTraceProfilerCleanup() {
Index: cfe/trunk/tools/driver/cc1_main.cpp
===
--- cfe/trunk/tools/driver/cc1_main.cpp
+++ cfe/trunk/tools/driver/cc1_main.cpp
@@ -216,9 +216,10 @@
   bool Success = CompilerInvocation::CreateFromArgs(
   Clang->getInvocation(), Argv.begin(), Argv.end(), Diags);
 
-  if (Clang->getFrontendOpts().TimeTrace)
-llvm::timeTraceProfilerInitialize();
-
+  if (Clang->getFrontendOpts().TimeTrace) {
+llvm::timeTraceProfilerInitialize(
+Clang->getFrontendOpts().TimeTraceGranularity);
+  }
   // --print-supported-cpus takes priority over the actual compilation.
   if (Clang->getFrontendOpts().PrintSupportedCPUs)
 return PrintSupportedCPUs(Clang->getTargetOpts().Triple);
Index: cfe/trunk/docs/ClangCommandLineReference.rst
===
--- cfe/trunk/docs/ClangCommandLineReference.rst
+++ cfe/trunk/docs/ClangCommandLineReference.rst
@@ -1944,6 +1944,14 @@
 
 .. option:: -ftime-report
 
+.. option:: -ftime-trace
+
+Turn on time profiler
+
+.. option:: -ftime-trace-granularity=
+
+Minimum time granularity (in microseconds) traced by time profiler
+
 .. option:: -ftls-model=
 
 .. option:: -ftrap-function=
Index: cfe/trunk/include/clang/Basic/CodeGenOptions.def
===
--- cfe/trunk/include/clang/Basic/CodeGenOptions.def
+++ cfe/trunk/include/clang/Basic/CodeGenOptions.def
@@ -225,6 +225,8 @@
 CODEGENOPT(StrictVTablePointers, 1, 0) ///< Optimize based on the strict vtable pointers
 CODEGENOPT(TimePasses, 1, 0) ///< Set when -ftime-report is enabled.
 CODEGENOPT(TimeTrace , 1, 0) ///< Set when -ftime-trace is enabled.
+VALUE_CODEGENOPT(TimeTraceGranularity, 32, 500) ///< Minimum time granularity (in microseconds),
+   ///< traced by time profiler
 CODEGENOPT(UnrollLoops   , 1, 0) ///< Control whether loops are unrolled.
 CODEGENOPT(RerollLoops   , 1, 0) ///< Control whether loops are rerolled.
 CODEGENOPT(NoUseJumpTables   , 1, 0) ///< Set when -fno-jump-tables is enabled.
Index: cfe/trunk/include/clang/Frontend/FrontendOptions.h
===
--- cfe/trunk/include/clang/Frontend/FrontendOptions.h
+++ cfe/trunk/include/clang/Frontend/FrontendOptions.h
@@ -451,6 +451,9 @@
 

[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] D65202: [Support] Fix `-ftime-trace-granularity` option

[Anton Afanasyev via Phabricator via cfe-commits]

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



Comment at: clang/docs/ClangCommandLineReference.rst:1951
+
+Minimum time granularity (in microseconds) traced by time profiler
+

anton-afanasyev wrote:
> sammccall wrote:
> > is there any possibility of wanting a granularity <1ms in the future? This 
> > sets up a backward-compatibility trap if so.
> Does it really make sense? One can use `-ftime-trace-granularity=0`, which 
> can show all events.
To be more correct, this granularity value (`-ftime-trace-granularity=0`) will 
work after this old patch https://reviews.llvm.org/D63325 is commited.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65202



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

[PATCH] D65202: [Support] Fix `-ftime-trace-granularity` option

[Anton Afanasyev via Phabricator via cfe-commits]

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

Remove default repeat


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65202

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/TimeProfiler.h
  llvm/lib/Support/TimeProfiler.cpp

Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -24,12 +24,6 @@
 
 namespace llvm {
 
-static cl::opt TimeTraceGranularity(
-"time-trace-granularity",
-cl::desc(
-"Minimum time granularity (in microseconds) traced by time profiler"),
-cl::init(500));
-
 TimeTraceProfiler *TimeTraceProfilerInstance = nullptr;
 
 typedef duration DurationType;
@@ -161,12 +155,16 @@
   SmallVector Entries;
   StringMap CountAndTotalPerName;
   time_point StartTime;
+
+  // Minimum time granularity (in microseconds)
+  unsigned TimeTraceGranularity;
 };
 
-void timeTraceProfilerInitialize() {
+void timeTraceProfilerInitialize(unsigned TimeTraceGranularity) {
   assert(TimeTraceProfilerInstance == nullptr &&
  "Profiler should not be initialized");
   TimeTraceProfilerInstance = new TimeTraceProfiler();
+  TimeTraceProfilerInstance->TimeTraceGranularity = TimeTraceGranularity;
 }
 
 void timeTraceProfilerCleanup() {
Index: llvm/include/llvm/Support/TimeProfiler.h
===
--- llvm/include/llvm/Support/TimeProfiler.h
+++ llvm/include/llvm/Support/TimeProfiler.h
@@ -19,7 +19,7 @@
 /// Initialize the time trace profiler.
 /// This sets up the global \p TimeTraceProfilerInstance
 /// variable to be the profiler instance.
-void timeTraceProfilerInitialize();
+void timeTraceProfilerInitialize(unsigned TimeTraceGranularity);
 
 /// Cleanup the time trace profiler, if it was initialized.
 void timeTraceProfilerCleanup();
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -216,9 +216,10 @@
   bool Success = CompilerInvocation::CreateFromArgs(
   Clang->getInvocation(), Argv.begin(), Argv.end(), Diags);
 
-  if (Clang->getFrontendOpts().TimeTrace)
-llvm::timeTraceProfilerInitialize();
-
+  if (Clang->getFrontendOpts().TimeTrace) {
+llvm::timeTraceProfilerInitialize(
+Clang->getFrontendOpts().TimeTraceGranularity);
+  }
   // --print-supported-cpus takes priority over the actual compilation.
   if (Clang->getFrontendOpts().PrintSupportedCPUs)
 return PrintSupportedCPUs(Clang->getTargetOpts().Triple);
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -1,5 +1,5 @@
 // REQUIRES: shell
-// RUN: %clangxx -S -ftime-trace -mllvm --time-trace-granularity=0 -o %T/check-time-trace %s
+// RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o %T/check-time-trace %s
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
 // RUN:   | FileCheck %s
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1769,6 +1769,8 @@
   Opts.ShowTimers = Args.hasArg(OPT_ftime_report);
   Opts.PrintSupportedCPUs = Args.hasArg(OPT_print_supported_cpus);
   Opts.TimeTrace = Args.hasArg(OPT_ftime_trace);
+  Opts.TimeTraceGranularity = getLastArgIntValue(
+  Args, OPT_ftime_trace_granularity_EQ, Opts.TimeTraceGranularity, Diags);
   Opts.ShowVersion = Args.hasArg(OPT_version);
   Opts.ASTMergeFiles = Args.getAllArgValues(OPT_ast_merge);
   Opts.LLVMArgs = Args.getAllArgValues(OPT_mllvm);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4575,6 +4575,7 @@
   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_parseable_fixits);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_report);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_trace);
+  Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_granularity_EQ);
   Args.AddLastArg(CmdArgs, options::OPT_ftrapv);
   Args.AddLastArg(CmdArgs, options::OPT_malign_double);
 
Index: 

[PATCH] D65202: [Support] Fix `-ftime-trace-granularity` option

[Anton Afanasyev via Phabricator via cfe-commits]

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



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1772
   Opts.TimeTrace = Args.hasArg(OPT_ftime_trace);
+  Opts.TimeTraceGranularity =
+  getLastArgIntValue(Args, OPT_ftime_trace_granularity_EQ, 500, Diags);

sammccall wrote:
> You've got the default repeated here. Is it possible to set this 
> conditionally here, or use the existing value as default like
> 
> `Opts.TTG = getLastArgIntValue(Args, OPT_fttg_EQ, Opts.TTG, Diags)`
Ok, done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65202



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

[PATCH] D65202: [Support] Fix `-ftime-trace-granularity` option

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev marked an inline comment as done.
anton-afanasyev added a subscriber: aras-p.
anton-afanasyev added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:1943-1947
 .. option:: -ftime-report
 
+.. option:: -ftime-trace
+
+Turn on time profiler

lebedev.ri wrote:
> lebedev.ri wrote:
> > While there add a description to `-ftime-report` and document how they are 
> > different?
> I mean, state that one is chrome json trace and another is console table 
> output
The difference is actually deeper at the moment. `-ftime-report` output is 
buggy, short and uninformative (as noted by @aras-p in his blog post 
http://aras-p.info/blog/2019/01/12/Investigating-compile-times-and-Clang-ftime-report/).
 I would like just to remove `-ftime-report` option as obsolete and unsupported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65202



___
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] D65202: [Support] Fix `-ftime-trace-granularity` option

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:1951
+
+Minimum time granularity (in microseconds) traced by time profiler
+

sammccall wrote:
> is there any possibility of wanting a granularity <1ms in the future? This 
> sets up a backward-compatibility trap if so.
Does it really make sense? One can use `-ftime-trace-granularity=0`, which can 
show all events.



Comment at: llvm/lib/Support/TimeProfiler.cpp:27-30
+// Minimum time granularity (in microseconds) traced by time profiler
+unsigned TimeTraceGranularity = 500;
 
 TimeTraceProfiler *TimeTraceProfilerInstance = nullptr;

lebedev.ri wrote:
> Can `TimeTraceGranularity` perhaps be stored in `TimeTraceProfilerInstance`?
Sure, thanks.



Comment at: llvm/lib/Support/TimeProfiler.cpp:28
+// Minimum time granularity (in microseconds) traced by time profiler
+unsigned TimeTraceGranularity = 500;
 

sammccall wrote:
> sammccall wrote:
> > why does this have a default value?  It shouldn't be possible to use it 
> > without overwriting it, IIUC
> static
Moved to `struct TimeProfiler` member.



Comment at: llvm/lib/Support/TimeProfiler.cpp:28
+// Minimum time granularity (in microseconds) traced by time profiler
+unsigned TimeTraceGranularity = 500;
 

anton-afanasyev wrote:
> sammccall wrote:
> > sammccall wrote:
> > > why does this have a default value?  It shouldn't be possible to use it 
> > > without overwriting it, IIUC
> > static
> Moved to `struct TimeProfiler` member.
Hmm, ok, removing default value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65202



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

[PATCH] D65202: [Support] Fix `-ftime-trace-granularity` option

[Anton Afanasyev via Phabricator via cfe-commits]

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

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65202

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/TimeProfiler.h
  llvm/lib/Support/TimeProfiler.cpp

Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -24,12 +24,6 @@
 
 namespace llvm {
 
-static cl::opt TimeTraceGranularity(
-"time-trace-granularity",
-cl::desc(
-"Minimum time granularity (in microseconds) traced by time profiler"),
-cl::init(500));
-
 TimeTraceProfiler *TimeTraceProfilerInstance = nullptr;
 
 typedef duration DurationType;
@@ -161,12 +155,16 @@
   SmallVector Entries;
   StringMap CountAndTotalPerName;
   time_point StartTime;
+
+  // Minimum time granularity (in microseconds)
+  unsigned TimeTraceGranularity;
 };
 
-void timeTraceProfilerInitialize() {
+void timeTraceProfilerInitialize(unsigned TimeTraceGranularity) {
   assert(TimeTraceProfilerInstance == nullptr &&
  "Profiler should not be initialized");
   TimeTraceProfilerInstance = new TimeTraceProfiler();
+  TimeTraceProfilerInstance->TimeTraceGranularity = TimeTraceGranularity;
 }
 
 void timeTraceProfilerCleanup() {
Index: llvm/include/llvm/Support/TimeProfiler.h
===
--- llvm/include/llvm/Support/TimeProfiler.h
+++ llvm/include/llvm/Support/TimeProfiler.h
@@ -19,7 +19,7 @@
 /// Initialize the time trace profiler.
 /// This sets up the global \p TimeTraceProfilerInstance
 /// variable to be the profiler instance.
-void timeTraceProfilerInitialize();
+void timeTraceProfilerInitialize(unsigned TimeTraceGranularity);
 
 /// Cleanup the time trace profiler, if it was initialized.
 void timeTraceProfilerCleanup();
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -216,9 +216,10 @@
   bool Success = CompilerInvocation::CreateFromArgs(
   Clang->getInvocation(), Argv.begin(), Argv.end(), Diags);
 
-  if (Clang->getFrontendOpts().TimeTrace)
-llvm::timeTraceProfilerInitialize();
-
+  if (Clang->getFrontendOpts().TimeTrace) {
+llvm::timeTraceProfilerInitialize(
+Clang->getFrontendOpts().TimeTraceGranularity);
+  }
   // --print-supported-cpus takes priority over the actual compilation.
   if (Clang->getFrontendOpts().PrintSupportedCPUs)
 return PrintSupportedCPUs(Clang->getTargetOpts().Triple);
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -1,5 +1,5 @@
 // REQUIRES: shell
-// RUN: %clangxx -S -ftime-trace -mllvm --time-trace-granularity=0 -o %T/check-time-trace %s
+// RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o %T/check-time-trace %s
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
 // RUN:   | FileCheck %s
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1769,6 +1769,8 @@
   Opts.ShowTimers = Args.hasArg(OPT_ftime_report);
   Opts.PrintSupportedCPUs = Args.hasArg(OPT_print_supported_cpus);
   Opts.TimeTrace = Args.hasArg(OPT_ftime_trace);
+  Opts.TimeTraceGranularity =
+  getLastArgIntValue(Args, OPT_ftime_trace_granularity_EQ, 500, Diags);
   Opts.ShowVersion = Args.hasArg(OPT_version);
   Opts.ASTMergeFiles = Args.getAllArgValues(OPT_ast_merge);
   Opts.LLVMArgs = Args.getAllArgValues(OPT_mllvm);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4575,6 +4575,7 @@
   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_parseable_fixits);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_report);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_trace);
+  Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_granularity_EQ);
   Args.AddLastArg(CmdArgs, options::OPT_ftrapv);
   Args.AddLastArg(CmdArgs, options::OPT_malign_double);
 
Index: 

[PATCH] D60663: Time profiler: small fixes and optimizations

[Anton Afanasyev via Phabricator via cfe-commits]

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



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:27
 
+static cl::opt TimeTraceGranularity(
+"time-trace-granularity",

sammccall wrote:
> I know this is late, but... this shows up in the help for any tool that links 
> in libSupport, many of which don't support the time profiler. Can you mark 
> this as hidden or (preferably) move this to cc1_main?
@sammccall Yes, thanks! Here is the fix https://reviews.llvm.org/D65202 , 
please, review it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60663



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

[PATCH] D65202: [Support] Fix `-ftime-trace-granularity` option

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev created this revision.
anton-afanasyev added a reviewer: sammccall.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

Move `-ftime-trace-granularity` option to frontend options. Without patch
this option is showed up in the help for any tool that links libSupport.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65202

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/TimeProfiler.h
  llvm/lib/Support/TimeProfiler.cpp

Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -24,11 +24,8 @@
 
 namespace llvm {
 
-static cl::opt TimeTraceGranularity(
-"time-trace-granularity",
-cl::desc(
-"Minimum time granularity (in microseconds) traced by time profiler"),
-cl::init(500));
+// Minimum time granularity (in microseconds) traced by time profiler
+unsigned TimeTraceGranularity = 500;
 
 TimeTraceProfiler *TimeTraceProfilerInstance = nullptr;
 
@@ -163,10 +160,11 @@
   time_point StartTime;
 };
 
-void timeTraceProfilerInitialize() {
+void timeTraceProfilerInitialize(unsigned TimeTraceGranularity_) {
   assert(TimeTraceProfilerInstance == nullptr &&
  "Profiler should not be initialized");
   TimeTraceProfilerInstance = new TimeTraceProfiler();
+  TimeTraceGranularity = TimeTraceGranularity_;
 }
 
 void timeTraceProfilerCleanup() {
Index: llvm/include/llvm/Support/TimeProfiler.h
===
--- llvm/include/llvm/Support/TimeProfiler.h
+++ llvm/include/llvm/Support/TimeProfiler.h
@@ -19,7 +19,7 @@
 /// Initialize the time trace profiler.
 /// This sets up the global \p TimeTraceProfilerInstance
 /// variable to be the profiler instance.
-void timeTraceProfilerInitialize();
+void timeTraceProfilerInitialize(unsigned TimeTraceGranularity);
 
 /// Cleanup the time trace profiler, if it was initialized.
 void timeTraceProfilerCleanup();
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -216,9 +216,10 @@
   bool Success = CompilerInvocation::CreateFromArgs(
   Clang->getInvocation(), Argv.begin(), Argv.end(), Diags);
 
-  if (Clang->getFrontendOpts().TimeTrace)
-llvm::timeTraceProfilerInitialize();
-
+  if (Clang->getFrontendOpts().TimeTrace) {
+llvm::timeTraceProfilerInitialize(
+Clang->getFrontendOpts().TimeTraceGranularity);
+  }
   // --print-supported-cpus takes priority over the actual compilation.
   if (Clang->getFrontendOpts().PrintSupportedCPUs)
 return PrintSupportedCPUs(Clang->getTargetOpts().Triple);
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -1,5 +1,5 @@
 // REQUIRES: shell
-// RUN: %clangxx -S -ftime-trace -mllvm --time-trace-granularity=0 -o %T/check-time-trace %s
+// RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o %T/check-time-trace %s
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
 // RUN:   | FileCheck %s
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1769,6 +1769,8 @@
   Opts.ShowTimers = Args.hasArg(OPT_ftime_report);
   Opts.PrintSupportedCPUs = Args.hasArg(OPT_print_supported_cpus);
   Opts.TimeTrace = Args.hasArg(OPT_ftime_trace);
+  Opts.TimeTraceGranularity =
+  getLastArgIntValue(Args, OPT_ftime_trace_granularity_EQ, 500, Diags);
   Opts.ShowVersion = Args.hasArg(OPT_version);
   Opts.ASTMergeFiles = Args.getAllArgValues(OPT_ast_merge);
   Opts.LLVMArgs = Args.getAllArgValues(OPT_mllvm);
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4575,6 +4575,7 @@
   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_parseable_fixits);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_report);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_trace);
+  Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_granularity_EQ);
   Args.AddLastArg(CmdArgs, options::OPT_ftrapv);
   Args.AddLastArg(CmdArgs, options::OPT_malign_double);
 
Index: 

[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

[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

[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

[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

[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

[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

[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

[PATCH] D61914: [Support][Test] Time profiler: add regression test

[Anton Afanasyev via Phabricator via cfe-commits]

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

In D61914#1534508 , @plotfi wrote:

> cfe/trunk/test/Driver/check-time-trace.cpp appears to fail on Darwin. Did you 
> mean to pass the target explicitly ?


This commit is reverted now. How can I pass the target explicitly?




Comment at: cfe/trunk/test/Driver/check-time-trace.cpp:1
+// RUN: %clangxx -ftime-trace %s 2>&1 | grep "Time trace json-file dumped to" \
+// RUN:   | awk '{print $NF}' | xargs cat \

plotfi wrote:
> This test should probably have // REQUIRES: shell
Thanks! I haven't found `REQUIRES: shell` here: 
`https://llvm.org/docs/CommandGuide/FileCheck.html`. Does this directive mean 
the target OS must have utils like `awk` and `xargs`?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61914



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

[PATCH] D61914: [Support][Test] Time profiler: add regression test

[Anton Afanasyev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362792: [Support][Test] Time profiler: add regression test 
(authored by anton-afanasyev, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61914?vs=203532=203544#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61914

Files:
  cfe/trunk/test/Driver/check-time-trace.cpp
  cfe/trunk/tools/driver/cc1_main.cpp


Index: cfe/trunk/test/Driver/check-time-trace.cpp
===
--- cfe/trunk/test/Driver/check-time-trace.cpp
+++ cfe/trunk/test/Driver/check-time-trace.cpp
@@ -0,0 +1,23 @@
+// RUN: %clangxx -ftime-trace %s 2>&1 | grep "Time trace json-file dumped to" \
+// RUN:   | awk '{print $NF}' | xargs cat \
+// RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
+
+// CHECK: "traceEvents": [
+// CHECK: "args":
+// CHECK: "detail":
+// CHECK: "dur":
+// CHECK: "name": "Source"
+// CHECK-NEXT: "ph":
+// CHECK-NEXT: "pid":
+// CHECK-NEXT: "tid":
+// CHECK-NEXT: "ts":
+// CHECK: "name": "clang"
+// CHECK: "name": "process_name"
+
+#include 
+
+int main() {
+  std::cout << "Foo" << std::endl;
+  return 0;
+}
Index: cfe/trunk/tools/driver/cc1_main.cpp
===
--- cfe/trunk/tools/driver/cc1_main.cpp
+++ cfe/trunk/tools/driver/cc1_main.cpp
@@ -241,6 +241,11 @@
 
 llvm::timeTraceProfilerWrite(*profilerOutput);
 llvm::timeTraceProfilerCleanup();
+
+llvm::errs() << "Time trace json-file dumped to " << Path.str() << "\n";
+llvm::errs()
+<< "Use chrome://tracing or Speedscope App "
+   "(https://www.speedscope.app) for flamegraph visualization\n";
   }
 
   // Our error handler depends on the Diagnostics object, which we're


Index: cfe/trunk/test/Driver/check-time-trace.cpp
===
--- cfe/trunk/test/Driver/check-time-trace.cpp
+++ cfe/trunk/test/Driver/check-time-trace.cpp
@@ -0,0 +1,23 @@
+// RUN: %clangxx -ftime-trace %s 2>&1 | grep "Time trace json-file dumped to" \
+// RUN:   | awk '{print $NF}' | xargs cat \
+// RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
+
+// CHECK: "traceEvents": [
+// CHECK: "args":
+// CHECK: "detail":
+// CHECK: "dur":
+// CHECK: "name": "Source"
+// CHECK-NEXT: "ph":
+// CHECK-NEXT: "pid":
+// CHECK-NEXT: "tid":
+// CHECK-NEXT: "ts":
+// CHECK: "name": "clang"
+// CHECK: "name": "process_name"
+
+#include 
+
+int main() {
+  std::cout << "Foo" << std::endl;
+  return 0;
+}
Index: cfe/trunk/tools/driver/cc1_main.cpp
===
--- cfe/trunk/tools/driver/cc1_main.cpp
+++ cfe/trunk/tools/driver/cc1_main.cpp
@@ -241,6 +241,11 @@
 
 llvm::timeTraceProfilerWrite(*profilerOutput);
 llvm::timeTraceProfilerCleanup();
+
+llvm::errs() << "Time trace json-file dumped to " << Path.str() << "\n";
+llvm::errs()
+<< "Use chrome://tracing or Speedscope App "
+   "(https://www.speedscope.app) for flamegraph visualization\n";
   }
 
   // Our error handler depends on the Diagnostics object, which we're
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61914: [Support][Test] Time profiler: add regression test

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev updated this revision to Diff 203532.
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/D61914/new/

https://reviews.llvm.org/D61914

Files:
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp


Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -241,6 +241,11 @@
 
 llvm::timeTraceProfilerWrite(*profilerOutput);
 llvm::timeTraceProfilerCleanup();
+
+llvm::errs() << "Time trace json-file dumped to " << Path.str() << "\n";
+llvm::errs()
+<< "Use chrome://tracing or Speedscope App "
+   "(https://www.speedscope.app) for flamegraph visualization\n";
   }
 
   // Our error handler depends on the Diagnostics object, which we're
Index: clang/test/Driver/check-time-trace.cpp
===
--- /dev/null
+++ clang/test/Driver/check-time-trace.cpp
@@ -0,0 +1,23 @@
+// RUN: %clangxx -ftime-trace %s 2>&1 | grep "Time trace json-file dumped to" \
+// RUN:   | awk '{print $NF}' | xargs cat \
+// RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
+
+// CHECK: "traceEvents": [
+// CHECK: "args":
+// CHECK: "detail":
+// CHECK: "dur":
+// CHECK: "name": "Source"
+// CHECK-NEXT: "ph":
+// CHECK-NEXT: "pid":
+// CHECK-NEXT: "tid":
+// CHECK-NEXT: "ts":
+// CHECK: "name": "clang"
+// CHECK: "name": "process_name"
+
+#include 
+
+int main() {
+  std::cout << "Foo" << std::endl;
+  return 0;
+}


Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -241,6 +241,11 @@
 
 llvm::timeTraceProfilerWrite(*profilerOutput);
 llvm::timeTraceProfilerCleanup();
+
+llvm::errs() << "Time trace json-file dumped to " << Path.str() << "\n";
+llvm::errs()
+<< "Use chrome://tracing or Speedscope App "
+   "(https://www.speedscope.app) for flamegraph visualization\n";
   }
 
   // Our error handler depends on the Diagnostics object, which we're
Index: clang/test/Driver/check-time-trace.cpp
===
--- /dev/null
+++ clang/test/Driver/check-time-trace.cpp
@@ -0,0 +1,23 @@
+// RUN: %clangxx -ftime-trace %s 2>&1 | grep "Time trace json-file dumped to" \
+// RUN:   | awk '{print $NF}' | xargs cat \
+// RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
+
+// CHECK: "traceEvents": [
+// CHECK: "args":
+// CHECK: "detail":
+// CHECK: "dur":
+// CHECK: "name": "Source"
+// CHECK-NEXT: "ph":
+// CHECK-NEXT: "pid":
+// CHECK-NEXT: "tid":
+// CHECK-NEXT: "ts":
+// CHECK: "name": "clang"
+// CHECK: "name": "process_name"
+
+#include 
+
+int main() {
+  std::cout << "Foo" << std::endl;
+  return 0;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61914: [Support][Test] Time profiler: add regression test

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev marked 4 inline comments as done.
anton-afanasyev added a comment.

In D61914#1512021 , @aganea wrote:

> Could you please move the test to a more approriate location? (ie. 
> clang/trunk/test/Driver/)


Thanks, I've moved it there.




Comment at: clang/tools/driver/cc1_main.cpp:245
+
+llvm::errs() << "Time trace json-file dumped to " << Path.str() << "\n";
+llvm::errs()

aganea wrote:
> This seems a bit too chatty. Suround these two lines with `if 
> (Config->Verbose)` ?
I don't think it should be done this way for several reasons:
1. This info is actually needed by user when one uses `--ftime-trace` since 
json-file is usually dumped to random filename located in `/tmp/`. Therefore 
`--ftime-trace` means `--ftime-trace --verbose` itself. Without this info user 
cannot understand where dumped file is. How can he know about `--verbose`?
2. Option `--ftime-report` is much more chatty, it dumps several tables to 
terminal. This option just dumps to file, but tells where file is.
3. I have not found elegant way to check `--verbose` option here, one should 
add it to `clang::FrontendOptions` or so.



Comment at: llvm/test/Support/check-time-trace.cxx:4
+
+// CHECK: "args":{"name":"clang"}
+

aganea wrote:
> I don't see any other `-ftime-trace` tests, I would add a few more exhaustive 
> file format checks here.
Ok, I've added more checks, parsing json file by python.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61914



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

[PATCH] D61945: ftime-trace as a CoreOption

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev accepted this revision.
anton-afanasyev added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D61945



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

[PATCH] D61914: [Support][Test] Time profiler: add regression test

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev created this revision.
anton-afanasyev added a reviewer: thakis.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Add output to `llvm::errs()` when `-ftime-trace` option is enabled,
add regression test checking this option works as expected.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61914

Files:
  clang/tools/driver/cc1_main.cpp
  llvm/test/Support/check-time-trace.cxx


Index: llvm/test/Support/check-time-trace.cxx
===
--- /dev/null
+++ llvm/test/Support/check-time-trace.cxx
@@ -0,0 +1,11 @@
+// RUN: clang++ -ftime-trace %s 2>&1 | grep "Time trace json-file dumped to" \
+// RUN:   | awk '{print $NF}' | xargs cat | FileCheck %s
+
+// CHECK: "args":{"name":"clang"}
+
+#include 
+
+int main() {
+  std::cout << "Foo" << std::endl;
+  return 0;
+}
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -241,6 +241,11 @@
 
 llvm::timeTraceProfilerWrite(*profilerOutput);
 llvm::timeTraceProfilerCleanup();
+
+llvm::errs() << "Time trace json-file dumped to " << Path.str() << "\n";
+llvm::errs()
+<< "Use chrome://tracing or Speedscope App "
+   "(https://www.speedscope.app) for flamegraph visualization\n";
   }
 
   // Our error handler depends on the Diagnostics object, which we're


Index: llvm/test/Support/check-time-trace.cxx
===
--- /dev/null
+++ llvm/test/Support/check-time-trace.cxx
@@ -0,0 +1,11 @@
+// RUN: clang++ -ftime-trace %s 2>&1 | grep "Time trace json-file dumped to" \
+// RUN:   | awk '{print $NF}' | xargs cat | FileCheck %s
+
+// CHECK: "args":{"name":"clang"}
+
+#include 
+
+int main() {
+  std::cout << "Foo" << std::endl;
+  return 0;
+}
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -241,6 +241,11 @@
 
 llvm::timeTraceProfilerWrite(*profilerOutput);
 llvm::timeTraceProfilerCleanup();
+
+llvm::errs() << "Time trace json-file dumped to " << Path.str() << "\n";
+llvm::errs()
+<< "Use chrome://tracing or Speedscope App "
+   "(https://www.speedscope.app) for flamegraph visualization\n";
   }
 
   // Our error handler depends on the Diagnostics object, which we're
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61822: make -ftime-trace also print template arguments

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev accepted this revision.
anton-afanasyev added a comment.
This revision is now accepted and ready to land.

LGTM
Thanks!


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

https://reviews.llvm.org/D61822



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

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

Patch by @thakis for BE passes tracing: https://reviews.llvm.org/D60782


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58675



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

[PATCH] D60663: Time profiler: small fixes and optimizations

[Anton Afanasyev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL358448: Time profiler: small fixes and optimizations 
(authored by anton-afanasyev, committed by ).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D60663?vs=195069=195244#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60663

Files:
  cfe/trunk/tools/driver/cc1_main.cpp
  llvm/trunk/include/llvm/Support/TimeProfiler.h
  llvm/trunk/lib/Support/TimeProfiler.cpp

Index: llvm/trunk/include/llvm/Support/TimeProfiler.h
===
--- llvm/trunk/include/llvm/Support/TimeProfiler.h
+++ llvm/trunk/include/llvm/Support/TimeProfiler.h
@@ -1,9 +1,8 @@
 //===- llvm/Support/TimeProfiler.h - Hierarchical Time Profiler -*- C++ -*-===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===--===//
 
@@ -33,7 +32,7 @@
 /// Write profiling data to output file.
 /// Data produced is JSON, in Chrome "Trace Event" format, see
 /// https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview
-void timeTraceProfilerWrite(std::unique_ptr );
+void timeTraceProfilerWrite(raw_pwrite_stream );
 
 /// Manually begin a time section, with the given \p Name and \p Detail.
 /// Profiler copies the string data, so the pointers can be given into
@@ -51,6 +50,13 @@
 /// the section; and when it is destroyed, it stops it. If the time profiler
 /// is not initialized, the overhead is a single branch.
 struct TimeTraceScope {
+
+  TimeTraceScope() = delete;
+  TimeTraceScope(const TimeTraceScope &) = delete;
+  TimeTraceScope =(const TimeTraceScope &) = delete;
+  TimeTraceScope(TimeTraceScope &&) = delete;
+  TimeTraceScope =(TimeTraceScope &&) = delete;
+
   TimeTraceScope(StringRef Name, StringRef Detail) {
 if (TimeTraceProfilerInstance != nullptr)
   timeTraceProfilerBegin(Name, Detail);
Index: llvm/trunk/lib/Support/TimeProfiler.cpp
===
--- llvm/trunk/lib/Support/TimeProfiler.cpp
+++ llvm/trunk/lib/Support/TimeProfiler.cpp
@@ -1,30 +1,35 @@
 //===-- TimeProfiler.cpp - Hierarchical Time Profiler -===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===--===//
 //
-/// \file Hierarchical time profiler implementation.
+// This file implements hierarchical time profiler.
 //
 //===--===//
 
 #include "llvm/Support/TimeProfiler.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include 
 #include 
 #include 
-#include 
 #include 
 
 using namespace std::chrono;
 
 namespace llvm {
 
+static cl::opt TimeTraceGranularity(
+"time-trace-granularity",
+cl::desc(
+"Minimum time granularity (in microseconds) traced by time profiler"),
+cl::init(500));
+
 TimeTraceProfiler *TimeTraceProfilerInstance = nullptr;
 
 static std::string escapeString(StringRef Src) {
@@ -61,18 +66,21 @@
   DurationType Duration;
   std::string Name;
   std::string Detail;
+
+  Entry(time_point &, DurationType &, std::string &,
+std::string &)
+  : Start(std::move(S)), Duration(std::move(D)), Name(std::move(N)),
+Detail(std::move(Dt)){};
 };
 
 struct TimeTraceProfiler {
   TimeTraceProfiler() {
-Stack.reserve(8);
-Entries.reserve(128);
 StartTime = steady_clock::now();
   }
 
   void begin(std::string Name, llvm::function_ref Detail) {
-Entry E = {steady_clock::now(), {}, Name, Detail()};
-Stack.push_back(std::move(E));
+Stack.emplace_back(steady_clock::now(), DurationType{}, std::move(Name),
+   Detail());
   }
 
   void end() {
@@ -80,8 +88,8 @@
 auto  = Stack.back();
 E.Duration = steady_clock::now() - E.Start;
 
-// Only include sections longer than 500us.
-if (duration_cast(E.Duration).count() > 500)
+// Only include sections longer than TimeTraceGranularity msec.
+if (duration_cast(E.Duration).count() > 

[PATCH] D60663: Time profiler: small fixes and optimizations

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

In D60663#1465721 , @lebedev.ri wrote:

> Looks good ignoring the json bits.
>
> Re license:
>
> > https://reviews.llvm.org/D58675
> >  This is the first part of time tracing system, I have splitted them cause 
> > this part is mostly written by Aras Pranckevicius except of several minor 
> > fixes concerning formatting.
>
> So i can't and won't claim any legal knowledge, but it maybe would be good 
> for him to at least comment here, that he is ok with this?


I've communicated to Aras by mail, he is ok with new license header:

> "Sure! I started my branch before the LLVM license change. The new one is 
> fine."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60663



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

[PATCH] D60663: Time profiler: small fixes and optimizations

[Anton Afanasyev via Phabricator via cfe-commits]

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

Updated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60663

Files:
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/TimeProfiler.h
  llvm/lib/Support/TimeProfiler.cpp

Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -1,30 +1,35 @@
 //===-- TimeProfiler.cpp - Hierarchical Time Profiler -===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===--===//
 //
-/// \file Hierarchical time profiler implementation.
+// This file implements hierarchical time profiler.
 //
 //===--===//
 
 #include "llvm/Support/TimeProfiler.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include 
 #include 
 #include 
-#include 
 #include 
 
 using namespace std::chrono;
 
 namespace llvm {
 
+static cl::opt TimeTraceGranularity(
+"time-trace-granularity",
+cl::desc(
+"Minimum time granularity (in microsecons) traced by time profiler"),
+cl::init(500));
+
 TimeTraceProfiler *TimeTraceProfilerInstance = nullptr;
 
 static std::string escapeString(StringRef Src) {
@@ -61,18 +66,21 @@
   DurationType Duration;
   std::string Name;
   std::string Detail;
+
+  Entry(time_point &, DurationType &, std::string &,
+std::string &)
+  : Start(std::move(S)), Duration(std::move(D)), Name(std::move(N)),
+Detail(std::move(Dt)){};
 };
 
 struct TimeTraceProfiler {
   TimeTraceProfiler() {
-Stack.reserve(8);
-Entries.reserve(128);
 StartTime = steady_clock::now();
   }
 
   void begin(std::string Name, llvm::function_ref Detail) {
-Entry E = {steady_clock::now(), {}, Name, Detail()};
-Stack.push_back(std::move(E));
+Stack.emplace_back(steady_clock::now(), DurationType{}, std::move(Name),
+   Detail());
   }
 
   void end() {
@@ -80,8 +88,8 @@
 auto  = Stack.back();
 E.Duration = steady_clock::now() - E.Start;
 
-// Only include sections longer than 500us.
-if (duration_cast(E.Duration).count() > 500)
+// Only include sections longer than 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
@@ -100,20 +108,20 @@
 Stack.pop_back();
   }
 
-  void Write(std::unique_ptr ) {
+  void Write(raw_pwrite_stream ) {
 assert(Stack.empty() &&
"All profiler sections should be ended when calling Write");
 
-*OS << "{ \"traceEvents\": [\n";
+OS << "{ \"traceEvents\": [\n";
 
 // Emit all events for the main flame graph.
 for (const auto  : Entries) {
   auto StartUs = duration_cast(E.Start - StartTime).count();
   auto DurUs = duration_cast(E.Duration).count();
-  *OS << "{ \"pid\":1, \"tid\":0, \"ph\":\"X\", \"ts\":" << StartUs
-  << ", \"dur\":" << DurUs << ", \"name\":\"" << escapeString(E.Name)
-  << "\", \"args\":{ \"detail\":\"" << escapeString(E.Detail)
-  << "\"} },\n";
+  OS << "{ \"pid\":1, \"tid\":0, \"ph\":\"X\", \"ts\":" << StartUs
+ << ", \"dur\":" << DurUs << ", \"name\":\"" << escapeString(E.Name)
+ << "\", \"args\":{ \"detail\":\"" << escapeString(E.Detail)
+ << "\"} },\n";
 }
 
 // Emit totals by section name as additional "thread" events, sorted from
@@ -121,32 +129,32 @@
 int Tid = 1;
 std::vector SortedTotals;
 SortedTotals.reserve(CountAndTotalPerName.size());
-for (const auto  : CountAndTotalPerName) {
+for (const auto  : CountAndTotalPerName)
   SortedTotals.emplace_back(E.getKey(), E.getValue());
-}
-std::sort(SortedTotals.begin(), SortedTotals.end(),
-  [](const NameAndCountAndDurationType ,
- const NameAndCountAndDurationType ) {
-return A.second.second > B.second.second;
-  });
+
+llvm::sort(SortedTotals.begin(), SortedTotals.end(),
+   [](const NameAndCountAndDurationType ,
+  const NameAndCountAndDurationType ) {
+ return A.second.second > B.second.second;
+   });
 for (const auto  : SortedTotals) {
   auto DurUs = 

[PATCH] D60663: Time profiler: small fixes and optimizations

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev updated this revision to Diff 195068.
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/D60663/new/

https://reviews.llvm.org/D60663

Files:
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/TimeProfiler.h
  llvm/lib/Support/TimeProfiler.cpp

Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -1,19 +1,19 @@
 //===-- TimeProfiler.cpp - Hierarchical Time Profiler -===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===--===//
 //
-/// \file Hierarchical time profiler implementation.
+// This file implements hierarchical time profiler.
 //
 //===--===//
 
 #include "llvm/Support/TimeProfiler.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include 
 #include 
@@ -25,6 +25,12 @@
 
 namespace llvm {
 
+static cl::opt TimeTraceGranularity(
+"time-trace-granularity",
+cl::desc(
+"Minimum time granularity (in microsecons) traced by time profiler"),
+cl::init(500));
+
 TimeTraceProfiler *TimeTraceProfilerInstance = nullptr;
 
 static std::string escapeString(StringRef Src) {
@@ -61,18 +67,21 @@
   DurationType Duration;
   std::string Name;
   std::string Detail;
+
+  Entry(time_point &, DurationType &, std::string &,
+std::string &)
+  : Start(std::move(S)), Duration(std::move(D)), Name(std::move(N)),
+Detail(std::move(Dt)){};
 };
 
 struct TimeTraceProfiler {
   TimeTraceProfiler() {
-Stack.reserve(8);
-Entries.reserve(128);
 StartTime = steady_clock::now();
   }
 
   void begin(std::string Name, llvm::function_ref Detail) {
-Entry E = {steady_clock::now(), {}, Name, Detail()};
-Stack.push_back(std::move(E));
+Stack.emplace_back(steady_clock::now(), DurationType{}, std::move(Name),
+   Detail());
   }
 
   void end() {
@@ -80,8 +89,8 @@
 auto  = Stack.back();
 E.Duration = steady_clock::now() - E.Start;
 
-// Only include sections longer than 500us.
-if (duration_cast(E.Duration).count() > 500)
+// Only include sections longer than 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
@@ -100,20 +109,20 @@
 Stack.pop_back();
   }
 
-  void Write(std::unique_ptr ) {
+  void Write(raw_pwrite_stream ) {
 assert(Stack.empty() &&
"All profiler sections should be ended when calling Write");
 
-*OS << "{ \"traceEvents\": [\n";
+OS << "{ \"traceEvents\": [\n";
 
 // Emit all events for the main flame graph.
 for (const auto  : Entries) {
   auto StartUs = duration_cast(E.Start - StartTime).count();
   auto DurUs = duration_cast(E.Duration).count();
-  *OS << "{ \"pid\":1, \"tid\":0, \"ph\":\"X\", \"ts\":" << StartUs
-  << ", \"dur\":" << DurUs << ", \"name\":\"" << escapeString(E.Name)
-  << "\", \"args\":{ \"detail\":\"" << escapeString(E.Detail)
-  << "\"} },\n";
+  OS << "{ \"pid\":1, \"tid\":0, \"ph\":\"X\", \"ts\":" << StartUs
+ << ", \"dur\":" << DurUs << ", \"name\":\"" << escapeString(E.Name)
+ << "\", \"args\":{ \"detail\":\"" << escapeString(E.Detail)
+ << "\"} },\n";
 }
 
 // Emit totals by section name as additional "thread" events, sorted from
@@ -121,32 +130,32 @@
 int Tid = 1;
 std::vector SortedTotals;
 SortedTotals.reserve(CountAndTotalPerName.size());
-for (const auto  : CountAndTotalPerName) {
+for (const auto  : CountAndTotalPerName)
   SortedTotals.emplace_back(E.getKey(), E.getValue());
-}
-std::sort(SortedTotals.begin(), SortedTotals.end(),
-  [](const NameAndCountAndDurationType ,
- const NameAndCountAndDurationType ) {
-return A.second.second > B.second.second;
-  });
+
+llvm::sort(SortedTotals.begin(), SortedTotals.end(),
+   [](const NameAndCountAndDurationType ,
+  const NameAndCountAndDurationType ) {
+ return A.second.second > B.second.second;
+   });
 for (const auto  : SortedTotals) {
   auto 

[PATCH] D60663: Time profiler: small fixes and optimizations

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev marked 7 inline comments as done.
anton-afanasyev added a subscriber: aras-p.
anton-afanasyev added a comment.

In D60663#1465721 , @lebedev.ri wrote:

> Looks good ignoring the json bits.
>
> Re license:
>
> > https://reviews.llvm.org/D58675
> >  This is the first part of time tracing system, I have splitted them cause 
> > this part is mostly written by Aras Pranckevicius except of several minor 
> > fixes concerning formatting.
>
> So i can't and won't claim any legal knowledge, but it maybe would be good 
> for him to at least comment here, that he is ok with this?


Oh, sure! @aras-p -- I have changed license header to fit it with current LLVM 
Relicensing state. Are you ok with this? (see 
https://reviews.llvm.org/D60663#change-jr7Lagn5WNFy)




Comment at: llvm/lib/Support/TimeProfiler.cpp:20
 #include 
 #include 
 #include 

lebedev.ri wrote:
> Unused header?
Yes, thanks.



Comment at: llvm/lib/Support/TimeProfiler.cpp:47-48
   void begin(std::string Name, llvm::function_ref Detail) {
-Entry E = {steady_clock::now(), {}, Name, Detail()};
+Entry E = {steady_clock::now(), {}, std::move(Name), Detail()};
 Stack.push_back(std::move(E));
   }

lebedev.ri wrote:
> Does
> ```
> Stack.emplace_back(steady_clock::now(), {}, std::move(Name), Detail());
> ```
> not work?
> (also, the `std::string` returned from `Detail` function invocation is moved?)
Ok, changed.



Comment at: llvm/lib/Support/TimeProfiler.cpp:57
 // Only include sections longer than 500us.
 if (duration_cast(E.Duration).count() > 500)
   Entries.emplace_back(E);

lebedev.ri wrote:
> I feel like this should be 
> ```
> static cl::opt TimeProfileGranularity(
> "time-profile-granularity",
> cl::desc(""),
> cl::init(500));
> ```
> ?
I planned to change this later together with unit tests (cause they need small 
time granularity), but can fix it now. Changed, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60663



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev marked 23 inline comments as done.
anton-afanasyev added a comment.

In D58675#1465336 , @lebedev.ri wrote:

> Some post-commit review (please submit a new review, don't replace this diff)
>  As usual, the incorrect license headers keep slipping through.


Ok, I've made a separate review for this: https://reviews.llvm.org/D60663




Comment at: llvm/trunk/include/llvm/Support/TimeProfiler.h:5-6
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//

lebedev.ri wrote:
> OOOPS, wrong license.
Yes, thanks.



Comment at: llvm/trunk/include/llvm/Support/TimeProfiler.h:53
+/// is not initialized, the overhead is a single branch.
+struct TimeTraceScope {
+  TimeTraceScope(StringRef Name, StringRef Detail) {

lebedev.ri wrote:
> Did you mean to explicitly prohibit all the default constructors / 
> `operator=`?
Ok, I'm to add lines like `TimeTraceScope() = delete;` here.



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:5-6
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//

lebedev.ri wrote:
> Wrong license.
Yes, I'm to fix it.



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:29-30
+
+static std::string escapeString(StringRef Src) {
+  std::string OS;
+  for (const unsigned char  : Src) {

lebedev.ri wrote:
> lebedev.ri wrote:
> > `SmallString<32>` ?
> > Also, it is safe to `OS.reserve(Src.size())`
> Also, you probably want to add `raw_svector_ostream` ontop, and `<<` into it.
This function has been already dropped here https://reviews.llvm.org/D60609 . 
I'm to submit code without it if json library using are ok for performance.



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:59-60
+  DurationType Duration;
+  std::string Name;
+  std::string Detail;
+};

lebedev.ri wrote:
> `SmallString<32>`?
Hmm, would it make a sense for `Detail`? `Entry`s are heap-allocated, the 
actual size is ranging from several bytes to several hundreds. Also, 
getQualifiedNameAsString() which is usually used for `Detail` returns 
std::string.



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:70-71
+
+  void begin(std::string Name, llvm::function_ref Detail) {
+Entry E = {steady_clock::now(), {}, Name, Detail()};
+Stack.push_back(std::move(E));

lebedev.ri wrote:
> Why not either take `StringRef` arg, or at least `std::move()` it when 
> creating `Entry`?
Do you mean `std::move(Name)`? Ok.



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:89
+// itself.
+if (std::find_if(++Stack.rbegin(), Stack.rend(), [&](const Entry ) {
+  return Val.Name == E.Name;

lebedev.ri wrote:
> llvm::find_if(llvm::reverse(), ) 
Hmm, one need not `[rbegin(), rend()]`, but `[rbegin()++,rend()]`, so have to 
explicitly specify begin and end, `llvm::reverse(Stack)` is inappropriate here.



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:99
+
+  void Write(std::unique_ptr ) {
+assert(Stack.empty() &&

lebedev.ri wrote:
> Why pass `std::unique_ptr` ?
> Just `raw_pwrite_stream `
Yes, thanks! 
This was blindly copied from `CompilerInstance::createOutputFile()` return type.



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:122
+  SortedTotals.push_back(E);
+}
+std::sort(SortedTotals.begin(), SortedTotals.end(),

lebedev.ri wrote:
> Elide `{}`
Ok, thanks.



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:123
+}
+std::sort(SortedTotals.begin(), SortedTotals.end(),
+  [](const NameAndDuration , const NameAndDuration ) {

lebedev.ri wrote:
> llvm::sort <- this is a correctness issue.
Yes, thanks!



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:144-145
+
+  std::vector Stack;
+  std::vector Entries;
+  std::unordered_map TotalPerName;

lebedev.ri wrote:
> Would it make sense to make these `SmallVector`?
Ok, I'm to change it to
```
  SmallVector Stack;  
  SmallVector Entries;
```
`Stack` size may be enough for small sources while `Entries` usually amounts 
many thousands of `Entry`s.



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:146-147
+  std::vector Entries;
+  std::unordered_map TotalPerName;
+  std::unordered_map CountPerName;
+  time_point StartTime;

lebedev.ri wrote:
> 1. Eww, `std::unordered_map`, that will have *horrible* perf.
> 2. Eww, map with key = string. Use `llvm::StringMap`
You are right, but this is already fixed and submitted in this follow-up: 
https://reviews.llvm.org/D60404



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:162
+
+void 

[PATCH] D60663: Time profiler: small fixes and optimizations

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev created this revision.
anton-afanasyev added a reviewer: lebedev.ri.
Herald added subscribers: llvm-commits, cfe-commits, mgrang, hiraditya.
Herald added projects: clang, LLVM.

Fixes from Roman's review here: https://reviews.llvm.org/D58675#1465336


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60663

Files:
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/TimeProfiler.h
  llvm/lib/Support/TimeProfiler.cpp

Index: llvm/lib/Support/TimeProfiler.cpp
===
--- llvm/lib/Support/TimeProfiler.cpp
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -1,13 +1,12 @@
 //===-- TimeProfiler.cpp - Hierarchical Time Profiler -===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===--===//
 //
-/// \file Hierarchical time profiler implementation.
+// This file implements hierarchical time profiler.
 //
 //===--===//
 
@@ -41,13 +40,11 @@
 
 struct TimeTraceProfiler {
   TimeTraceProfiler() {
-Stack.reserve(8);
-Entries.reserve(128);
 StartTime = steady_clock::now();
   }
 
   void begin(std::string Name, llvm::function_ref Detail) {
-Entry E = {steady_clock::now(), {}, Name, Detail()};
+Entry E = {steady_clock::now(), {}, std::move(Name), Detail()};
 Stack.push_back(std::move(E));
   }
 
@@ -76,7 +73,7 @@
 Stack.pop_back();
   }
 
-  void Write(std::unique_ptr ) {
+  void Write(raw_pwrite_stream ) {
 assert(Stack.empty() &&
"All profiler sections should be ended when calling Write");
 
@@ -103,14 +100,14 @@
 int Tid = 1;
 std::vector SortedTotals;
 SortedTotals.reserve(CountAndTotalPerName.size());
-for (const auto  : CountAndTotalPerName) {
+for (const auto  : CountAndTotalPerName)
   SortedTotals.emplace_back(E.getKey(), E.getValue());
-}
-std::sort(SortedTotals.begin(), SortedTotals.end(),
-  [](const NameAndCountAndDurationType ,
- const NameAndCountAndDurationType ) {
-return A.second.second > B.second.second;
-  });
+
+llvm::sort(SortedTotals.begin(), SortedTotals.end(),
+   [](const NameAndCountAndDurationType ,
+  const NameAndCountAndDurationType ) {
+ return A.second.second > B.second.second;
+   });
 for (const auto  : SortedTotals) {
   auto DurUs = duration_cast(E.second.second).count();
   auto Count = CountAndTotalPerName[E.first].first;
@@ -140,12 +137,12 @@
 {"args", json::Object{{"name", "clang"}}},
 });
 
-*OS << formatv("{0:2}", json::Value(json::Object(
-{{"traceEvents", std::move(Events)}})));
+OS << formatv("{0:2}", json::Value(json::Object(
+   {{"traceEvents", std::move(Events)}})));
   }
 
-  std::vector Stack;
-  std::vector Entries;
+  SmallVector Stack;
+  SmallVector Entries;
   StringMap CountAndTotalPerName;
   time_point StartTime;
 };
@@ -161,7 +158,7 @@
   TimeTraceProfilerInstance = nullptr;
 }
 
-void timeTraceProfilerWrite(std::unique_ptr ) {
+void timeTraceProfilerWrite(raw_pwrite_stream ) {
   assert(TimeTraceProfilerInstance != nullptr &&
  "Profiler object can't be null");
   TimeTraceProfilerInstance->Write(OS);
Index: llvm/include/llvm/Support/TimeProfiler.h
===
--- llvm/include/llvm/Support/TimeProfiler.h
+++ llvm/include/llvm/Support/TimeProfiler.h
@@ -1,9 +1,8 @@
 //===- llvm/Support/TimeProfiler.h - Hierarchical Time Profiler -*- C++ -*-===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===--===//
 
@@ -33,7 +32,7 @@
 /// Write profiling data to output file.
 /// Data produced is JSON, in Chrome "Trace Event" format, see
 /// https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview
-void timeTraceProfilerWrite(std::unique_ptr );
+void timeTraceProfilerWrite(raw_pwrite_stream );
 
 /// Manually begin a time section, with the given \p Name and \p Detail.
 /// Profiler copies the string data, so the 

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

Use native llvm JSON library update: https://reviews.llvm.org/D60609


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

Fix hashing update: https://reviews.llvm.org/D60404


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

In D58675#1449053 , @thakis wrote:

> ps: Hooray for landing this, and thanks for the cool feature!


Thanks! It's @aras_p accomplishment.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

In D58675#1449051 , @thakis wrote:

> Looks like this landed without tests. Please add tests in a follow-up.
>
> Also, it looks like the flag is currently a cc1 flag. This should probably be 
> a CoreOption instead so that both the gcc-style and the cl-style drivers 
> expose it. Right now, users can't really use this if I read the patch right 
> (without using -Xclang, which is discouraged.)


Ok, thanks, I'm to add tests in a follow-up.

It works like `clang -ftime-trace main.cpp` now. Not sure I know where 
CoreOption is, but I'm to figure out it, thanks.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357340: Adds `-ftime-trace` option to clang that produces 
Chrome `chrome://tracing`… (authored by anton-afanasyev, committed by ).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D58675?vs=192525=192963#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58675

Files:
  cfe/trunk/include/clang/Basic/CodeGenOptions.def
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Frontend/FrontendOptions.h
  cfe/trunk/lib/CodeGen/BackendUtil.cpp
  cfe/trunk/lib/CodeGen/CodeGenModule.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CompilerInstance.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/Parse/ParseAST.cpp
  cfe/trunk/lib/Parse/ParseDeclCXX.cpp
  cfe/trunk/lib/Parse/ParseTemplate.cpp
  cfe/trunk/lib/Sema/Sema.cpp
  cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
  cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
  cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
  cfe/trunk/tools/driver/cc1_main.cpp
  llvm/trunk/include/llvm/Support/TimeProfiler.h
  llvm/trunk/lib/IR/LegacyPassManager.cpp
  llvm/trunk/lib/Support/CMakeLists.txt
  llvm/trunk/lib/Support/TimeProfiler.cpp

Index: cfe/trunk/lib/CodeGen/BackendUtil.cpp
===
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp
@@ -42,6 +42,7 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/TargetRegistry.h"
+#include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
@@ -1382,6 +1383,9 @@
   const llvm::DataLayout , Module *M,
   BackendAction Action,
   std::unique_ptr OS) {
+
+  llvm::TimeTraceScope TimeScope("Backend", StringRef(""));
+
   std::unique_ptr EmptyModule;
   if (!CGOpts.ThinLTOIndexFile.empty()) {
 // If we are performing a ThinLTO importing compile, load the function index
Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp
@@ -58,6 +58,7 @@
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MD5.h"
+#include "llvm/Support/TimeProfiler.h"
 
 using namespace clang;
 using namespace CodeGen;
@@ -2482,6 +2483,9 @@
 if (!shouldEmitFunction(GD))
   return;
 
+llvm::TimeTraceScope TimeScope(
+"CodeGen Function", [&]() { return FD->getQualifiedNameAsString(); });
+
 if (const auto *Method = dyn_cast(D)) {
   // Make sure to emit the definition(s) before we emit the thunks.
   // This is necessary for the generation of certain thunks.
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -4548,6 +4548,7 @@
   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_print_source_range_info);
   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_parseable_fixits);
   Args.AddLastArg(CmdArgs, options::OPT_ftime_report);
+  Args.AddLastArg(CmdArgs, options::OPT_ftime_trace);
   Args.AddLastArg(CmdArgs, options::OPT_ftrapv);
   Args.AddLastArg(CmdArgs, options::OPT_malign_double);
 
Index: cfe/trunk/lib/Sema/Sema.cpp
===
--- cfe/trunk/lib/Sema/Sema.cpp
+++ cfe/trunk/lib/Sema/Sema.cpp
@@ -39,6 +39,8 @@
 #include "clang/Sema/TemplateInstCallback.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/Support/TimeProfiler.h"
+
 using namespace clang;
 using namespace sema;
 
@@ -92,6 +94,12 @@
   SourceManager  = S->getSourceManager();
   SourceLocation IncludeLoc = SM.getIncludeLoc(SM.getFileID(Loc));
   if (IncludeLoc.isValid()) {
+if (llvm::timeTraceProfilerEnabled()) {
+  const FileEntry *FE = SM.getFileEntryForID(SM.getFileID(Loc));
+  llvm::timeTraceProfilerBegin(
+  "Source", FE != nullptr ? FE->getName() : StringRef(""));
+}
+
 IncludeStack.push_back(IncludeLoc);
 S->DiagnoseNonDefaultPragmaPack(
 Sema::PragmaPackDiagnoseKind::NonDefaultStateAtInclude, IncludeLoc);
@@ -99,10 +107,14 @@
   break;
 }
 case ExitFile:
-  if (!IncludeStack.empty())
+  if (!IncludeStack.empty()) {
+if (llvm::timeTraceProfilerEnabled())
+  llvm::timeTraceProfilerEnd();
+
 S->DiagnoseNonDefaultPragmaPack(
 Sema::PragmaPackDiagnoseKind::ChangedStateAtExit,
 

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

In D58675#1448112 , @rnk wrote:

> lgtm
>
> I think this is ready. We can adjust the overloads after the fact. I'd like 
> to get the feature in so we can make improvements independently.


Ok, so I'm to commit this and fix three points in the next commit(s):

1. Improve hashing (it's easy).
2. Use json utility.
3. Improve function_ref constructor and delete StringRef("...") explicit 
casting.


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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev updated this revision to Diff 192525.
anton-afanasyev marked 10 inline comments as done.

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

https://reviews.llvm.org/D58675

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/TimeProfiler.h
  llvm/lib/IR/LegacyPassManager.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/TimeProfiler.cpp

Index: llvm/lib/Support/TimeProfiler.cpp
===
--- /dev/null
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -0,0 +1,185 @@
+//===-- TimeProfiler.cpp - Hierarchical Time Profiler -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+/// \file Hierarchical time profiler implementation.
+//
+//===--===//
+
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/TimeProfiler.h"
+#include "llvm/Support/FileSystem.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace std::chrono;
+
+namespace llvm {
+
+TimeTraceProfiler *TimeTraceProfilerInstance = nullptr;
+
+static std::string escapeString(StringRef Src) {
+  std::string OS;
+  for (const unsigned char : Src) {
+switch (C) {
+case '"':
+case '/':
+case '\\':
+case '\b':
+case '\f':
+case '\n':
+case '\r':
+case '\t':
+  OS += '\\';
+  OS += C;
+  break;
+default:
+  if (isPrint(C)) {
+OS += C;
+  }
+}
+  }
+  return OS;
+}
+
+typedef duration DurationType;
+typedef std::pair NameAndDuration;
+
+struct Entry {
+  time_point Start;
+  DurationType Duration;
+  std::string Name;
+  std::string Detail;
+};
+
+struct TimeTraceProfiler {
+  TimeTraceProfiler() {
+Stack.reserve(8);
+Entries.reserve(128);
+StartTime = steady_clock::now();
+  }
+
+  void begin(std::string Name, llvm::function_ref Detail) {
+Entry E = {steady_clock::now(), {}, Name, Detail()};
+Stack.push_back(std::move(E));
+  }
+
+  void end() {
+assert(!Stack.empty() && "Must call begin() first");
+auto  = Stack.back();
+E.Duration = steady_clock::now() - E.Start;
+
+// Only include sections longer than 500us.
+if (duration_cast(E.Duration).count() > 500)
+  Entries.emplace_back(E);
+
+// Track total time taken by each "name", but only the topmost levels of
+// them; e.g. if there's a template instantiation that instantiates other
+// templates from within, we only want to add the topmost one. "topmost"
+// happens to be the ones that don't have any currently open entries above
+// itself.
+if (std::find_if(++Stack.rbegin(), Stack.rend(), [&](const Entry ) {
+  return Val.Name == E.Name;
+}) == Stack.rend()) {
+  TotalPerName[E.Name] += E.Duration;
+  CountPerName[E.Name]++;
+}
+
+Stack.pop_back();
+  }
+
+  void Write(std::unique_ptr ) {
+assert(Stack.empty() &&
+   "All profiler sections should be ended when calling Write");
+
+*OS << "{ \"traceEvents\": [\n";
+
+// Emit all events for the main flame graph.
+for (const auto  : Entries) {
+  auto StartUs = duration_cast(E.Start - StartTime).count();
+  auto DurUs = duration_cast(E.Duration).count();
+  *OS << "{ \"pid\":1, \"tid\":0, \"ph\":\"X\", \"ts\":" << StartUs
+  << ", \"dur\":" << DurUs << ", \"name\":\""
+  << escapeString(E.Name) << "\", \"args\":{ \"detail\":\""
+  << escapeString(E.Detail) << "\"} },\n";
+}
+
+// Emit totals by section name as additional "thread" events, sorted from
+// longest one.
+int Tid = 1;
+std::vector SortedTotals;
+SortedTotals.reserve(TotalPerName.size());
+for (const auto  : TotalPerName) {
+  SortedTotals.push_back(E);
+}
+std::sort(SortedTotals.begin(), SortedTotals.end(),
+  [](const NameAndDuration , const NameAndDuration ) {
+return A.second > B.second;
+  });
+for (const auto  : SortedTotals) {
+  auto DurUs = duration_cast(E.second).count();
+  *OS << "{ \"pid\":1, \"tid\":" << Tid << ", \"ph\":\"X\", \"ts\":" << 

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[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:154
   if (HaveLexer) {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 P.Initialize();

takuto.ikuta wrote:
> anton-afanasyev wrote:
> > takuto.ikuta wrote:
> > > Remove StringRef?
> > I use `StringRef("")` to explicitly cast to one of overloaded 
> > `TimeScope(StringRef, StringRef)` and `TimeScope(StringRef, 
> > fuction_ref(std::string()))`.
> I think function_ref(std::string()) does not have constructor receiving 
> StringRef, so I feel explicit cast is redundant.
> 
The compiler tries to use function_ref<..>(Callable&&, ...) constructor and 
instantiates `function_ref::function_ref`, so 
one gets error like:
```
error: call to constructor of 'llvm::TimeTraceScope' is ambiguous
llvm::TimeTraceScope TimeScope("Frontend", "");
 ^ ~~
.../include/llvm/Support/TimeProfiler.h:54:3: note: candidate constructor
  TimeTraceScope(StringRef Name, StringRef Detail) {
  ^
.../include/llvm/Support/TimeProfiler.h:58:3: note: candidate constructor
  TimeTraceScope(StringRef Name, llvm::function_ref Detail) {
  ^

```


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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

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



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

takuto.ikuta wrote:
> Remove StringRef?
I use `StringRef("")` to explicitly cast to one of overloaded 
`TimeScope(StringRef, StringRef)` and `TimeScope(StringRef, 
fuction_ref(std::string()))`.



Comment at: llvm/lib/Support/TimeProfiler.cpp:103
+
+*OS << "{ \"traceEvents\": [\n";
+

takuto.ikuta wrote:
> Don't we want to use json utility for this?
> https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/Support/JSON.h
> 
This implementation looks compact and fast. I've read proposal for this library 
https://docs.google.com/document/d/1OEF9IauWwNuSigZzvvbjc1cVS1uGHRyGTXaoy3DjqM4 
, it's recent, so I'm not sure it's stable and speed optimized. Do you actually 
can advise it? I can do it in the next refactor commit as well.



Comment at: llvm/lib/Support/TimeProfiler.cpp:44
+default:
+  if (isPrint(C)) {
+OS += C;

takuto.ikuta wrote:
> include StringExtras.h for this?
Should one do it? It's already implicitly included.


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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev updated this revision to Diff 192268.
anton-afanasyev marked an inline comment as done.

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

https://reviews.llvm.org/D58675

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/TimeProfiler.h
  llvm/lib/IR/LegacyPassManager.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/TimeProfiler.cpp

Index: llvm/lib/Support/TimeProfiler.cpp
===
--- /dev/null
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -0,0 +1,184 @@
+//===-- TimeProfiler.cpp - Hierarchical Time Profiler -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+/// \file Hierarchical time profiler implementation.
+//
+//===--===//
+
+#include "llvm/Support/TimeProfiler.h"
+#include "llvm/Support/FileSystem.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace std::chrono;
+
+namespace llvm {
+
+TimeTraceProfiler *TimeTraceProfilerInstance = nullptr;
+
+static std::string escapeString(StringRef Src) {
+  std::string OS;
+  for (const unsigned char : Src) {
+switch (C) {
+case '"':
+case '/':
+case '\\':
+case '\b':
+case '\f':
+case '\n':
+case '\r':
+case '\t':
+  OS += '\\';
+  OS += C;
+  break;
+default:
+  if (isPrint(C)) {
+OS += C;
+  }
+}
+  }
+  return OS;
+}
+
+typedef duration DurationType;
+typedef std::pair NameAndDuration;
+
+struct Entry {
+  time_point Start;
+  DurationType Duration;
+  std::string Name;
+  std::string Detail;
+};
+
+struct TimeTraceProfiler {
+  TimeTraceProfiler() {
+Stack.reserve(8);
+Entries.reserve(128);
+StartTime = steady_clock::now();
+  }
+
+  void begin(std::string Name, llvm::function_ref Detail) {
+Entry E = {steady_clock::now(), {}, Name, Detail()};
+Stack.push_back(std::move(E));
+  }
+
+  void end() {
+assert(!Stack.empty() && "Must call Begin first");
+auto  = Stack.back();
+E.Duration = steady_clock::now() - E.Start;
+
+// Only include sections longer than 500us.
+if (duration_cast(E.Duration).count() > 500)
+  Entries.emplace_back(E);
+
+// Track total time taken by each "name", but only the topmost levels of
+// them; e.g. if there's a template instantiation that instantiates other
+// templates from within, we only want to add the topmost one. "topmost"
+// happens to be the ones that don't have any currently open entries above
+// itself.
+if (std::find_if(++Stack.rbegin(), Stack.rend(), [&](const Entry ) {
+  return Val.Name == E.Name;
+}) == Stack.rend()) {
+  TotalPerName[E.Name] += E.Duration;
+  CountPerName[E.Name]++;
+}
+
+Stack.pop_back();
+  }
+
+  void Write(std::unique_ptr ) {
+assert(Stack.empty() &&
+   "All profiler sections should be ended when calling Write");
+
+*OS << "{ \"traceEvents\": [\n";
+
+// Emit all events for the main flame graph.
+for (const auto  : Entries) {
+  auto StartUs = duration_cast(E.Start - StartTime).count();
+  auto DurUs = duration_cast(E.Duration).count();
+  *OS << "{ \"pid\":1, \"tid\":0, \"ph\":\"X\", \"ts\":" << StartUs
+  << ", \"dur\":" << DurUs << ", \"name\":\""
+  << escapeString(E.Name) << "\", \"args\":{ \"detail\":\""
+  << escapeString(E.Detail) << "\"} },\n";
+}
+
+// Emit totals by section name as additional "thread" events, sorted from
+// longest one.
+int Tid = 1;
+std::vector SortedTotals;
+SortedTotals.reserve(TotalPerName.size());
+for (const auto  : TotalPerName) {
+  SortedTotals.push_back(E);
+}
+std::sort(SortedTotals.begin(), SortedTotals.end(),
+  [](const NameAndDuration , const NameAndDuration ) {
+return A.second > B.second;
+  });
+for (const auto  : SortedTotals) {
+  auto DurUs = duration_cast(E.second).count();
+  *OS << "{ \"pid\":1, \"tid\":" << Tid << ", \"ph\":\"X\", \"ts\":" << 0
+  << ", \"dur\":" << DurUs 

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Support/TimeProfiler.cpp:33
+switch (C) {
+case '"':
+case '\\':

takuto.ikuta wrote:
> Include escape for '/' too.
> https://tools.ietf.org/html/rfc8259#section-7
Yes, thanks!


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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

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

Updated following @rnk review notes.


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

https://reviews.llvm.org/D58675

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/TimeProfiler.h
  llvm/lib/IR/LegacyPassManager.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/TimeProfiler.cpp

Index: llvm/lib/Support/TimeProfiler.cpp
===
--- /dev/null
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -0,0 +1,185 @@
+//===-- TimeProfiler.cpp - Hierarchical Time Profiler -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+/// \file Hierarchical time profiler implementation.
+//
+//===--===//
+
+#include "llvm/Support/TimeProfiler.h"
+#include "llvm/Support/FileSystem.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace std::chrono;
+
+namespace llvm {
+
+TimeTraceProfiler *TimeTraceProfilerInstance = nullptr;
+
+static std::string escapeString(const char *Src) {
+  std::string OS;
+  while (*Src) {
+char C = *Src;
+switch (C) {
+case '"':
+case '\\':
+case '\b':
+case '\f':
+case '\n':
+case '\r':
+case '\t':
+  OS += '\\';
+  OS += C;
+  break;
+default:
+  if (C >= 32 && C < 126) {
+OS += C;
+  }
+}
+++Src;
+  }
+  return OS;
+}
+
+typedef duration DurationType;
+typedef std::pair NameAndDuration;
+
+struct Entry {
+  time_point Start;
+  DurationType Duration;
+  std::string Name;
+  llvm::function_ref Detail;
+};
+
+struct TimeTraceProfiler {
+  TimeTraceProfiler() {
+Stack.reserve(8);
+Entries.reserve(128);
+StartTime = steady_clock::now();
+  }
+
+  void begin(const std::string , llvm::function_ref Detail) {
+Entry E = {steady_clock::now(), {}, Name, Detail};
+Stack.emplace_back(E);
+  }
+
+  void end() {
+assert(!Stack.empty() && "Must call Begin first");
+auto  = Stack.back();
+E.Duration = steady_clock::now() - E.Start;
+
+// Only include sections longer than 500us.
+if (duration_cast(E.Duration).count() > 500)
+  Entries.emplace_back(E);
+
+// Track total time taken by each "name", but only the topmost levels of
+// them; e.g. if there's a template instantiation that instantiates other
+// templates from within, we only want to add the topmost one. "topmost"
+// happens to be the ones that don't have any currently open entries above
+// itself.
+if (std::find_if(++Stack.rbegin(), Stack.rend(), [&](const Entry ) {
+  return Val.Name == E.Name;
+}) == Stack.rend()) {
+  TotalPerName[E.Name] += E.Duration;
+  CountPerName[E.Name]++;
+}
+
+Stack.pop_back();
+  }
+
+  void Write(std::unique_ptr ) {
+assert(Stack.empty() &&
+   "All profiler sections should be ended when calling Write");
+
+*OS << "{ \"traceEvents\": [\n";
+
+// Emit all events for the main flame graph.
+for (const auto  : Entries) {
+  auto StartUs = duration_cast(E.Start - StartTime).count();
+  auto DurUs = duration_cast(E.Duration).count();
+  *OS << "{ \"pid\":1, \"tid\":0, \"ph\":\"X\", \"ts\":" << StartUs
+  << ", \"dur\":" << DurUs << ", \"name\":\""
+  << escapeString(E.Name.c_str()) << "\", \"args\":{ \"detail\":\""
+  << escapeString(E.Detail().str().c_str()) << "\"} },\n";
+}
+
+// Emit totals by section name as additional "thread" events, sorted from
+// longest one.
+int Tid = 1;
+std::vector SortedTotals;
+SortedTotals.reserve(TotalPerName.size());
+for (const auto  : TotalPerName) {
+  SortedTotals.push_back(E);
+}
+std::sort(SortedTotals.begin(), SortedTotals.end(),
+  [](const NameAndDuration , const NameAndDuration ) {
+return A.second > B.second;
+  });
+for (const auto  : SortedTotals) {
+  auto DurUs = duration_cast(E.second).count();
+  *OS << 

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

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



Comment at: llvm/lib/IR/LegacyPassManager.cpp:1632-1634
+  bool profileTime = llvm::TimeTraceProfilerEnabled();
+  if (profileTime)
+llvm::TimeTraceProfilerBegin("OptFunction", F.getName().data());

anton-afanasyev wrote:
> rnk wrote:
> > Someone is going to have to figure out where the equivalent annotations 
> > should live in the new passmanager. I wasn't able to find it just by 
> > looking myself, so I won't ask you to do it. I guess it's in 
> > llvm/include/llvm/IR/PassManager.h.
> Ok, I'm to look for it.
Looks like PassManager.h changing is enough to support new PassManager. But I 
prefer to make this change in subsequent commits.



Comment at: llvm/lib/IR/LegacyPassManager.cpp:1686
 
+  llvm::TimeTraceScope timeScope("OptModule", M.getName().data());
   for (Function  : M)

rnk wrote:
> I think these OptModule and OptFunction labels may need some improvement. For 
> a backend-heavy compilation like LLVM's X86ISelLowering.cpp, these labels 
> aren't as good as they could be: {F8455962}
> I think it's fine to leave that for later, though.
Yes, leaving this for later. The subsequent commits are planned.



Comment at: llvm/lib/Support/TimeProfiler.cpp:70
+
+  void Begin(const std::string , const std::string ) {
+Entry e = {steady_clock::now(), {}, name, detail};

rnk wrote:
> I'm tempted to micro-optimize this with StringRef and StringSaver, but I 
> think it's unnecessary. Calling malloc may affect the profile, but probably 
> not much.
Ok, I'm not changing this.


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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

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



Comment at: clang/lib/Parse/ParseAST.cpp:172
+  {
+llvm::TimeTraceScope scope("Backend", "");
+Consumer->HandleTranslationUnit(S.getASTContext());

rnk wrote:
> I think you may want to move this to `clang::EmitBackendOutput`, which is 
> closer to real "backend-y" actions. I don't think there's any other heavy 
> lifting that happens in HandleTranslationUnit, it looks like diagnostic 
> teardown and setup for calling EmitBackendOutput.
Yes, thanks.



Comment at: clang/lib/Sema/Sema.cpp:98
+if (llvm::TimeTraceProfilerEnabled()) {
+  auto fe = SM.getFileEntryForID(SM.getFileID(Loc));
+  llvm::TimeTraceProfilerBegin(

rnk wrote:
> This doesn't match the LLVM naming and auto usage conventions: 
> https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
> 
> Yes, there is an active debate about changing the way we name variables, but 
> please just match what we have for now.
Ok



Comment at: llvm/lib/IR/LegacyPassManager.cpp:1632-1634
+  bool profileTime = llvm::TimeTraceProfilerEnabled();
+  if (profileTime)
+llvm::TimeTraceProfilerBegin("OptFunction", F.getName().data());

rnk wrote:
> Someone is going to have to figure out where the equivalent annotations 
> should live in the new passmanager. I wasn't able to find it just by looking 
> myself, so I won't ask you to do it. I guess it's in 
> llvm/include/llvm/IR/PassManager.h.
Ok, I'm to look for it.



Comment at: llvm/lib/Support/TimeProfiler.cpp:28
+
+static std::string EscapeString(const char *src) {
+  std::string os;

rnk wrote:
> Here and else where things should be named the LLVM way for consistency:
> https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
> `escapeString`, `Src`, etc. Tedious, I know.
Ok



Comment at: llvm/lib/Support/TimeProfiler.h:1
+//===- llvm/Support/TimeProfiler.h - Hierarchical Time Profiler -*- C++ 
-*-===//
+//

rnk wrote:
> I applied this patch locally to try it, and I noticed this header should be 
> in llvm/include/llvm/Support, not llvm/lib/Support.
Oops, you're right, this is accidentally moved in patch! Thanks.


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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev added a comment.

Ping!
Should I add more FE guys to review this?


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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev updated this revision to Diff 11.

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

https://reviews.llvm.org/D58675

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/tools/driver/cc1_main.cpp
  llvm/lib/IR/LegacyPassManager.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/TimeProfiler.cpp
  llvm/lib/Support/TimeProfiler.h

Index: llvm/lib/Support/TimeProfiler.h
===
--- /dev/null
+++ llvm/lib/Support/TimeProfiler.h
@@ -0,0 +1,71 @@
+//===- llvm/Support/TimeProfiler.h - Hierarchical Time Profiler -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_SUPPORT_TIME_PROFILER_H
+#define LLVM_SUPPORT_TIME_PROFILER_H
+
+#include "llvm/Support/raw_ostream.h"
+
+namespace llvm {
+
+struct TimeTraceProfiler;
+extern TimeTraceProfiler *TimeTraceProfilerInstance;
+
+/// Initialize the time trace profiler.
+/// This sets up the global \p TimeTraceProfilerInstance
+/// variable to be the profiler instance.
+void TimeTraceProfilerInitialize();
+
+/// Cleanup the time trace profiler, if it was initialized.
+void TimeTraceProfilerCleanup();
+
+/// Is the time trace profiler enabled, i.e. initialized?
+inline bool TimeTraceProfilerEnabled() {
+  return TimeTraceProfilerInstance != nullptr;
+}
+
+/// Write profiling data to output file.
+/// Data produced is JSON, in Chrome "Trace Event" format, see
+/// https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview
+void TimeTraceProfilerWrite(std::unique_ptr );
+
+/// Manually begin a time section, with the given \p name and \p detail.
+/// Profiler copies the string data, so the pointers can be given into
+/// temporaries. Time sections can be hierarchical; every Begin must have a
+/// matching End pair but they can nest.
+void TimeTraceProfilerBegin(const char *name, const char *detail);
+
+/// Manually end the last time section.
+void TimeTraceProfilerEnd();
+
+/// The TimeTraceScope is a helper class to call the begin and end functions.
+/// of the time trace profiler.  When the object is constructed, it
+/// begins the section; and wen it is destroyed, it stops
+/// it.  If the time profiler is not initialized, the overhead
+/// is a single branch.
+struct TimeTraceScope {
+  TimeTraceScope(const char *name, const char *detail) {
+if (TimeTraceProfilerInstance != nullptr)
+  TimeTraceProfilerBegin(name, detail);
+  }
+  ~TimeTraceScope() {
+if (TimeTraceProfilerInstance != nullptr)
+  TimeTraceProfilerEnd();
+  }
+};
+
+/// Evaluates expression if time trace profiler is enabled, or passed null when
+/// it is not. Useful to avoid possibly expensive work in creating a string for
+/// profiling, when profiler is not enabled at all.
+#define TIME_TRACE_OR_NULL(expr)   \
+  (llvm::TimeTraceProfilerInstance != nullptr ? (expr) : nullptr)
+
+} // end namespace llvm
+
+#endif
Index: llvm/lib/Support/TimeProfiler.cpp
===
--- /dev/null
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -0,0 +1,178 @@
+//===-- TimeProfiler.cpp - Hierarchical Time Profiler -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+/// \file Hierarchical time profiler implementation.
+//
+//===--===//
+
+#include "llvm/Support/TimeProfiler.h"
+#include "llvm/Support/FileSystem.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace std::chrono;
+
+namespace llvm {
+
+TimeTraceProfiler *TimeTraceProfilerInstance = nullptr;
+
+static std::string EscapeString(const char *src) {
+  std::string os;
+  while (*src) {
+char c = *src;
+switch (c) {
+case '"':
+case '\\':
+case '\b':
+case '\f':
+case '\n':
+case '\r':
+case '\t':
+  os += '\\';
+  os += c;
+  break;
+default:
+  if (c >= 32 && c < 126) {
+os 

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:23
 #include "clang/Sema/Lookup.h"
+#include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"

RKSimon wrote:
> anton-afanasyev wrote:
> > RKSimon wrote:
> > > nfc change?
> > What do you mean? This change is not NFC.
> Moving the "clang/Sema/SemaInternal.h" include?
Ok, I'm to move this line back. This was done by clang-format.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58675



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