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

[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