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

2019-07-24 Thread 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&id=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] D65202: [Support] Fix `-ftime-trace-granularity` option

2019-07-24 Thread 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

2019-07-24 Thread 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

2019-07-24 Thread 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

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

2019-07-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Looks ok




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

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


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

2019-07-24 Thread 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

2019-07-24 Thread 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: clang/include/clang/Frontend/FrontendO

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

2019-07-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



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

is there any possibility of wanting a granularity <1ms in the future? This sets 
up a backward-compatibility trap if so.



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);

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)`



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

why does this have a default value?  It shouldn't be possible to use it without 
overwriting it, IIUC



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

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


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

2019-07-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



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

While there add a description to `-ftime-report` and document how they are 
different?



Comment at: clang/include/clang/Driver/Options.td:1759-1761
 def ftime_report : Flag<["-"], "ftime-report">, Group, 
Flags<[CC1Option]>;
-def ftime_trace : Flag<["-"], "ftime-trace">, Group, 
Flags<[CC1Option, CoreOption]>;
+def ftime_trace : Flag<["-"], "ftime-trace">, Group,
+  HelpText<"Turn on time profiler">, Flags<[CC1Option, CoreOption]>;

While there add description to `ftime_report` and document their difference?



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

Can `TimeTraceGranularity` perhaps be stored in `TimeTraceProfilerInstance`?


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

2019-07-24 Thread 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: clang/inc