public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: David Malcolm <dmalcolm@redhat.com>
To: Richard Biener <richard.guenther@gmail.com>
Cc: Richard Biener <rguenther@suse.de>,
	gcc-patches@gcc.gnu.org, Jakub Jelinek <jakub@redhat.com>
Subject: [PATCH v2] SARIF and -ftime-report's output [PR109361]
Date: Fri, 28 Jul 2023 16:19:44 -0400	[thread overview]
Message-ID: <ab58900340f81c5ebf4743d5a8b756878c21fa7e.camel@redhat.com> (raw)
In-Reply-To: <CAFiYyc39g-ouZBa8TRM+QC3cLNaio_+GxMYUb_WXdmCjFNkF3A@mail.gmail.com>

On Fri, 2023-07-28 at 08:00 +0200, Richard Biener wrote:
> 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.

[...snip...]

> > > 
> > > '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.

Thanks.

Here's an updated version of the patch which implements (a).

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
As before, I've tested this with my analyzer integration testsuite and
was able to use the .sarif data to generate reports about which source
files get slowed down by the analyzer [1]. I've validated the generated
.sarif files against the SARIF schema.

OK for trunk?
Dave
[1] https://github.com/davidmalcolm/gcc-analyzer-integration-tests/issues/5


This patch adds support for embeddding profiling information about the
compiler itself into the SARIF output.

Specifically, if SARIF diagnostic output is requested, via
-fdiagnostics-format=sarif-file or -fdiagnostics-format=sarif-stderr,
then any -ftime-report output is written in JSON form into the SARIF
output, rather than to stderr.

In earlier versions 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 fouling my build scripts).

The timing information is written to the 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
              }
          }
      }
  ]

The documentation notes that the precise output format is subject
to change.

gcc/ChangeLog:
	PR analyzer/109361
	* diagnostic-client-data-hooks.h (class sarif_object): New forward
	decl.
	(diagnostic_client_data_hooks::add_sarif_invocation_properties):
	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): Clarify that -ftime-report
	writes to stderr.  Document that if SARIF diagnostic output is
	requested then any timing information is written in JSON form as
	part of the SARIF output, rather than to stderr.
	* 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.
	* 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/diagnostic-client-data-hooks.h            |   6 +
 gcc/diagnostic-format-sarif.cc                |  45 +++--
 gcc/diagnostic-format-sarif.h                 |  45 +++++
 gcc/doc/invoke.texi                           |  12 +-
 .../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/tree-diagnostic-client-data-hooks.cc      |  26 ++-
 9 files changed, 326 insertions(+), 16 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/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 5e483988027..1eff71962d7 100644
--- a/gcc/diagnostic-format-sarif.cc
+++ b/gcc/diagnostic-format-sarif.cc
@@ -32,13 +32,14 @@ along with GCC; see the file COPYING3.  If not see
 #include "diagnostic-client-data-hooks.h"
 #include "diagnostic-diagram.h"
 #include "text-art/canvas.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 ()
@@ -49,17 +50,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) {}
@@ -79,13 +80,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,
@@ -232,7 +233,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.  */
@@ -250,16 +268,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
@@ -319,7 +342,7 @@ sarif_result::add_related_location (json::object *location_obj)
   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.  */
@@ -415,7 +438,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 fa765d5a0dd..5ea45c6566d 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -19932,8 +19932,16 @@ print some statistics about each pass when it finishes.
 
 @opindex ftime-report
 @item -ftime-report
-Makes the compiler print some statistics about the time consumed by each
-pass when it finishes.
+Makes the compiler print some statistics to stderr about the time consumed
+by each pass when it finishes.
+
+If SARIF output of diagnostics was requested via
+@option{-fdiagnostics-format=sarif-file} or
+@option{-fdiagnostics-format=sarif-stderr} then the @option{-ftime-report}
+information is instead emitted in JSON form as part of SARIF output.  The
+precise format of this JSON data is subject to change, and the values may
+not exactly match those emitted to stderr due to being written out at a
+slightly different place within the compiler.
 
 @opindex ftime-report-details
 @item -ftime-report-details
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..be6b1e76bd4
--- /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 -ftime-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..e9c2b5e072e
--- /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 -ftime-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/tree-diagnostic-client-data-hooks.cc b/gcc/tree-diagnostic-client-data-hooks.cc
index 1a35f4cb122..fc972ce0e13 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,26 @@ public:
     return lang_hooks.get_sarif_source_language (filename);
   }
 
+  void
+  add_sarif_invocation_properties (sarif_object &invocation_obj)
+    const final override
+  {
+    if (g_timer)
+      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);
+
+	  /* If the user requested SARIF output, then assume they want the
+	     time report data in the SARIF output, and *not* later emitted on
+	     stderr.
+	     Implement this by cleaning up the global timer instance now.  */
+	  delete g_timer;
+	  g_timer = NULL;
+	}
+  }
+
 private:
   compiler_version_info m_version_info;
   current_fndecl_logical_location m_current_fndecl_logical_loc;
-- 
2.26.3





  reply	other threads:[~2023-07-28 20:19 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-04 17:08 [PATCH] Add -fsarif-time-report [PR109361] David Malcolm
2023-04-11  8:43 ` Richard Biener
2023-07-27 22:22   ` David Malcolm
2023-07-28  6:00     ` Richard Biener
2023-07-28 20:19       ` David Malcolm [this message]
2023-07-31  7:06         ` [PATCH v2] SARIF and -ftime-report's output [PR109361] Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ab58900340f81c5ebf4743d5a8b756878c21fa7e.camel@redhat.com \
    --to=dmalcolm@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=rguenther@suse.de \
    --cc=richard.guenther@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).