[v2] Enable match.pd dumping with -fdump-tree-original
Commit Message
This is a respin of:
https://gcc.gnu.org/pipermail/gcc-patches/2022-March/592307.html
that implements Richard's suggestion around the cgraph.cc change.
Otherwise the patch is as before.
Bootstrapped and regtested on aarch64-linux-gnu, OK for trunk?
Thanks,
Alex
------
I noticed that, while the C/C++ frontends invoke the GENERIC match.pd
simplifications to do early folding, the debug output from
generic-match.cc does not appear in the -fdump-tree-original output,
even with -fdump-tree-original-folding or -fdump-tree-original-all. This
patch fixes that.
For example, before the patch, for the following code:
int a[2];
void bar ();
void f()
{
if ((unsigned long)(a + 1) == 0)
bar ();
}
on AArch64 at -O0, -fdump-tree-original-all would give:
;; Function f (null)
;; enabled by -tree-original
{
if (0)
{
bar ();
}
}
After the patch, we get:
Applying pattern match.pd:3774, generic-match.cc:24535
Matching expression match.pd:146, generic-match.cc:23
Applying pattern match.pd:5638, generic-match.cc:13388
;; Function f (null)
;; enabled by -tree-original
{
if (0)
{
bar ();
}
}
The reason we don't get the match.pd output as it stands, is that the
original dump is treated specially in c-opts.cc: it gets its own state
which is independent from that used by other dump files in the compiler.
Like most of the compiler, the generated generic-match.cc has code of
the form:
if (dump_file && (dump_flags & TDF_FOLDING))
fprintf (dump_file, ...);
But, as it stands, -fdump-tree-original has its own FILE * and flags in
c-opts.cc (original_dump_{file,flags}) and never touches the global
dump_{file,flags} (managed by dumpfile.{h,cc}). This patch adjusts the
code in c-opts.cc to use the main dump infrastructure used by the rest
of the compiler, instead of treating the original dump specially.
We take the opportunity to make a small refactor: the code in
c-gimplify.cc:c_genericize can, with this change, use the global dump
infrastructure to get the original dump file and flags instead of using
the bespoke get_dump_info function implemented in c-opts.cc. With this
change, we remove the only use of get_dump_info, so this can be removed.
Note that we also fix a leak of the original dump file in
c_common_parse_file. I originally thought it might be possible to
achieve this with only one static call to dump_finish () (by simply
moving it earlier in the loop), but unfortunately the dump file is
required to be open while c_parse_final_cleanups runs, as we (e.g.)
perform some template instantiations here for C++, which need to appear
in the original dump file.
We adjust cgraph_node::get_create to avoid introducing noise in the
original dump file: without this, these "Introduced new external node"
lines start appearing in the original dump files, which breaks tests
that do a scan-tree-dump-times on the original dump looking for a
certain function name.
gcc/c-family/ChangeLog:
* c-common.h (get_dump_info): Delete.
* c-gimplify.cc (c_genericize): Get TDI_original dump file info
from the global dump_manager instead of the (now obsolete)
get_dump_info.
* c-opts.cc (original_dump_file): Delete.
(original_dump_flags): Delete.
(c_common_parse_file): Switch to using global dump_manager to
manage the original dump file; fix leak of dump file.
(get_dump_info): Delete.
gcc/ChangeLog:
* cgraph.cc (cgraph_node::get_create): Don't dump if the current
symtab state is PARSING.
Comments
On Fri, 6 May 2022, Alex Coplan wrote:
> This is a respin of:
> https://gcc.gnu.org/pipermail/gcc-patches/2022-March/592307.html
> that implements Richard's suggestion around the cgraph.cc change.
> Otherwise the patch is as before.
>
> Bootstrapped and regtested on aarch64-linux-gnu, OK for trunk?
OK.
Thanks,
Richard.
> Thanks,
> Alex
>
> ------
>
> I noticed that, while the C/C++ frontends invoke the GENERIC match.pd
> simplifications to do early folding, the debug output from
> generic-match.cc does not appear in the -fdump-tree-original output,
> even with -fdump-tree-original-folding or -fdump-tree-original-all. This
> patch fixes that.
>
> For example, before the patch, for the following code:
>
> int a[2];
> void bar ();
> void f()
> {
> if ((unsigned long)(a + 1) == 0)
> bar ();
> }
>
> on AArch64 at -O0, -fdump-tree-original-all would give:
>
> ;; Function f (null)
> ;; enabled by -tree-original
>
>
> {
> if (0)
> {
> bar ();
> }
> }
>
> After the patch, we get:
>
> Applying pattern match.pd:3774, generic-match.cc:24535
> Matching expression match.pd:146, generic-match.cc:23
> Applying pattern match.pd:5638, generic-match.cc:13388
>
> ;; Function f (null)
> ;; enabled by -tree-original
>
>
> {
> if (0)
> {
> bar ();
> }
> }
>
> The reason we don't get the match.pd output as it stands, is that the
> original dump is treated specially in c-opts.cc: it gets its own state
> which is independent from that used by other dump files in the compiler.
> Like most of the compiler, the generated generic-match.cc has code of
> the form:
>
> if (dump_file && (dump_flags & TDF_FOLDING))
> fprintf (dump_file, ...);
>
> But, as it stands, -fdump-tree-original has its own FILE * and flags in
> c-opts.cc (original_dump_{file,flags}) and never touches the global
> dump_{file,flags} (managed by dumpfile.{h,cc}). This patch adjusts the
> code in c-opts.cc to use the main dump infrastructure used by the rest
> of the compiler, instead of treating the original dump specially.
>
> We take the opportunity to make a small refactor: the code in
> c-gimplify.cc:c_genericize can, with this change, use the global dump
> infrastructure to get the original dump file and flags instead of using
> the bespoke get_dump_info function implemented in c-opts.cc. With this
> change, we remove the only use of get_dump_info, so this can be removed.
>
> Note that we also fix a leak of the original dump file in
> c_common_parse_file. I originally thought it might be possible to
> achieve this with only one static call to dump_finish () (by simply
> moving it earlier in the loop), but unfortunately the dump file is
> required to be open while c_parse_final_cleanups runs, as we (e.g.)
> perform some template instantiations here for C++, which need to appear
> in the original dump file.
>
> We adjust cgraph_node::get_create to avoid introducing noise in the
> original dump file: without this, these "Introduced new external node"
> lines start appearing in the original dump files, which breaks tests
> that do a scan-tree-dump-times on the original dump looking for a
> certain function name.
>
> gcc/c-family/ChangeLog:
>
> * c-common.h (get_dump_info): Delete.
> * c-gimplify.cc (c_genericize): Get TDI_original dump file info
> from the global dump_manager instead of the (now obsolete)
> get_dump_info.
> * c-opts.cc (original_dump_file): Delete.
> (original_dump_flags): Delete.
> (c_common_parse_file): Switch to using global dump_manager to
> manage the original dump file; fix leak of dump file.
> (get_dump_info): Delete.
>
> gcc/ChangeLog:
>
> * cgraph.cc (cgraph_node::get_create): Don't dump if the current
> symtab state is PARSING.
>
@@ -950,7 +950,6 @@ extern bool c_common_post_options (const char **);
extern bool c_common_init (void);
extern void c_common_finish (void);
extern void c_common_parse_file (void);
-extern FILE *get_dump_info (int, dump_flags_t *);
extern alias_set_type c_common_get_alias_set (tree);
extern void c_register_builtin_type (tree, const char*);
extern bool c_promoting_integer_type_p (const_tree);
@@ -40,6 +40,7 @@ along with GCC; see the file COPYING3. If not see
#include "dumpfile.h"
#include "c-ubsan.h"
#include "tree-nested.h"
+#include "context.h"
/* The gimplification pass converts the language-dependent trees
(ld-trees) emitted by the parser into language-independent trees
@@ -552,6 +553,7 @@ c_genericize_control_r (tree *stmt_p, int *walk_subtrees, void *data)
void
c_genericize (tree fndecl)
{
+ dump_file_info *dfi;
FILE *dump_orig;
dump_flags_t local_dump_flags;
struct cgraph_node *cgn;
@@ -581,7 +583,9 @@ c_genericize (tree fndecl)
do_warn_duplicated_branches_r, NULL);
/* Dump the C-specific tree IR. */
- dump_orig = get_dump_info (TDI_original, &local_dump_flags);
+ dfi = g->get_dumps ()->get_dump_file_info (TDI_original);
+ dump_orig = dfi->pstream;
+ local_dump_flags = dfi->pflags;
if (dump_orig)
{
fprintf (dump_orig, "\n;; Function %s",
@@ -41,6 +41,7 @@ along with GCC; see the file COPYING3. If not see
#include "mkdeps.h"
#include "dumpfile.h"
#include "file-prefix-map.h" /* add_*_prefix_map() */
+#include "context.h"
#ifndef DOLLARS_IN_IDENTIFIERS
# define DOLLARS_IN_IDENTIFIERS true
@@ -100,10 +101,6 @@ static size_t deferred_count;
/* Number of deferred options scanned for -include. */
static size_t include_cursor;
-/* Dump files/flags to use during parsing. */
-static FILE *original_dump_file = NULL;
-static dump_flags_t original_dump_flags;
-
/* Whether any standard preincluded header has been preincluded. */
static bool done_preinclude;
@@ -1226,15 +1223,13 @@ c_common_init (void)
void
c_common_parse_file (void)
{
- unsigned int i;
-
- i = 0;
- for (;;)
+ auto dumps = g->get_dumps ();
+ for (unsigned int i = 0;;)
{
c_finish_options ();
/* Open the dump file to use for the original dump output
here, to be used during parsing for the current file. */
- original_dump_file = dump_begin (TDI_original, &original_dump_flags);
+ dumps->dump_start (TDI_original, &dump_flags);
pch_init ();
push_file_scope ();
c_parse_file ();
@@ -1248,29 +1243,15 @@ c_common_parse_file (void)
cpp_clear_file_cache (parse_in);
this_input_filename
= cpp_read_main_file (parse_in, in_fnames[i]);
- if (original_dump_file)
- {
- dump_end (TDI_original, original_dump_file);
- original_dump_file = NULL;
- }
/* If an input file is missing, abandon further compilation.
cpplib has issued a diagnostic. */
if (!this_input_filename)
break;
+ dumps->dump_finish (TDI_original);
}
c_parse_final_cleanups ();
-}
-
-/* Returns the appropriate dump file for PHASE to dump with FLAGS. */
-
-FILE *
-get_dump_info (int phase, dump_flags_t *flags)
-{
- gcc_assert (phase == TDI_original);
-
- *flags = original_dump_flags;
- return original_dump_file;
+ dumps->dump_finish (TDI_original);
}
/* Common finish hook for the C, ObjC and C++ front ends. */
@@ -538,6 +538,7 @@ cgraph_node::get_create (tree decl)
return first_clone;
cgraph_node *node = cgraph_node::create (decl);
+
if (first_clone)
{
first_clone->clone_of = node;
@@ -545,12 +546,12 @@ cgraph_node::get_create (tree decl)
node->order = first_clone->order;
symtab->symtab_prevail_in_asm_name_hash (node);
node->decl->decl_with_vis.symtab_node = node;
- if (dump_file)
+ if (dump_file && symtab->state != PARSING)
fprintf (dump_file, "Introduced new external node "
"(%s) and turned into root of the clone tree.\n",
node->dump_name ());
}
- else if (dump_file)
+ else if (dump_file && symtab->state != PARSING)
fprintf (dump_file, "Introduced new external node "
"(%s).\n", node->dump_name ());
return node;