Re: [PATCH] Add -fsarif-time-report [PR109361]

[Richard Biener <richard.guenther@gmail.com>]

On Fri, Jul 28, 2023 at 12:23 AM David Malcolm via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Tue, 2023-04-11 at 08:43 +0000, Richard Biener wrote:
> > On Tue, 4 Apr 2023, David Malcolm wrote:
> >
> > > Richi, Jakub: I can probably self-approve this, but it's
> > > technically a
> > > new feature.  OK if I push this to trunk in stage 4?  I believe
> > > it's
> > > low risk, and is very useful for benchmarking -fanalyzer.
> >
> > Please wait for stage1 at this point.  One comment on the patch
> > below ...
> >
> > >
> > > This patch adds support for embeddding profiling information about
> > > the
> > > compiler itself into the SARIF output.
> > >
> > > In an earlier version of this patch I extended -ftime-report so
> > > that
> > > as well as writing to stderr, it would embed the information in any
> > > SARIF output.  This turned out to be awkward to use, in that I
> > > found
> > > myself needing to get the data in JSON form without also having it
> > > emitted on stderr (which was affecting the output of the build).
> > >
> > > Hence this version of the patch adds a new -fsarif-time-report,
> > > similar
> > > to the existing -ftime-report for requesting GCC profile itself
> > > using
> > > the timevar machinery.
> > >
> > > Specifically, if -fsarif-time-report is specified, the timing
> > > information will be captured (as if -ftime-report were specified),
> > > and
> > > will be embedded in JSON form within any SARIF as a
> > > "gcc/timeReport"
> > > property within a property bag of the "invocation" object.
> > >
> > > Here's an example of the output:
> > >
> > >   "invocations": [
> > >       {
> > >           "executionSuccessful": true,
> > >           "toolExecutionNotifications": [],
> > >           "properties": {
> > >               "gcc/timeReport": {
> > >                   "timevars": [
> > >                       {
> > >                           "name": "phase setup",
> > >                           "elapsed": {
> > >                               "user": 0.04,
> > >                               "sys": 0,
> > >                               "wall": 0.04,
> > >                               "ggc_mem": 1863472
> > >                           }
> > >                       },
> > >
> > >                       [...snip...]
> > >
> > >                       {
> > >                           "name": "analyzer: processing worklist",
> > >                           "elapsed": {
> > >                               "user": 0.06,
> > >                               "sys": 0,
> > >                               "wall": 0.06,
> > >                               "ggc_mem": 48
> > >                           }
> > >                       },
> > >                       {
> > >                           "name": "analyzer: emitting diagnostics",
> > >                           "elapsed": {
> > >                               "user": 0.01,
> > >                               "sys": 0,
> > >                               "wall": 0.01,
> > >                               "ggc_mem": 0
> > >                           }
> > >                       },
> > >                       {
> > >                           "name": "TOTAL",
> > >                           "elapsed": {
> > >                               "user": 0.21,
> > >                               "sys": 0.03,
> > >                               "wall": 0.24,
> > >                               "ggc_mem": 3368736
> > >                           }
> > >                       }
> > >                   ],
> > >                   "CHECKING_P": true,
> > >                   "flag_checking": true
> > >               }
> > >           }
> > >       }
> > >   ]
> > >
> > > I have successfully used this in my analyzer integration tests to
> > > get
> > > timing information about which source files get slowed down by the
> > > analyzer.  I've validated the generated .sarif files against the
> > > SARIF
> > > schema.
> > >
> > > The documentation notes that the precise output format is subject
> > > to change.
> > >
> > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> > >
> > > gcc/ChangeLog:
> > >         PR analyzer/109361
> > >         * common.opt (fsarif-time-report): New option.
> >
> > 'sarif' is currently used only with -fdiagnostics-format= it seems.
> > We already have
> >
> > ftime-report
> > Common Var(time_report)
> > Report the time taken by each compiler pass.
> >
> > ftime-report-details
> > Common Var(time_report_details)
> > Record times taken by sub-phases separately.
> >
> > so -fsarif-time-report is not a) -ftime-report-sarif and b) it's
> > unclear if it applies to -ftime-report or to both -ftime-report
> > and -ftime-report-details?  (note -ftime-report-details needs
> > -ftime-report to be effective)
> >
> > I'd rather have a -ftime-report-format= (or -freport-format in
> > case we want to cover -fmem-report, -fmem-report-wpa,
> > -fpre-ipa-mem-report and -fpost-ipa-mem-report as well?)
> >
> > ISTR there's a summer of code project in this are as well.
> >
> > Thanks,
> > Richard.
>
> Revisiting this; sorry about the delay.
>
> As I understand the status quo, we currently have:
> * -ftime-report: enable capturing of timing information (with a slight
> speed hit), and report it to stderr
> * -ftime-report-details: tweak how that information is captured (if -
> ftime-report is enabled), so that timevar->children is populated and
> printed
>
> There seem to be two things here:
> - what timing data we capture
> - where that timing data goes
>
> What I need is to some way to specify that the output should go to the
> SARIF file, rather than to stderr.
>
> Some ways we could do this:
> (a) simply enforce that if SARIF diagnostics were requested with -
> fdiagnostics-format=sarif-{file|stderr} that the time report goes there
> in JSON form, rather than to stderr
> (b) add an option to specify where the time report goes
> (c) add options to allow the time report to potentially go to multiple
> places (both stderr and SARIF, one or the other, neither); this seems
> overcomplex to me.
> (d) something else?
>
> The patch I posted implements a form of (b), but right now I'm leaning
> towards option (a): if the user requested SARIF output, then the time
> report goes to the SARIF output, rather than stderr.

