| Message ID | 20230404170857.608270-1-dmalcolm@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add -fsarif-time-report [PR109361] | expand |
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. > * 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): 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; >
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. 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; > > >
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; > > > > > >
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;
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. 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. * 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): 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