I'm fine with (a), but -fdiagnostics-format= doesn't naturally apply to
-ftime-report (or -fmem-report), those are not "diagnostics" in my
opinion but they are auxiliary data for the compilation process
rather than the input to it.  But yes, -ftime-report-format= would be
too specific, maybe -faux-format=.

That said, we can go with (a) and do something else later if desired.
I don't think preserving behavior in this area will be important so we
don't have to get it right immediately.

Richard.

>
> Thoughts?
> Dave
>
>
> >
> > >         * diagnostic-client-data-hooks.h (class sarif_object): New
> > > forward
> > >         decl.
> > >         (diagnostic_client_data_hooks::add_sarif_invocation_propert
> > > ies):
> > >         New vfunc.
> > >         * diagnostic-format-sarif.cc: Include "diagnostic-format-
> > > sarif.h".
> > >         (class sarif_invocation): Inherit from sarif_object rather
> > >         than json::object.
> > >         (class sarif_result): Likewise.
> > >         (class sarif_ice_notification): Likewise.
> > >         (sarif_object::get_or_create_properties): New.
> > >         (sarif_invocation::prepare_to_flush): Add "context" param.
> > > Use it
> > >         to call the context's add_sarif_invocation_properties hook.
> > >         (sarif_builder::flush_to_file): Pass m_context to
> > >         sarif_invocation::prepare_to_flush.
> > >         * diagnostic-format-sarif.h: New header.
> > >         * doc/invoke.texi (Developer Options): Add -fsarif-time-
> > > report.
> > >         * timevar.cc: Include "json.h".
> > >         (timer::named_items::m_hash_map): Split out type into...
> > >         (timer::named_items::hash_map_t): ...this new typedef.
> > >         (timer::named_items::make_json): New function.
> > >         (timevar_diff): New function.
> > >         (make_json_for_timevar_time_def): New function.
> > >         (timer::timevar_def::make_json): New function.
> > >         (timer::make_json): New function.
> > >         * timevar.h (class json::value): New forward decl.
> > >         (timer::make_json): New decl.
> > >         (timer::timevar_def::make_json): New decl.
> > >         * toplev.cc (toplev::~toplev): Guard the call to
> > > timer::print so
> > >         that it doesn't happen on just -fsarif-time-report.
> > >         (toplev::start_timevars): Add sarif_time_report to the
> > > flags that
> > >         can lead to timevar_init being called.
> > >         * tree-diagnostic-client-data-hooks.cc: Include
> > >         "diagnostic-format-sarif.h" and "timevar.h".
> > >         (compiler_data_hooks::add_sarif_invocation_properties): New
> > > vfunc
> > >         implementation.
> > >
> > > gcc/testsuite/ChangeLog:
> > >         PR analyzer/109361
> > >         * c-c++-common/diagnostic-format-sarif-file-timevars-1.c:
> > > New test.
> > >         * c-c++-common/diagnostic-format-sarif-file-timevars-2.c:
> > > New test.
> > >
> > > Signed-off-by: David Malcolm <dmalcolm@redhat.com>
> > > ---
> > >  gcc/common.opt                                |   4 +
> > >  gcc/diagnostic-client-data-hooks.h            |   6 +
> > >  gcc/diagnostic-format-sarif.cc                |  45 +++--
> > >  gcc/diagnostic-format-sarif.h                 |  45 +++++
> > >  gcc/doc/invoke.texi                           |  11 +-
> > >  .../diagnostic-format-sarif-file-timevars-1.c |  18 ++
> > >  .../diagnostic-format-sarif-file-timevars-2.c |  21 +++
> > >  gcc/timevar.cc                                | 164
> > > +++++++++++++++++-
> > >  gcc/timevar.h                                 |   5 +
> > >  gcc/toplev.cc                                 |  10 +-
> > >  gcc/tree-diagnostic-client-data-hooks.cc      |  18 +-
> > >  11 files changed, 330 insertions(+), 17 deletions(-)
> > >  create mode 100644 gcc/diagnostic-format-sarif.h
> > >  create mode 100644 gcc/testsuite/c-c++-common/diagnostic-format-
> > > sarif-file-timevars-1.c
> > >  create mode 100644 gcc/testsuite/c-c++-common/diagnostic-format-
> > > sarif-file-timevars-2.c
> > >
> > > diff --git a/gcc/common.opt b/gcc/common.opt
> > > index e558385c7f4..cf1f696cac2 100644
> > > --- a/gcc/common.opt
> > > +++ b/gcc/common.opt
> > > @@ -2519,6 +2519,10 @@ frename-registers
> > >  Common Var(flag_rename_registers) Optimization EnabledBy(funroll-
> > > loops)
> > >  Perform a register renaming optimization pass.
> > >
> > > +fsarif-time-report
> > > +Common Var(sarif_time_report)
> > > +Report the time taken by each compiler pass in any SARIF output.
> > > +
> > >  fschedule-fusion
> > >  Common Var(flag_schedule_fusion) Init(2) Optimization
> > >  Perform a target dependent instruction fusion optimization pass.
> > > diff --git a/gcc/diagnostic-client-data-hooks.h b/gcc/diagnostic-
> > > client-data-hooks.h
> > > index 5f8b9a25294..e3f2d056207 100644
> > > --- a/gcc/diagnostic-client-data-hooks.h
> > > +++ b/gcc/diagnostic-client-data-hooks.h
> > > @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3.  If not
> > > see
> > >  #ifndef GCC_DIAGNOSTIC_CLIENT_DATA_HOOKS_H
> > >  #define GCC_DIAGNOSTIC_CLIENT_DATA_HOOKS_H
> > >
> > > +class sarif_object;
> > >  class client_version_info;
> > >
> > >  /* A bundle of additional metadata, owned by the
> > > diagnostic_context,
> > > @@ -41,6 +42,11 @@ class diagnostic_client_data_hooks
> > >       See SARIF v2.1.0 Appendix J for suggested values.  */
> > >    virtual const char *
> > >    maybe_get_sarif_source_language (const char *filename) const =
> > > 0;
> > > +
> > > +  /* Hook to allow client to populate a SARIF "invocation" object
> > > with
> > > +     a custom property bag (see SARIF v2.1.0 section 3.8).  */
> > > +  virtual void
> > > +  add_sarif_invocation_properties (sarif_object &invocation_obj)
> > > const = 0;
> > >  };
> > >
> > >  /* Factory function for making an instance of
> > > diagnostic_client_data_hooks
> > > diff --git a/gcc/diagnostic-format-sarif.cc b/gcc/diagnostic-
> > > format-sarif.cc
> > > index fd29ac2ca3b..f88f429be58 100644
> > > --- a/gcc/diagnostic-format-sarif.cc
> > > +++ b/gcc/diagnostic-format-sarif.cc
> > > @@ -29,13 +29,14 @@ along with GCC; see the file COPYING3.  If not
> > > see
> > >  #include "cpplib.h"
> > >  #include "logical-location.h"
> > >  #include "diagnostic-client-data-hooks.h"
> > > +#include "diagnostic-format-sarif.h"
> > >
> > >  class sarif_builder;
> > >
> > >  /* Subclass of json::object for SARIF invocation objects
> > >     (SARIF v2.1.0 section 3.20).  */
> > >
> > > -class sarif_invocation : public json::object
> > > +class sarif_invocation : public sarif_object
> > >  {
> > >  public:
> > >    sarif_invocation ()
> > > @@ -46,17 +47,17 @@ public:
> > >    void add_notification_for_ice (diagnostic_context *context,
> > >                                  diagnostic_info *diagnostic,
> > >                                  sarif_builder *builder);
> > > -  void prepare_to_flush ();
> > > +  void prepare_to_flush (diagnostic_context *context);
> > >
> > >  private:
> > >    json::array *m_notifications_arr;
> > >    bool m_success;
> > >  };
> > >
> > > -/* Subclass of json::object for SARIF result objects
> > > +/* Subclass of sarif_object for SARIF result objects
> > >     (SARIF v2.1.0 section 3.27).  */
> > >
> > > -class sarif_result : public json::object
> > > +class sarif_result : public sarif_object
> > >  {
> > >  public:
> > >    sarif_result () : m_related_locations_arr (NULL) {}
> > > @@ -71,13 +72,13 @@ private:
> > >    json::array *m_related_locations_arr;
> > >  };
> > >
> > > -/* Subclass of json::object for SARIF notification objects
> > > +/* Subclass of sarif_object for SARIF notification objects
> > >     (SARIF v2.1.0 section 3.58).
> > >
> > >     This subclass is specifically for notifying when an
> > >     internal compiler error occurs.  */
> > >
> > > -class sarif_ice_notification : public json::object
> > > +class sarif_ice_notification : public sarif_object
> > >  {
> > >  public:
> > >    sarif_ice_notification (diagnostic_context *context,
> > > @@ -220,7 +221,24 @@ private:
> > >
> > >  static sarif_builder *the_builder;
> > >
> > > -/* class sarif_invocation : public json::object.  */
> > > +/* class sarif_object : public json::object.  */
> > > +
> > > +sarif_property_bag &
> > > +sarif_object::get_or_create_properties ()
> > > +{
> > > +  json::value *properties_val = get ("properties");
> > > +  if (properties_val)
> > > +    {
> > > +      if (properties_val->get_kind () == json::JSON_OBJECT)
> > > +       return *static_cast <sarif_property_bag *>
> > > (properties_val);
> > > +    }
> > > +
> > > +  sarif_property_bag *bag = new sarif_property_bag ();
> > > +  set ("properties", bag);
> > > +  return *bag;
> > > +}
> > > +
> > > +/* class sarif_invocation : public sarif_object.  */
> > >
> > >  /* Handle an internal compiler error DIAGNOSTIC occurring on
> > > CONTEXT.
> > >     Add an object representing the ICE to the notifications array.
> > > */
> > > @@ -238,16 +256,21 @@ sarif_invocation::add_notification_for_ice
> > > (diagnostic_context *context,
> > >  }
> > >
> > >  void
> > > -sarif_invocation::prepare_to_flush ()
> > > +sarif_invocation::prepare_to_flush (diagnostic_context *context)
> > >  {
> > >    /* "executionSuccessful" property (SARIF v2.1.0 section
> > > 3.20.14).  */
> > >    set ("executionSuccessful", new json::literal (m_success));
> > >
> > >    /* "toolExecutionNotifications" property (SARIF v2.1.0 section
> > > 3.20.21).  */
> > >    set ("toolExecutionNotifications", m_notifications_arr);
> > > +
> > > +  /* Call client hook, allowing it to create a custom property bag
> > > for
> > > +     this object (SARIF v2.1.0 section 3.8) e.g. for recording
> > > time vars.  */
> > > +  if (context->m_client_data_hooks)
> > > +    context->m_client_data_hooks->add_sarif_invocation_properties
> > > (*this);
> > >  }
> > >
> > > -/* class sarif_result : public json::object.  */
> > > +/* class sarif_result : public sarif_object.  */
> > >
> > >  /* Handle secondary diagnostics that occur within a diagnostic
> > > group.
> > >     The closest SARIF seems to have to nested diagnostics is the
> > > @@ -280,7 +303,7 @@ sarif_result::on_nested_diagnostic
> > > (diagnostic_context *context,
> > >    m_related_locations_arr->append (location_obj);
> > >  }
> > >
> > > -/* class sarif_ice_notification : public json::object.  */
> > > +/* class sarif_ice_notification : public sarif_object.  */
> > >
> > >  /* sarif_ice_notification's ctor.
> > >     DIAGNOSTIC is an internal compiler error.  */
> > > @@ -364,7 +387,7 @@ sarif_builder::end_group ()
> > >  void
> > >  sarif_builder::flush_to_file (FILE *outf)
> > >  {
> > > -  m_invocation_obj->prepare_to_flush ();
> > > +  m_invocation_obj->prepare_to_flush (m_context);
> > >    json::object *top = make_top_level_object (m_invocation_obj,
> > > m_results_array);
> > >    top->dump (outf);
> > >    m_invocation_obj = NULL;
> > > diff --git a/gcc/diagnostic-format-sarif.h b/gcc/diagnostic-format-
> > > sarif.h
> > > new file mode 100644
> > > index 00000000000..82ed9b9ee44
> > > --- /dev/null
> > > +++ b/gcc/diagnostic-format-sarif.h
> > > @@ -0,0 +1,45 @@
> > > +/* SARIF output for diagnostics.
> > > +   Copyright (C) 2023 Free Software Foundation, Inc.
> > > +   Contributed by David Malcolm <dmalcolm@redhat.com>.
> > > +
> > > +This file is part of GCC.
> > > +
> > > +GCC is free software; you can redistribute it and/or modify it
> > > under
> > > +the terms of the GNU General Public License as published by the
> > > Free
> > > +Software Foundation; either version 3, or (at your option) any
> > > later
> > > +version.
> > > +
> > > +GCC is distributed in the hope that it will be useful, but WITHOUT
> > > ANY
> > > +WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > > +FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> > > License
> > > +for more details.
> > > +
> > > +You should have received a copy of the GNU General Public License
> > > +along with GCC; see the file COPYING3.  If not see
> > > +<http://www.gnu.org/licenses/>.  */
> > > +
> > > +#ifndef GCC_DIAGNOSTIC_FORMAT_SARIF_H
> > > +#define GCC_DIAGNOSTIC_FORMAT_SARIF_H
> > > +
> > > +#include "json.h"
> > > +
> > > +/* Concrete subclass of json::object for SARIF property bags
> > > +   (SARIF v2.1.0 section 3.8).  */
> > > +
> > > +class sarif_property_bag : public json::object
> > > +{
> > > +};
> > > +
> > > +/* Concrete subclass of json::object for SARIF objects that can
> > > +   contain property bags (as per SARIF v2.1.0 section 3.8.1, which
> > > has:
> > > +   "In addition to those properties that are explicitly
> > > documented, every
> > > +   object defined in this document MAY contain a property named
> > > properties
> > > +   whose value is a property bag.")  */
> > > +
> > > +class sarif_object : public json::object
> > > +{
> > > +public:
> > > +  sarif_property_bag &get_or_create_properties ();
> > > +};
> > > +
> > > +#endif /* ! GCC_DIAGNOSTIC_FORMAT_SARIF_H */
> > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> > > index def2df4584b..f43ba4f9dd5 100644
> > > --- a/gcc/doc/invoke.texi
> > > +++ b/gcc/doc/invoke.texi
> > > @@ -745,7 +745,7 @@ Objective-C and Objective-C++ Dialects}.
> > >  -fmem-report  -fpre-ipa-mem-report  -fpost-ipa-mem-report
> > >  -fopt-info  -fopt-info-@var{options}@r{[}=@var{file}@r{]}
> > >  -fmultiflags  -fprofile-report
> > > --frandom-seed=@var{string}  -fsched-verbose=@var{n}
> > > +-frandom-seed=@var{string}  -fsarif-time-report
> > > -fsched-verbose=@var{n}
> > >  -fsel-sched-verbose  -fsel-sched-dump-cfg  -fsel-sched-pipelining-
> > > verbose
> > >  -fstats  -fstack-usage  -ftime-report  -ftime-report-details
> > >  -fvar-tracking-assignments-toggle  -gtoggle
> > > @@ -19631,6 +19631,15 @@ computing CRC32).
> > >
> > >  The @var{string} should be different for every file you compile.
> > >
> > > +@opindex fsarif-time-report
> > > +@item -fsarif-time-report
> > > +Equivalent to @option{-ftime-report}, except the information is
> > > emitted
> > > +in JSON form as part of SARIF output (such as from
> > > +@option{-fdiagnostics-format=sarif-file}).  The precise format of
> > > the
> > > +JSON data is subject to change, and the values may not exactly
> > > match those
> > > +emitted by @option{-ftime-report} due to being written out at a
> > > slightly
> > > +different place within the compiler.
> > > +
> > >  @opindex save-temps
> > >  @item -save-temps
> > >  Store the usual ``temporary'' intermediate files permanently; name
> > > them
> > > diff --git a/gcc/testsuite/c-c++-common/diagnostic-format-sarif-
> > > file-timevars-1.c b/gcc/testsuite/c-c++-common/diagnostic-format-
> > > sarif-file-timevars-1.c
> > > new file mode 100644
> > > index 00000000000..2b87aed8147
> > > --- /dev/null
> > > +++ b/gcc/testsuite/c-c++-common/diagnostic-format-sarif-file-
> > > timevars-1.c
> > > @@ -0,0 +1,18 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-fdiagnostics-format=sarif-file -fsarif-time-
> > > report" } */
> > > +
> > > +#warning message
> > > +
> > > +/* Verify that some JSON was written to a file with the expected
> > > name.  */
> > > +/* { dg-final { verify-sarif-file } } */
> > > +
> > > +/* We expect various properties.
> > > +   The indentation here reflects the expected hierarchy, though
> > > these tests
> > > +   don't check for that, merely the string fragments we expect.
> > > +
> > > +   { dg-final { scan-sarif-file {"invocations": } } }
> > > +     { dg-final { scan-sarif-file {"properties": } } }
> > > +       { dg-final { scan-sarif-file {"gcc/timeReport": } } }
> > > +         { dg-final { scan-sarif-file {"timevars": } } }
> > > +           { dg-final { scan-sarif-file {"name": "TOTAL",} } }
> > > +*/
> > > diff --git a/gcc/testsuite/c-c++-common/diagnostic-format-sarif-
> > > file-timevars-2.c b/gcc/testsuite/c-c++-common/diagnostic-format-
> > > sarif-file-timevars-2.c
> > > new file mode 100644
> > > index 00000000000..fa48171aacd
> > > --- /dev/null
> > > +++ b/gcc/testsuite/c-c++-common/diagnostic-format-sarif-file-
> > > timevars-2.c
> > > @@ -0,0 +1,21 @@
> > > +/* As per diagnostic-format-sarif-file-timevars-1.c, but
> > > +   adding -ftime-report-details.  */
> > > +
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-fdiagnostics-format=sarif-file -fsarif-time-
> > > report -ftime-report-details" } */
> > > +
> > > +#warning message
> > > +
> > > +/* Verify that some JSON was written to a file with the expected
> > > name.  */
> > > +/* { dg-final { verify-sarif-file } } */
> > > +
> > > +/* We expect various properties.
> > > +   The indentation here reflects the expected hierarchy, though
> > > these tests
> > > +   don't check for that, merely the string fragments we expect.
> > > +
> > > +   { dg-final { scan-sarif-file {"invocations": } } }
> > > +     { dg-final { scan-sarif-file {"properties": } } }
> > > +       { dg-final { scan-sarif-file {"gcc/timeReport": } } }
> > > +         { dg-final { scan-sarif-file {"timevars": } } }
> > > +           { dg-final { scan-sarif-file {"name": "TOTAL",} } }
> > > +*/
> > > diff --git a/gcc/timevar.cc b/gcc/timevar.cc
> > > index d695297aae7..5368ff06ac9 100644
> > > --- a/gcc/timevar.cc
> > > +++ b/gcc/timevar.cc
> > > @@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not
> > > see
> > >  #include "coretypes.h"
> > >  #include "timevar.h"
> > >  #include "options.h"
> > > +#include "json.h"
> > >
> > >  #ifndef HAVE_CLOCK_T
> > >  typedef int clock_t;
> > > @@ -135,6 +136,8 @@ class timer::named_items
> > >    void pop ();
> > >    void print (FILE *fp, const timevar_time_def *total);
> > >
> > > +  json::value *make_json () const;
> > > +
> > >   private:
> > >    /* Which timer instance does this relate to?  */
> > >    timer *m_timer;
> > > @@ -143,7 +146,8 @@ class timer::named_items
> > >       Note that currently we merely store/compare the raw string
> > >       pointers provided by client code; we don't take a copy,
> > >       or use strcmp.  */
> > > -  hash_map <const char *, timer::timevar_def> m_hash_map;
> > > +  typedef hash_map <const char *, timer::timevar_def> hash_map_t;
> > > +  hash_map_t m_hash_map;
> > >
> > >    /* The order in which items were originally inserted.  */
> > >    auto_vec <const char *> m_names;
> > > @@ -207,6 +211,23 @@ timer::named_items::print (FILE *fp, const
> > > timevar_time_def *total)
> > >      }
> > >  }
> > >
> > > +/* Create a json value representing this object, suitable for use
> > > +   in SARIF output.  */
> > > +
> > > +json::value *
> > > +timer::named_items::make_json () const
> > > +{
> > > +  json::array *arr = new json::array ();
> > > +  for (const char *item_name : m_names)
> > > +    {
> > > +      hash_map_t &mut_map = const_cast <hash_map_t &>
> > > (m_hash_map);
> > > +      timer::timevar_def *def = mut_map.get (item_name);
> > > +      gcc_assert (def);
> > > +      arr->append (def->make_json ());
> > > +    }
> > > +  return arr;
> > > +}
> > > +
> > >  /* Fill the current times into TIME.  The definition of this
> > > function
> > >     also defines any or all of the HAVE_USER_TIME, HAVE_SYS_TIME,
> > > and
> > >     HAVE_WALL_TIME macros.  */
> > > @@ -251,6 +272,19 @@ timevar_accumulate (struct timevar_time_def
> > > *timer,
> > >    timer->ggc_mem += stop_time->ggc_mem - start_time->ggc_mem;
> > >  }
> > >
> > > +/* Get the difference between STOP_TIME and START_TIME.  */
> > > +
> > > +static void
> > > +timevar_diff (struct timevar_time_def *out,
> > > +             const timevar_time_def &start_time,
> > > +             const timevar_time_def &stop_time)
> > > +{
> > > +  out->user = stop_time.user - start_time.user;
> > > +  out->sys = stop_time.sys - start_time.sys;
> > > +  out->wall = stop_time.wall - start_time.wall;
> > > +  out->ggc_mem = stop_time.ggc_mem - start_time.ggc_mem;
> > > +}
> > > +
> > >  /* Class timer's constructor.  */
> > >
> > >  timer::timer () :
> > > @@ -791,6 +825,134 @@ timer::print (FILE *fp)
> > >    validate_phases (fp);
> > >  }
> > >
> > > +/* Create a json value representing this object, suitable for use
> > > +   in SARIF output.  */
> > > +
> > > +json::object *
> > > +make_json_for_timevar_time_def (const timevar_time_def &ttd)
> > > +{
> > > +  json::object *obj = new json::object ();
> > > +  obj->set ("user", new json::float_number (ttd.user));
> > > +  obj->set ("sys", new json::float_number (ttd.sys));
> > > +  obj->set ("wall", new json::float_number (ttd.wall));
> > > +  obj->set ("ggc_mem", new json::integer_number (ttd.ggc_mem));
> > > +  return obj;
> > > +}
> > > +
> > > +/* Create a json value representing this object, suitable for use
> > > +   in SARIF output.  */
> > > +
> > > +json::value *
> > > +timer::timevar_def::make_json () const
> > > +{
> > > +  json::object *timevar_obj = new json::object ();
> > > +  timevar_obj->set ("name", new json::string (name));
> > > +  timevar_obj->set ("elapsed", make_json_for_timevar_time_def
> > > (elapsed));
> > > +
> > > +  if (children)
> > > +    {
> > > +      bool any_children_with_time = false;
> > > +      for (auto i : *children)
> > > +       if (!all_zero (i.second))
> > > +         {
> > > +           any_children_with_time = true;
> > > +           break;
> > > +         }
> > > +      if (any_children_with_time)
> > > +       {
> > > +         json::array *children_arr = new json::array ();
> > > +         timevar_obj->set ("children", children_arr);
> > > +         for (auto i : *children)
> > > +           {
> > > +             /* Don't emit timing variables if we're going to get
> > > a row of
> > > +                zeroes.  */
> > > +             if (all_zero (i.second))
> > > +               continue;
> > > +             json::object *child_obj = new json::object;
> > > +             children_arr->append (child_obj);
> > > +             child_obj->set ("name", new json::string (i.first-
> > > >name));
> > > +             child_obj->set ("elapsed",
> > > +                             make_json_for_timevar_time_def
> > > (i.second));
> > > +           }
> > > +       }
> > > +    }
> > > +
> > > +  return timevar_obj;
> > > +}
> > > +
> > > +/* Create a json value representing this object, suitable for use
> > > +   in SARIF output.  */
> > > +
> > > +json::value *
> > > +timer::make_json () const
> > > +{
> > > +  /* Only generate stuff if we have some sort of time
> > > information.  */
> > > +#if defined (HAVE_USER_TIME) || defined (HAVE_SYS_TIME) || defined
> > > (HAVE_WALL_TIME)
> > > +  json::object *report_obj = new json::object ();
> > > +  json::array *json_arr = new json::array ();
> > > +  report_obj->set ("timevars", json_arr);
> > > +
> > > +  for (unsigned id = 0; id < (unsigned int) TIMEVAR_LAST; ++id)
> > > +    {
> > > +      const timevar_def *tv = &m_timevars[(timevar_id_t) id];
> > > +
> > > +      /* Don't print the total execution time here; this isn't
> > > initialized
> > > +        by the time the sarif output runs.  */
> > > +      if ((timevar_id_t) id == TV_TOTAL)
> > > +       continue;
> > > +
> > > +      /* Don't emit timing variables that were never used.  */
> > > +      if (!tv->used)
> > > +       continue;
> > > +
> > > +      bool any_children_with_time = false;
> > > +      if (tv->children)
> > > +       for (child_map_t::iterator i = tv->children->begin ();
> > > +            i != tv->children->end (); ++i)
> > > +         if (! all_zero ((*i).second))
> > > +           {
> > > +             any_children_with_time = true;
> > > +             break;
> > > +           }
> > > +
> > > +      /* Don't emit timing variables if we're going to get a row
> > > of
> > > +        zeroes.  Unless there are children with non-zero time.  */
> > > +      if (! any_children_with_time
> > > +         && all_zero (tv->elapsed))
> > > +       continue;
> > > +
> > > +      json_arr->append (tv->make_json ());
> > > +    }
> > > +
> > > +  /* Special-case for total.  */
> > > +  {
> > > +    /* Get our own total up till now, without affecting TV_TOTAL.
> > > */
> > > +    struct timevar_time_def total_now;
> > > +    struct timevar_time_def total_elapsed;
> > > +    get_time (&total_now);
> > > +    timevar_diff (&total_elapsed, m_timevars[TV_TOTAL].start_time,
> > > total_now);
> > > +
> > > +    json::object *total_obj = new json::object();
> > > +    json_arr->append (total_obj);
> > > +    total_obj->set ("name", new json::string ("TOTAL"));
> > > +    total_obj->set ("elapsed", make_json_for_timevar_time_def
> > > (total_elapsed));
> > > +  }
> > > +
> > > +  if (m_jit_client_items)
> > > +    report_obj->set ("client_items", m_jit_client_items->make_json
> > > ());
> > > +
> > > +  report_obj->set ("CHECKING_P", new json::literal
> > > ((bool)CHECKING_P));
> > > +  report_obj->set ("flag_checking", new json::literal
> > > ((bool)flag_checking));
> > > +
> > > +  return report_obj;
> > > +
> > > +#else /* defined (HAVE_USER_TIME) || defined (HAVE_SYS_TIME)
> > > +         || defined (HAVE_WALL_TIME) */
> > > +  return NULL;
> > > +#endif /* defined (HAVE_USER_TIME) || defined (HAVE_SYS_TIME)
> > > +         || defined (HAVE_WALL_TIME) */
> > > +}
> > > +
> > >  /* Get the name of the topmost item.  For use by jit for
> > > validating
> > >     inputs to gcc_jit_timer_pop.  */
> > >  const char *
> > > diff --git a/gcc/timevar.h b/gcc/timevar.h
> > > index ad465731609..e359e9fa1fa 100644
> > > --- a/gcc/timevar.h
> > > +++ b/gcc/timevar.h
> > > @@ -21,6 +21,8 @@
> > >  #ifndef GCC_TIMEVAR_H
> > >  #define GCC_TIMEVAR_H
> > >
> > > +namespace json { class value; }
> > > +
> > >  /* Timing variables are used to measure elapsed time in various
> > >     portions of the compiler.  Each measures elapsed user, system,
> > > and
> > >     wall-clock time, as appropriate to and supported by the host
> > > @@ -119,6 +121,7 @@ class timer
> > >    void pop_client_item ();
> > >
> > >    void print (FILE *fp);
> > > +  json::value *make_json () const;
> > >
> > >    const char *get_topmost_item_name () const;
> > >
> > > @@ -140,6 +143,8 @@ class timer
> > >    /* Private type: a timing variable.  */
> > >    struct timevar_def
> > >    {
> > > +    json::value *make_json () const;
> > > +
> > >      /* Elapsed time for this variable.  */
> > >      struct timevar_time_def elapsed;
> > >
> > > diff --git a/gcc/toplev.cc b/gcc/toplev.cc
> > > index 109c9d58cbd..5847efec346 100644
> > > --- a/gcc/toplev.cc
> > > +++ b/gcc/toplev.cc
> > > @@ -2151,7 +2151,10 @@ toplev::~toplev ()
> > >    if (g_timer && m_use_TV_TOTAL)
> > >      {
> > >        g_timer->stop (TV_TOTAL);
> > > -      g_timer->print (stderr);
> > > +      /* -fsarif-time-report doesn't lead to the output being
> > > printed
> > > +        to stderr; the other flags do.  */
> > > +      if (time_report || !quiet_flag || flag_detailed_statistics)
> > > +       g_timer->print (stderr);
> > >        delete g_timer;
> > >        g_timer = NULL;
> > >      }
> > > @@ -2163,7 +2166,10 @@ toplev::~toplev ()
> > >  void
> > >  toplev::start_timevars ()
> > >  {
> > > -  if (time_report || !quiet_flag  || flag_detailed_statistics)
> > > +  if (time_report
> > > +      || sarif_time_report
> > > +      || !quiet_flag
> > > +      || flag_detailed_statistics)
> > >      timevar_init ();
> > >
> > >    timevar_start (TV_TOTAL);
> > > diff --git a/gcc/tree-diagnostic-client-data-hooks.cc b/gcc/tree-
> > > diagnostic-client-data-hooks.cc
> > > index 1a35f4cb122..63e8500435a 100644
> > > --- a/gcc/tree-diagnostic-client-data-hooks.cc
> > > +++ b/gcc/tree-diagnostic-client-data-hooks.cc
> > > @@ -1,5 +1,5 @@
> > >  /* Implementation of diagnostic_client_data_hooks for the
> > > compilers
> > > -   (e.g. with knowledge of "tree" and lang_hooks).
> > > +   (e.g. with knowledge of "tree", lang_hooks, and timevars).
> > >     Copyright (C) 2022-2023 Free Software Foundation, Inc.
> > >     Contributed by David Malcolm <dmalcolm@redhat.com>.
> > >
> > > @@ -27,8 +27,10 @@ along with GCC; see the file COPYING3.  If not
> > > see
> > >  #include "diagnostic.h"
> > >  #include "tree-logical-location.h"
> > >  #include "diagnostic-client-data-hooks.h"
> > > +#include "diagnostic-format-sarif.h"
> > >  #include "langhooks.h"
> > >  #include "plugin.h"
> > > +#include "timevar.h"
> > >
> > >  /* Concrete class for supplying a diagnostic_context with
> > > information
> > >     about a specific plugin within the client, when the client is
> > > the
> > > @@ -111,7 +113,7 @@ private:
> > >  };
> > >
> > >  /* Subclass of diagnostic_client_data_hooks for use by compilers
> > > proper
> > > -   i.e. with knowledge of "tree", access to langhooks, etc.  */
> > > +   i.e. with knowledge of "tree", access to langhooks, timevars
> > > etc.  */
> > >
> > >  class compiler_data_hooks : public diagnostic_client_data_hooks
> > >  {
> > > @@ -135,6 +137,18 @@ public:
> > >      return lang_hooks.get_sarif_source_language (filename);
> > >    }
> > >
> > > +  void
> > > +  add_sarif_invocation_properties (sarif_object &invocation_obj)
> > > +    const final override
> > > +  {
> > > +    if (g_timer && sarif_time_report)
> > > +      if (json::value *timereport_val = g_timer->make_json ())
> > > +       {
> > > +         sarif_property_bag &bag_obj =
> > > invocation_obj.get_or_create_properties ();
> > > +         bag_obj.set ("gcc/timeReport", timereport_val);
> > > +       }
> > > +  }
> > > +
> > >  private:
> > >    compiler_version_info m_version_info;
> > >    current_fndecl_logical_location m_current_fndecl_logical_loc;
> > >
> >
>